From 3634677634406713ecd5841b6dc6e2a672c0a017 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 12:27:10 +0000 Subject: [PATCH 1/7] Start fix for #933 Assisted-by: Claude Code:claude-opus-5-5 From 8990c2e948f1063d5e67aef7b144d99ddc3488b3 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 12:30:20 +0000 Subject: [PATCH 2/7] Test rollback/remove of a superseded agent record Regression tests for #933: an agent-mode record A left in the manifest after a hosted scan pinned the superseding patch B. Rollback and remove must restore the lock, drop record A and exit 0, both after a reinstall (tree holds B's bytes) and before one (tree still holds A's bytes). Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/in_process_rollback_hosted.rs | 184 ++++++++++++++++++ 1 file changed, 184 insertions(+) diff --git a/crates/socket-patch-cli/tests/in_process_rollback_hosted.rs b/crates/socket-patch-cli/tests/in_process_rollback_hosted.rs index d1b5b1607..32cdb0d0f 100644 --- a/crates/socket-patch-cli/tests/in_process_rollback_hosted.rs +++ b/crates/socket-patch-cli/tests/in_process_rollback_hosted.rs @@ -1372,3 +1372,187 @@ async fn npm_hosted_round_trip_manifest_less_vex() { .expect("manifest-less VEX cells panicked"); }); } + +// --------------------------------------------------------------------------- +// Agent record superseded by a hosted pin (#933) +// --------------------------------------------------------------------------- +// +// An agent-mode apply recorded patch A in `.socket/manifest.json`; a later +// hosted scan pinned the same `name@version` to a SUPERSEDING patch (the +// fixture's UUID, B) and left record A in place. After a reinstall the tree +// holds B's bytes, which are neither of A's sides, so restoring A in place +// would fail "modified after patching". The hosted leg owns that package +// now: rollback and remove restore the lock, drop the superseded record +// with a warning, and exit 0. + +const SUPERSEDED_UUID: &str = "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa"; +const ORIGINAL_INDEX: &[u8] = b"module.exports = 'original';\n"; +const A_PATCHED_INDEX: &[u8] = b"module.exports = 'patched by A';\n"; +const B_PATCHED_INDEX: &[u8] = b"module.exports = 'patched by B';\n"; + +/// The npm project wired by a real hosted scan to patch B, with agent +/// record A left in the manifest (its before-blob cached, as agent apply +/// leaves it) and `installed` as the installed `index.js`. Returns the +/// pristine lock bytes. +async fn write_superseded_agent_fixture(root: &Path, server: &MockServer, installed: &[u8]) -> String { + let pristine = write_npm_project(root); + let pkg = root.join("node_modules").join(NAME); + std::fs::write(pkg.join("index.js"), A_PATCHED_INDEX).unwrap(); + + let before = socket_patch_core::hash::git_sha256::compute_git_sha256_from_bytes(ORIGINAL_INDEX); + let after = socket_patch_core::hash::git_sha256::compute_git_sha256_from_bytes(A_PATCHED_INDEX); + let mut record = patch_record(SUPERSEDED_UUID, GHSA); + record.files.clear(); + record.files.insert( + "package/index.js".to_string(), + PatchFileInfo { + before_hash: before.clone(), + after_hash: after, + }, + ); + let mut manifest = socket_patch_core::manifest::schema::PatchManifest::new(); + manifest.patches.insert(PURL.to_string(), record); + std::fs::create_dir_all(root.join(".socket/blobs")).unwrap(); + std::fs::write(root.join(".socket/blobs").join(&before), ORIGINAL_INDEX).unwrap(); + std::fs::write( + root.join(".socket/manifest.json"), + serde_json::to_string_pretty(&manifest).unwrap(), + ) + .unwrap(); + + mock_discovery(server).await; + mock_reference(server).await; + mock_view(server).await; + let code = scan_run(hosted_scan_args(root, server.uri())).await; + assert_eq!(code, 0, "the superseding hosted scan should succeed"); + let wired = std::fs::read_to_string(root.join("package-lock.json")).unwrap(); + assert!( + wired.contains(HOSTED_URL), + "the lock must pin the superseding hosted patch; got:\n{wired}" + ); + let manifest = std::fs::read_to_string(root.join(".socket/manifest.json")).unwrap(); + assert!( + manifest.contains(SUPERSEDED_UUID), + "the hosted scan leaves the superseded agent record A in place; got:\n{manifest}" + ); + + // What the next `npm ci` (or no reinstall at all) leaves installed. + std::fs::write(pkg.join("index.js"), installed).unwrap(); + mock_npm_registry(server).await; + pristine +} + +fn manifest_patch_keys(root: &Path) -> Vec { + let raw = std::fs::read_to_string(root.join(".socket/manifest.json")).unwrap(); + let v: Value = serde_json::from_str(&raw).unwrap(); + v["patches"] + .as_object() + .map(|m| m.keys().cloned().collect()) + .unwrap_or_default() +} + +/// Run `remove --json --yes` online as a scrubbed subprocess. +fn run_remove_subprocess_online(cwd: &Path, server: &MockServer, identifier: &str) -> (i32, Value) { + let out = scrubbed_cli() + .env( + "SOCKET_NPM_REGISTRY", + format!("{}/npm-registry", server.uri()), + ) + .args([ + "remove", + identifier, + "--json", + "--yes", + "--patch-server-url", + "http://patch.test", + "--cwd", + cwd.to_str().unwrap(), + ]) + .output() + .expect("run socket-patch"); + let envelope: Value = serde_json::from_slice(&out.stdout).unwrap_or_else(|e| { + panic!( + "remove --json stdout must be a pure JSON envelope: {e}\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr) + ) + }); + (out.status.code().unwrap_or(-1), envelope) +} + +/// #933: after the reinstall the tree holds B's bytes. Rollback restores +/// the lock, drops record A with `rollback_record_superseded`, leaves the +/// installed bytes for the reinstall to replace, and exits 0 — twice. +#[tokio::test] +#[serial] +async fn rollback_drops_an_agent_record_superseded_by_a_hosted_pin() { + let server = MockServer::start().await; + let tmp = tempfile::tempdir().unwrap(); + let pristine = write_superseded_agent_fixture(tmp.path(), &server, B_PATCHED_INDEX).await; + + let (code, envelope) = run_rollback_subprocess_online(tmp.path(), &server, &[]); + assert_eq!(code, 0, "rollback must not fail on the superseded record:\n{envelope:#}"); + assert!( + warning_codes(&envelope).contains(&"rollback_record_superseded".to_string()), + "the superseded record is named in warnings[]:\n{envelope:#}" + ); + let restored = std::fs::read_to_string(tmp.path().join("package-lock.json")).unwrap(); + assert_eq!(restored, pristine, "the hosted leg restores the lock"); + assert!( + manifest_patch_keys(tmp.path()).is_empty(), + "the superseded record leaves the manifest" + ); + let installed = std::fs::read(tmp.path().join("node_modules").join(NAME).join("index.js")).unwrap(); + assert_eq!( + installed, B_PATCHED_INDEX, + "B's installed bytes are the reinstall's to replace, never overwritten with A's original" + ); + + let (code, envelope) = run_rollback_subprocess_online(tmp.path(), &server, &[]); + assert_eq!(code, 0, "a re-run is a clean no-op:\n{envelope:#}"); +} + +/// #933 without a reinstall: the tree still holds A's patched bytes, so +/// the agent leg restores them in place as before. +#[tokio::test] +#[serial] +async fn rollback_restores_a_superseded_agent_record_still_installed() { + let server = MockServer::start().await; + let tmp = tempfile::tempdir().unwrap(); + let pristine = write_superseded_agent_fixture(tmp.path(), &server, A_PATCHED_INDEX).await; + + let (code, envelope) = run_rollback_subprocess_online(tmp.path(), &server, &[]); + assert_eq!(code, 0, "{envelope:#}"); + assert!( + !warning_codes(&envelope).contains(&"rollback_record_superseded".to_string()), + "A's bytes were restored in place, nothing was left to the reinstall:\n{envelope:#}" + ); + let restored = std::fs::read_to_string(tmp.path().join("package-lock.json")).unwrap(); + assert_eq!(restored, pristine); + assert!(manifest_patch_keys(tmp.path()).is_empty()); + let installed = std::fs::read(tmp.path().join("node_modules").join(NAME).join("index.js")).unwrap(); + assert_eq!(installed, ORIGINAL_INDEX); +} + +/// #933: `remove ` un-hosts the package instead of aborting on the +/// superseded agent record before the hosted leg runs. +#[tokio::test] +#[serial] +async fn remove_unhosts_a_package_whose_agent_record_is_superseded() { + let server = MockServer::start().await; + let tmp = tempfile::tempdir().unwrap(); + let pristine = write_superseded_agent_fixture(tmp.path(), &server, B_PATCHED_INDEX).await; + + let (code, envelope) = run_remove_subprocess_online(tmp.path(), &server, PURL); + assert_eq!(code, 0, "remove must not refuse the superseded record:\n{envelope:#}"); + let restored = std::fs::read_to_string(tmp.path().join("package-lock.json")).unwrap(); + assert_eq!( + restored, pristine, + "remove must not leave the hosted pin live" + ); + assert!(manifest_patch_keys(tmp.path()).is_empty()); + assert!( + envelope.to_string().contains("rollback_record_superseded"), + "the superseded record is reported:\n{envelope:#}" + ); +} From b43e50c68ca3bf099fe51626187373c977c574f2 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 12:37:15 +0000 Subject: [PATCH 3/7] Let rollback/remove unwind superseded records After an agent-to-hosted migration where the patch was replaced, the manifest still recorded agent patch A while the lockfile pinned hosted patch B. Rollback tried to restore A in place over B's installed bytes and exited 1 with "modified after patching" on every run; remove aborted the same way before restoring the lock, leaving B live. A manifest record whose package release a live hosted pin wires to a different patch is now superseded. When its installed copy holds neither side of the record, rollback and remove leave the copy to the hosted lock restore and the next install, report rollback_record_superseded and drop the record instead of failing. A copy that still holds A's patched bytes is restored as before. Fixes #933 Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 4 +- .../socket-patch-cli/src/commands/remove.rs | 1 + .../socket-patch-cli/src/commands/rollback.rs | 101 +++++++++++++++++- .../tests/in_process_rollback_hosted.rs | 22 +++- 4 files changed, 120 insertions(+), 8 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index dff38f2c8..212b49d0e 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -850,7 +850,7 @@ A bare `rollback` (or a scoped one, for its scope) restores the SYSTEM to unpatc 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. 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. 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). +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). 6. **GC** — blob, diff and legacy package-archive sweeps against the post-removal manifest, using the same artifact-reference policy as `remove`, retaining beforeHash blobs for (a) removed-but-not-installed entries (a crawler miss must not destroy the only local revert data — `remove` parity) and (b) EVERY entry remaining in the post-removal manifest — still-active patches (failed, drift-kept, eco-/path-excluded) keep their revert data, so a scoped or failed run never destroys the blobs a later rollback needs; only blobs referenced solely by genuinely-removed entries are swept. GC errors warn (`cleanup_failed`) and continue — they never affect the exit (repair's posture). **Confirmation prompt.** A wet, non-preserve run with work prompts once, remove-style, composing only the clauses that apply into one English list (`a and b`, `a, b, and c`) with counted nouns: `Roll back N patches`, `remove them from the local manifest`, `delete M vendored artifacts and their ledger records`, `restore H hosted packages to the upstream registry` (e.g. `Roll back 1 patch, remove it from the local manifest, and restore 1 hosted package to the upstream registry?`) — default yes, auto-accepted under `--yes`/`--json`/non-TTY (the shared `confirm` semantics; CI unaffected). Decline prints `Rollback cancelled.` and exits 0. `--dry-run` and `--preserve-state` runs are prompt-free (they delete no local state). @@ -886,7 +886,7 @@ v5.0 replaces v4's per-purl reverts and whole-ledger reverse replay (`revert_rem | Key | Shape | Meaning | |---|---|---| -| `warnings` | `[{code, detail}]` | Run-level warnings, now populated (previously always empty): `reinstall_required`, `hosted_state_not_preservable`, `out_of_scope_copies_restored`, `vendor_state_unreadable`, `cleanup_failed`, `manifest_write_failed`, `legacy_redirect_ledger_kept`, the upstream-restore advisories (`npm_allow_remote_left`, `pnpm_trust_lockfile_left`, `maven_trusted_checksums_left`, `nuget_default_config_left`, `upstream_uv_override_removed`), `ownership_not_restored` (a restored file whose ownership could not be put back — see the apply warnings), plus vendored/hosted leg advisories. New codes are additive (MINOR) | +| `warnings` | `[{code, detail}]` | Run-level warnings, now populated (previously always empty): `reinstall_required`, `hosted_state_not_preservable`, `out_of_scope_copies_restored`, `vendor_state_unreadable`, `cleanup_failed`, `manifest_write_failed`, `legacy_redirect_ledger_kept`, the upstream-restore advisories (`npm_allow_remote_left`, `pnpm_trust_lockfile_left`, `maven_trusted_checksums_left`, `nuget_default_config_left`, `upstream_uv_override_removed`), `ownership_not_restored` (a restored file whose ownership could not be put back — see the apply warnings), `rollback_record_superseded` (a manifest record superseded by a live hosted pin, left to the hosted leg — see Manifest cleanup), plus vendored/hosted leg advisories. New codes are additive (MINOR) | | `vendored` | `[purl]` | **Meaning narrowed (MAJOR)**: vendor-owned purls the run did NOT act on — today exactly the corrupt-vendor-ledger skip. | | `vendoredReverted` | `[purl]` | Ledger entries cleanly reverted this run (unwired + artifact deleted + entry dropped; previewed on dry-run) | | `vendoredPreserved` | `[purl]` | `--preserve-state`: unwired with artifact + ledger entry kept | diff --git a/crates/socket-patch-cli/src/commands/remove.rs b/crates/socket-patch-cli/src/commands/remove.rs index c5fbed562..d69b1183e 100644 --- a/crates/socket-patch-cli/src/commands/remove.rs +++ b/crates/socket-patch-cli/src/commands/remove.rs @@ -603,6 +603,7 @@ pub async fn run(args: RemoveArgs) -> i32 { &manifest, &vendored_keys, InnerSelection::Identifier(Some(&args.identifier)), + &super::rollback::superseded_by_hosted(&manifest, &hosted_pins), Some(&telemetry_client), ) .await diff --git a/crates/socket-patch-cli/src/commands/rollback.rs b/crates/socket-patch-cli/src/commands/rollback.rs index 29369279c..b901b54e3 100644 --- a/crates/socket-patch-cli/src/commands/rollback.rs +++ b/crates/socket-patch-cli/src/commands/rollback.rs @@ -14,8 +14,10 @@ use socket_patch_core::patch::rollback::{ VerifyRollbackResult, VerifyRollbackStatus, }; use socket_patch_core::telemetry::{track_patch_rollback_failed, track_patch_rolled_back}; +use socket_patch_core::utils::composer_version::composer_purls_equivalent; use socket_patch_core::utils::purl::{patch_matches, strip_purl_qualifiers}; use socket_patch_core::vendor::{purl_keys_cover, RevertOpts, VendorState}; +use socket_patch_core::vex::discover::canonical_base_purl; use std::collections::{HashMap, HashSet}; use std::path::{Path, PathBuf}; use std::time::Duration; @@ -441,8 +443,13 @@ pub(crate) struct RollbackOutcome { /// the revert data the retry needs. pub(crate) aborted: bool, /// Run warnings `(code, detail)` for copies left alone without failing - /// the run (today: `gradle_m2_copy_not_restored`). + /// the run (`gradle_m2_copy_not_restored`, `rollback_record_superseded`). pub(crate) warnings: Vec<(String, String)>, + /// In-scope manifest entries superseded by a live hosted pin whose + /// installed copies hold neither side of the recorded patch (#933): + /// left to the hosted leg's lock restore instead of failing, and + /// removable from the manifest like a rolled-back entry. Sorted. + pub(crate) superseded: Vec, } /// How `rollback_patches_inner` selects manifest entries. @@ -1449,6 +1456,7 @@ pub async fn run(args: RollbackArgs) -> i32 { &manifest, &vendored_keys, selection, + &superseded_by_hosted(&manifest, &hosted_pins), Some(&telemetry_client), ) .await @@ -1461,6 +1469,7 @@ pub async fn run(args: RollbackArgs) -> i32 { narrowed_out, aborted, warnings: agent_warnings, + superseded, }) => { // Copies left alone without failing the run (an unconsumed // `~/.m2` copy: `gradle_m2_copy_not_restored`). @@ -1562,6 +1571,7 @@ pub async fn run(args: RollbackArgs) -> i32 { } succeeded_purls.contains(*purl) || not_installed.contains(purl) + || superseded.contains(purl) || (narrowed_out.contains(purl) && !failed_bases.contains(strip_purl_qualifiers(purl))) }) @@ -2015,6 +2025,10 @@ pub(crate) async fn rollback_patches_inner( manifest: &PatchManifest, vendored_keys: &HashSet, selection: InnerSelection<'_>, + // Manifest purl -> the hosted uuid a live lockfile pin superseded its + // record with ([`superseded_by_hosted`]); empty when no hosted pin + // replaces a recorded patch. + superseded: &HashMap, // The client the caller already built. Constructing one per phase // printed the core client's "No SOCKET_API_TOKEN set" notice once per // construction — twice in a single rollback. `None` builds one on @@ -2064,6 +2078,7 @@ pub(crate) async fn rollback_patches_inner( narrowed_out: Vec::new(), aborted: false, warnings: Vec::new(), + superseded: Vec::new(), }); } @@ -2091,6 +2106,7 @@ pub(crate) async fn rollback_patches_inner( narrowed_out: Vec::new(), aborted: false, warnings: Vec::new(), + superseded: Vec::new(), }); } @@ -2490,6 +2506,7 @@ pub(crate) async fn rollback_patches_inner( narrowed_out: Vec::new(), aborted: true, warnings: Vec::new(), + superseded: Vec::new(), }); } @@ -2570,6 +2587,7 @@ pub(crate) async fn rollback_patches_inner( narrowed_out: Vec::new(), aborted: true, warnings: Vec::new(), + superseded: Vec::new(), }); } } @@ -2590,6 +2608,7 @@ pub(crate) async fn rollback_patches_inner( narrowed_out: narrowed_out.clone(), aborted: false, warnings: Vec::new(), + superseded: Vec::new(), }); } @@ -2597,6 +2616,7 @@ pub(crate) async fn rollback_patches_inner( let mut results: Vec = Vec::new(); let mut has_errors = false; let mut warnings: Vec<(String, String)> = Vec::new(); + let mut superseded_left: Vec = Vec::new(); for target in &rollback_targets { let (purl, pkg_path) = (&target.purl, &target.dir); @@ -2638,6 +2658,11 @@ pub(crate) async fn rollback_patches_inner( warnings.push(warning); continue; } + if let Some(warning) = superseded_record_skip(target, &result, superseded) { + warnings.push(warning); + superseded_left.push(purl.clone()); + continue; + } if !result.success { has_errors = true; // Under --silent (the summary muted) this line is the run's @@ -2685,6 +2710,8 @@ pub(crate) async fn rollback_patches_inner( results.push(result); } + superseded_left.sort(); + superseded_left.dedup(); Ok(RollbackOutcome { success: !has_errors, results, @@ -2693,6 +2720,7 @@ pub(crate) async fn rollback_patches_inner( narrowed_out, aborted: false, warnings, + superseded: superseded_left, }) } @@ -2771,6 +2799,76 @@ fn unconsumed_m2_skip(target: &CopyTarget, result: &RollbackResult) -> Option<(S }) } +/// Manifest records a live hosted pin has superseded (#933): purl -> the +/// hosted uuid the lockfiles wire for the same package release, when no +/// hosted pin for that release carries the record's own uuid. An agent → +/// hosted migration whose patch was replaced meanwhile leaves exactly this: +/// record A in the manifest, the lock pinning B. `vex` reports the same +/// state as `vex_record_superseded`. +pub(crate) fn superseded_by_hosted( + manifest: &PatchManifest, + pins: &[HostedPin], +) -> HashMap { + manifest + .patches + .iter() + .filter_map(|(purl, record)| { + let pkg = canonical_base_purl(purl); + let same: Vec<&HostedPin> = pins + .iter() + .filter(|pin| pin.purl == pkg || composer_purls_equivalent(&pin.purl, &pkg)) + .collect(); + if same.iter().any(|pin| pin.uuid == record.uuid) { + return None; + } + same.first().map(|pin| (purl.clone(), pin.uuid.clone())) + }) + .collect() +} + +/// The run warning that replaces a failed in-place restore of a manifest +/// record a live hosted pin superseded ([`superseded_by_hosted`]), when it +/// failed before writing anything because the installed copy holds bytes +/// that are neither side of the record (the superseding patch's, after a +/// reinstall) or lacks a file. The hosted leg's lock restore and the +/// reinstall it asks for unwind that copy; restoring the record's original +/// bytes over the superseding patch's would only mix the two. `None` for +/// any other result — a copy still holding the record's patched bytes is +/// restored as usual, and a file that cannot be read fails as usual. +fn superseded_record_skip( + target: &CopyTarget, + result: &RollbackResult, + superseded: &HashMap, +) -> Option<(String, String)> { + let wired = superseded.get(&target.purl)?; + if result.success || !result.files_rolled_back.is_empty() { + return None; + } + if result + .files_verified + .iter() + .any(|v| v.status == VerifyRollbackStatus::NotFound && !v.is_absent()) + { + return None; + } + let replaced = result + .files_verified + .iter() + .any(|v| v.status == VerifyRollbackStatus::HashMismatch || v.is_absent()); + replaced.then(|| { + ( + "rollback_record_superseded".to_string(), + format!( + "{}: the recorded patch is superseded by the lockfile-wired hosted patch {wired}; \ + left the installed copy at {} to the lockfile restore (the next \ + package-manager install puts the original files back)", + target.purl, + target.dir.display() + ), + ) + }) +} + /// The key `file` of a Maven record as it is joined onto `dir`: a Gradle /// hash dir holds the bare file name (`package/` dropped). fn maven_target_key(purl: &str, dir: &Path, file: &str) -> String { @@ -3083,6 +3181,7 @@ mod tests { &manifest, &vendored_keys, InnerSelection::Identifier(identifier), + &HashMap::new(), None, ) .await?; diff --git a/crates/socket-patch-cli/tests/in_process_rollback_hosted.rs b/crates/socket-patch-cli/tests/in_process_rollback_hosted.rs index 32cdb0d0f..5091adb59 100644 --- a/crates/socket-patch-cli/tests/in_process_rollback_hosted.rs +++ b/crates/socket-patch-cli/tests/in_process_rollback_hosted.rs @@ -1394,7 +1394,11 @@ const B_PATCHED_INDEX: &[u8] = b"module.exports = 'patched by B';\n"; /// record A left in the manifest (its before-blob cached, as agent apply /// leaves it) and `installed` as the installed `index.js`. Returns the /// pristine lock bytes. -async fn write_superseded_agent_fixture(root: &Path, server: &MockServer, installed: &[u8]) -> String { +async fn write_superseded_agent_fixture( + root: &Path, + server: &MockServer, + installed: &[u8], +) -> String { let pristine = write_npm_project(root); let pkg = root.join("node_modules").join(NAME); std::fs::write(pkg.join("index.js"), A_PATCHED_INDEX).unwrap(); @@ -1491,7 +1495,10 @@ async fn rollback_drops_an_agent_record_superseded_by_a_hosted_pin() { let pristine = write_superseded_agent_fixture(tmp.path(), &server, B_PATCHED_INDEX).await; let (code, envelope) = run_rollback_subprocess_online(tmp.path(), &server, &[]); - assert_eq!(code, 0, "rollback must not fail on the superseded record:\n{envelope:#}"); + assert_eq!( + code, 0, + "rollback must not fail on the superseded record:\n{envelope:#}" + ); assert!( warning_codes(&envelope).contains(&"rollback_record_superseded".to_string()), "the superseded record is named in warnings[]:\n{envelope:#}" @@ -1502,7 +1509,8 @@ async fn rollback_drops_an_agent_record_superseded_by_a_hosted_pin() { manifest_patch_keys(tmp.path()).is_empty(), "the superseded record leaves the manifest" ); - let installed = std::fs::read(tmp.path().join("node_modules").join(NAME).join("index.js")).unwrap(); + let installed = + std::fs::read(tmp.path().join("node_modules").join(NAME).join("index.js")).unwrap(); assert_eq!( installed, B_PATCHED_INDEX, "B's installed bytes are the reinstall's to replace, never overwritten with A's original" @@ -1530,7 +1538,8 @@ async fn rollback_restores_a_superseded_agent_record_still_installed() { let restored = std::fs::read_to_string(tmp.path().join("package-lock.json")).unwrap(); assert_eq!(restored, pristine); assert!(manifest_patch_keys(tmp.path()).is_empty()); - let installed = std::fs::read(tmp.path().join("node_modules").join(NAME).join("index.js")).unwrap(); + let installed = + std::fs::read(tmp.path().join("node_modules").join(NAME).join("index.js")).unwrap(); assert_eq!(installed, ORIGINAL_INDEX); } @@ -1544,7 +1553,10 @@ async fn remove_unhosts_a_package_whose_agent_record_is_superseded() { let pristine = write_superseded_agent_fixture(tmp.path(), &server, B_PATCHED_INDEX).await; let (code, envelope) = run_remove_subprocess_online(tmp.path(), &server, PURL); - assert_eq!(code, 0, "remove must not refuse the superseded record:\n{envelope:#}"); + assert_eq!( + code, 0, + "remove must not refuse the superseded record:\n{envelope:#}" + ); let restored = std::fs::read_to_string(tmp.path().join("package-lock.json")).unwrap(); assert_eq!( restored, pristine, From 9c0a0c3b29f8416a313736717d2e7b43a5c601a7 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 18:24:21 +0000 Subject: [PATCH 4/7] Route Gradle digests through utils::digest main has failed socket-patch-core's lib tests since Gradle support (#646) and the digest helpers (#865) both landed. The guard test production_digests_go_through_the_helpers flags three files #646 added that still hash inline: crawlers/gradle_cache.rs, patch/jvm_jar.rs and patch/sidecars/maven.rs. That breaks test, test-release and coverage on every open PR. Each inline sha1/sha256 call now goes through sha1_hex_of or sha256_hex_of, which compute the same lowercase hex. Behaviour is unchanged. Assisted-by: Claude Code:claude-opus-5-5 (cherry picked from commit 659ac2c24e5c5904e743b4bc98ea4645da2ed6a1) --- crates/socket-patch-core/src/crawlers/gradle_cache.rs | 9 ++++----- crates/socket-patch-core/src/patch/jvm_jar.rs | 7 ++----- crates/socket-patch-core/src/patch/sidecars/maven.rs | 4 +--- 3 files changed, 7 insertions(+), 13 deletions(-) diff --git a/crates/socket-patch-core/src/crawlers/gradle_cache.rs b/crates/socket-patch-core/src/crawlers/gradle_cache.rs index ef295ee27..afd7c4fba 100644 --- a/crates/socket-patch-core/src/crawlers/gradle_cache.rs +++ b/crates/socket-patch-core/src/crawlers/gradle_cache.rs @@ -70,8 +70,7 @@ pub fn hash_eq(dir_name: &str, sha1_hex: &str) -> bool { /// Whether `bytes` are the pristine download Gradle stored in the hash /// directory `dir_name` (their sha1 names it). pub fn pristine(dir_name: &str, bytes: &[u8]) -> bool { - use sha1::{Digest, Sha1}; - hash_eq(dir_name, &hex::encode(Sha1::digest(bytes))) + hash_eq(dir_name, &crate::utils::digest::sha1_hex_of(bytes)) } /// Whether `path` is a version directory of a `files-2.1` tree @@ -432,8 +431,6 @@ impl DerivedIndex { /// The [`DerivedCopies`] of the jar `jar_leaf` whose pristine bytes /// hash to `pristine_sha1`. pub fn query(&self, jar_leaf: &str, pristine_sha1: &str) -> DerivedCopies { - use sha1::{Digest, Sha1}; - let instrumented = format!("instrumented-{jar_leaf}"); let mut out = DerivedCopies { incomplete: self.incomplete, @@ -460,7 +457,9 @@ impl DerivedIndex { out.stale.push(path.clone()); } else if name == jar_leaf || name == instrumented { match crate::utils::fs::read_regular_to_bytes_sync(path) { - Ok(bytes) if hash_eq(&hex::encode(Sha1::digest(&bytes)), pristine_sha1) => { + Ok(bytes) + if hash_eq(&crate::utils::digest::sha1_hex_of(&bytes), pristine_sha1) => + { out.stale.push(path.clone()) } Ok(_) => out.unknown.push(path.clone()), diff --git a/crates/socket-patch-core/src/patch/jvm_jar.rs b/crates/socket-patch-core/src/patch/jvm_jar.rs index 82d679406..f38a84403 100644 --- a/crates/socket-patch-core/src/patch/jvm_jar.rs +++ b/crates/socket-patch-core/src/patch/jvm_jar.rs @@ -25,8 +25,6 @@ use std::collections::HashMap; use std::path::{Path, PathBuf}; -use sha1::Digest as _; - use crate::crawlers::gradle_cache; use crate::hash::git_sha256::compute_git_sha256_from_bytes; use crate::manifest::schema::PatchFileInfo; @@ -353,12 +351,11 @@ fn unpatched_members( } fn sha256_hex(bytes: &[u8]) -> String { - use sha2::Digest as _; - hex::encode(sha2::Sha256::digest(bytes)) + crate::utils::digest::sha256_hex_of(bytes) } fn sha1_hex(bytes: &[u8]) -> String { - hex::encode(sha1::Sha1::digest(bytes)) + crate::utils::digest::sha1_hex_of(bytes) } /// `/jvm-originals/.jar`. diff --git a/crates/socket-patch-core/src/patch/sidecars/maven.rs b/crates/socket-patch-core/src/patch/sidecars/maven.rs index f2f5a2466..8798bfce6 100644 --- a/crates/socket-patch-core/src/patch/sidecars/maven.rs +++ b/crates/socket-patch-core/src/patch/sidecars/maven.rs @@ -17,8 +17,6 @@ use std::path::{Path, PathBuf}; -use sha1::Digest as _; - use super::{ SidecarAdvisory, SidecarAdvisoryCode, SidecarError, SidecarFile, SidecarFileAction, SidecarPayload, SidecarSeverity, @@ -44,7 +42,7 @@ impl Algo { fn digest(self, bytes: &[u8]) -> String { match self { - Algo::Sha1 => hex::encode(sha1::Sha1::digest(bytes)), + Algo::Sha1 => crate::utils::digest::sha1_hex_of(bytes), Algo::Md5 => hex::encode(md5(bytes)), } } From 66ec386ba87fc2750efaf32cddd66f842b91fc58 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 13:22:38 +0000 Subject: [PATCH 5/7] Treat unpatched JVM copies as superseded too A Gradle or Maven copy holding the superseding hosted patch's jar fails rollback before file verification: its hash directory is not the download the old record patched (gradle_rollback_hash_mismatch), or its swapped jar has no backup here (jvm_jar_backup_missing). Those copies now get the same rollback_record_superseded handling as a plain hash mismatch, so remove no longer aborts on them before restoring the lock. Neither refusal writes anything. Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-cli/src/commands/rollback.rs | 94 ++++++++++++++++++- 1 file changed, 92 insertions(+), 2 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/rollback.rs b/crates/socket-patch-cli/src/commands/rollback.rs index b901b54e3..fce371f03 100644 --- a/crates/socket-patch-cli/src/commands/rollback.rs +++ b/crates/socket-patch-cli/src/commands/rollback.rs @@ -2830,7 +2830,9 @@ pub(crate) fn superseded_by_hosted( /// record a live hosted pin superseded ([`superseded_by_hosted`]), when it /// failed before writing anything because the installed copy holds bytes /// that are neither side of the record (the superseding patch's, after a -/// reinstall) or lacks a file. The hosted leg's lock restore and the +/// reinstall), lacks a file, or is a JVM copy this record never patched +/// (`gradle_rollback_hash_mismatch`, `jvm_jar_backup_missing`). The hosted +/// leg's lock restore and the /// reinstall it asks for unwind that copy; restoring the record's original /// bytes over the superseding patch's would only mix the two. `None` for /// any other result — a copy still holding the record's patched bytes is @@ -2851,10 +2853,19 @@ fn superseded_record_skip( { return None; } + // JVM copies are refused before verification: a Gradle hash directory + // the record's before-blob does not hash to is not the download this + // record patched (the superseding patch's jar lands in its own hash + // directory), and a swapped jar with no backup here was not swapped by + // this record. Neither refusal writes anything. let replaced = result .files_verified .iter() - .any(|v| v.status == VerifyRollbackStatus::HashMismatch || v.is_absent()); + .any(|v| v.status == VerifyRollbackStatus::HashMismatch || v.is_absent()) + || result.error.as_deref().is_some_and(|e| { + e.starts_with("gradle_rollback_hash_mismatch") + || e.starts_with("jvm_jar_backup_missing") + }); replaced.then(|| { ( "rollback_record_superseded".to_string(), @@ -3348,6 +3359,85 @@ mod tests { } } + fn superseded_map() -> HashMap { + HashMap::from([( + "pkg:npm/foo@1.0.0".to_string(), + "bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb".to_string(), + )]) + } + + #[test] + fn superseded_skip_covers_a_copy_holding_neither_side() { + let target = CopyTarget::plain("pkg:npm/foo@1.0.0", Path::new("/tmp/foo")); + let result = make_result(&[VerifyRollbackStatus::HashMismatch], &[]); + let (code, detail) = superseded_record_skip(&target, &result, &superseded_map()) + .expect("a superseded record's mismatched copy is left to the hosted leg"); + assert_eq!(code, "rollback_record_superseded"); + assert!( + detail.contains("bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb"), + "{detail}" + ); + } + + #[test] + fn superseded_skip_covers_jvm_copies_the_record_never_patched() { + let target = CopyTarget::plain("pkg:npm/foo@1.0.0", Path::new("/tmp/foo")); + for error in [ + "gradle_rollback_hash_mismatch: the before-blob for x does not hash", + "jvm_jar_backup_missing: no backup of the original jar", + ] { + let mut result = make_result(&[], &[]); + result.success = false; + result.error = Some(error.to_string()); + assert!( + superseded_record_skip(&target, &result, &superseded_map()).is_some(), + "{error}" + ); + } + } + + #[test] + fn superseded_skip_leaves_other_failures_and_records_alone() { + let target = CopyTarget::plain("pkg:npm/foo@1.0.0", Path::new("/tmp/foo")); + let mismatch = make_result(&[VerifyRollbackStatus::HashMismatch], &[]); + // Not superseded: the mismatch fails as before. + assert!(superseded_record_skip(&target, &mismatch, &HashMap::new()).is_none()); + // A missing before-blob is a real failure even when superseded. + let missing = make_result(&[VerifyRollbackStatus::MissingBlob], &[]); + assert!(superseded_record_skip(&target, &missing, &superseded_map()).is_none()); + // A file that exists but cannot be read may still hold A's bytes. + let mut unreadable = make_result(&[VerifyRollbackStatus::NotFound], &[]); + unreadable.files_verified[0].message = Some("Failed to hash file: EACCES".to_string()); + assert!(superseded_record_skip(&target, &unreadable, &superseded_map()).is_none()); + } + + #[test] + fn superseded_by_hosted_needs_a_pin_with_another_uuid() { + let mut manifest = PatchManifest::new(); + manifest + .patches + .insert("pkg:npm/foo@1.0.0".to_string(), make_record("aaaa")); + manifest + .patches + .insert("pkg:npm/bar@2.0.0".to_string(), make_record("cccc")); + let pin = |purl: &str, uuid: &str| HostedPin { + purl: purl.to_string(), + uuid: uuid.to_string(), + files: vec!["package-lock.json".to_string()], + }; + let pins = vec![ + pin("pkg:npm/foo@1.0.0", "bbbb"), + // bar's hosted pin carries the record's own uuid: not superseded. + pin("pkg:npm/bar@2.0.0", "cccc"), + ]; + let map = superseded_by_hosted(&manifest, &pins); + assert_eq!( + map, + HashMap::from([("pkg:npm/foo@1.0.0".to_string(), "bbbb".to_string())]) + ); + assert!(superseded_by_hosted(&manifest, &[]).is_empty()); + } + #[test] fn all_files_already_original_true_when_every_file_matches() { let r = make_result( From 1494f429bf956332fab3515193c1d451bf06a849 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 13:34:37 +0000 Subject: [PATCH 6/7] Keep JVM copies that still hold the old patch jvm_jar_backup_missing is only raised once every jar member verifies as the old record's patched bytes, so treating it as superseded would drop the record while the patched jar stays in the cache. It now fails as before. A gradle_rollback_hash_mismatch copy is left to the hosted leg only when none of the record's files there still hash to its patched bytes; a corrupt before-blob for the directory the record did patch fails as before. Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-cli/src/commands/rollback.rs | 171 ++++++++++++++---- 1 file changed, 132 insertions(+), 39 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/rollback.rs b/crates/socket-patch-cli/src/commands/rollback.rs index fce371f03..400de9a1a 100644 --- a/crates/socket-patch-cli/src/commands/rollback.rs +++ b/crates/socket-patch-cli/src/commands/rollback.rs @@ -2658,7 +2658,8 @@ pub(crate) async fn rollback_patches_inner( warnings.push(warning); continue; } - if let Some(warning) = superseded_record_skip(target, &result, superseded) { + let files = target.files.as_ref().unwrap_or(&patch.files); + if let Some(warning) = superseded_record_skip(target, &result, files, superseded).await { warnings.push(warning); superseded_left.push(purl.clone()); continue; @@ -2830,16 +2831,18 @@ pub(crate) fn superseded_by_hosted( /// record a live hosted pin superseded ([`superseded_by_hosted`]), when it /// failed before writing anything because the installed copy holds bytes /// that are neither side of the record (the superseding patch's, after a -/// reinstall), lacks a file, or is a JVM copy this record never patched -/// (`gradle_rollback_hash_mismatch`, `jvm_jar_backup_missing`). The hosted -/// leg's lock restore and the -/// reinstall it asks for unwind that copy; restoring the record's original -/// bytes over the superseding patch's would only mix the two. `None` for -/// any other result — a copy still holding the record's patched bytes is -/// restored as usual, and a file that cannot be read fails as usual. -fn superseded_record_skip( +/// reinstall), lacks a file, or is a Gradle hash directory this record +/// never patched (`gradle_rollback_hash_mismatch` with no file at the +/// record's patched bytes). The hosted leg's lock restore and the reinstall +/// it asks for unwind that copy; restoring the record's original bytes over +/// the superseding patch's would only mix the two. `None` for any other +/// result: a copy still holding the record's patched bytes (including a +/// swapped jar with no backup, `jvm_jar_backup_missing`) is restored or +/// fails as usual, and so does a file that cannot be read. +async fn superseded_record_skip( target: &CopyTarget, result: &RollbackResult, + files: &HashMap, superseded: &HashMap, ) -> Option<(String, String)> { let wired = superseded.get(&target.purl)?; @@ -2853,20 +2856,22 @@ fn superseded_record_skip( { return None; } - // JVM copies are refused before verification: a Gradle hash directory - // the record's before-blob does not hash to is not the download this - // record patched (the superseding patch's jar lands in its own hash - // directory), and a swapped jar with no backup here was not swapped by - // this record. Neither refusal writes anything. - let replaced = result + let mismatched = result .files_verified .iter() - .any(|v| v.status == VerifyRollbackStatus::HashMismatch || v.is_absent()) - || result.error.as_deref().is_some_and(|e| { - e.starts_with("gradle_rollback_hash_mismatch") - || e.starts_with("jvm_jar_backup_missing") - }); - replaced.then(|| { + .any(|v| v.status == VerifyRollbackStatus::HashMismatch || v.is_absent()); + // A Gradle hash directory is refused before verification when the + // record's before-blob does not hash to its name: normally the + // superseding patch's own download, which this record never patched. + // It is left only if no file there still holds the record's patched + // bytes (a corrupt blob for the directory the record DID patch fails + // as usual). + let foreign_gradle_dir = result + .error + .as_deref() + .is_some_and(|e| e.starts_with("gradle_rollback_hash_mismatch")) + && !holds_patched_bytes(target, files).await; + (mismatched || foreign_gradle_dir).then(|| { ( "rollback_record_superseded".to_string(), format!( @@ -2880,6 +2885,35 @@ fn superseded_record_skip( }) } +/// Whether any of `files` in `target`'s copy is at the record's patched +/// bytes, or cannot be checked (an unsafe key, a read error other than +/// "not found"), which may hide them. +async fn holds_patched_bytes(target: &CopyTarget, files: &HashMap) -> bool { + for (file, info) in files { + let key = maven_target_key(&target.purl, &target.dir, file); + let rel = Path::new(key.strip_prefix("package/").unwrap_or(&key)); + if rel.as_os_str().is_empty() + || !rel + .components() + .all(|c| matches!(c, std::path::Component::Normal(_))) + { + return true; + } + match tokio::fs::read(target.dir.join(rel)).await { + Ok(bytes) => { + if socket_patch_core::hash::git_sha256::compute_git_sha256_from_bytes(&bytes) + == info.after_hash + { + return true; + } + } + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(_) => return true, + } + } + false +} + /// The key `file` of a Maven record as it is joined onto `dir`: a Gradle /// hash dir holds the bare file name (`package/` dropped). fn maven_target_key(purl: &str, dir: &Path, file: &str) -> String { @@ -3366,11 +3400,45 @@ mod tests { )]) } - #[test] - fn superseded_skip_covers_a_copy_holding_neither_side() { - let target = CopyTarget::plain("pkg:npm/foo@1.0.0", Path::new("/tmp/foo")); + /// A record of one file, `package/index.js`, patched from `before` to + /// `after`, and a copy dir holding `installed` as that file. + fn superseded_copy( + installed: &[u8], + ) -> ( + tempfile::TempDir, + CopyTarget, + HashMap, + ) { + let tmp = tempfile::tempdir().unwrap(); + std::fs::write(tmp.path().join("index.js"), installed).unwrap(); + let files = HashMap::from([( + "package/index.js".to_string(), + PatchFileInfo { + before_hash: socket_patch_core::hash::git_sha256::compute_git_sha256_from_bytes( + b"original", + ), + after_hash: socket_patch_core::hash::git_sha256::compute_git_sha256_from_bytes( + b"patched by A", + ), + }, + )]); + let target = CopyTarget::plain("pkg:npm/foo@1.0.0", tmp.path()); + (tmp, target, files) + } + + fn refused(error: &str) -> RollbackResult { + let mut result = make_result(&[], &[]); + result.success = false; + result.error = Some(error.to_string()); + result + } + + #[tokio::test] + async fn superseded_skip_covers_a_copy_holding_neither_side() { + let (tmp, target, files) = superseded_copy(b"patched by B"); let result = make_result(&[VerifyRollbackStatus::HashMismatch], &[]); - let (code, detail) = superseded_record_skip(&target, &result, &superseded_map()) + let (code, detail) = superseded_record_skip(&target, &result, &files, &superseded_map()) + .await .expect("a superseded record's mismatched copy is left to the hosted leg"); assert_eq!(code, "rollback_record_superseded"); assert!( @@ -3379,36 +3447,61 @@ mod tests { ); } - #[test] - fn superseded_skip_covers_jvm_copies_the_record_never_patched() { - let target = CopyTarget::plain("pkg:npm/foo@1.0.0", Path::new("/tmp/foo")); + #[tokio::test] + async fn superseded_skip_covers_a_gradle_dir_the_record_never_patched() { + let (tmp, target, files) = superseded_copy(b"patched by B"); + let result = refused("gradle_rollback_hash_mismatch: the before-blob for x does not hash"); + assert!( + superseded_record_skip(&target, &result, &files, &superseded_map()) + .await + .is_some() + ); + } + + #[tokio::test] + async fn superseded_skip_keeps_jvm_copies_holding_the_patched_bytes() { + // The record's own patched bytes are still there: a corrupt blob for + // the directory it patched, or a swapped jar with no backup, fails. + let (tmp, target, files) = superseded_copy(b"patched by A"); for error in [ "gradle_rollback_hash_mismatch: the before-blob for x does not hash", - "jvm_jar_backup_missing: no backup of the original jar", + "jvm_jar_backup_missing: no original of lib-1.0.jar", ] { - let mut result = make_result(&[], &[]); - result.success = false; - result.error = Some(error.to_string()); + let result = refused(error); assert!( - superseded_record_skip(&target, &result, &superseded_map()).is_some(), + superseded_record_skip(&target, &result, &files, &superseded_map()) + .await + .is_none(), "{error}" ); } } - #[test] - fn superseded_skip_leaves_other_failures_and_records_alone() { - let target = CopyTarget::plain("pkg:npm/foo@1.0.0", Path::new("/tmp/foo")); + #[tokio::test] + async fn superseded_skip_leaves_other_failures_and_records_alone() { + let (_tmp, target, files) = superseded_copy(b"patched by B"); let mismatch = make_result(&[VerifyRollbackStatus::HashMismatch], &[]); // Not superseded: the mismatch fails as before. - assert!(superseded_record_skip(&target, &mismatch, &HashMap::new()).is_none()); + assert!( + superseded_record_skip(&target, &mismatch, &files, &HashMap::new()) + .await + .is_none() + ); // A missing before-blob is a real failure even when superseded. let missing = make_result(&[VerifyRollbackStatus::MissingBlob], &[]); - assert!(superseded_record_skip(&target, &missing, &superseded_map()).is_none()); + assert!( + superseded_record_skip(&target, &missing, &files, &superseded_map()) + .await + .is_none() + ); // A file that exists but cannot be read may still hold A's bytes. let mut unreadable = make_result(&[VerifyRollbackStatus::NotFound], &[]); unreadable.files_verified[0].message = Some("Failed to hash file: EACCES".to_string()); - assert!(superseded_record_skip(&target, &unreadable, &superseded_map()).is_none()); + assert!( + superseded_record_skip(&target, &unreadable, &files, &superseded_map()) + .await + .is_none() + ); } #[test] From ec6ca8596cd0d8306fdfb581fbf2d0853e698553 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 13:39:10 +0000 Subject: [PATCH 7/7] Read Gradle copies through the FIFO-safe opener The superseded-copy probe read installed files with a bare read, so a FIFO or device planted in a Gradle cache directory would block rollback and remove forever. It now uses read_regular_to_bytes like the neighbouring Gradle checks; a non-regular file is refused and counts as possibly patched, so the copy fails as before. Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-cli/src/commands/rollback.rs | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/crates/socket-patch-cli/src/commands/rollback.rs b/crates/socket-patch-cli/src/commands/rollback.rs index 400de9a1a..ab654edb6 100644 --- a/crates/socket-patch-cli/src/commands/rollback.rs +++ b/crates/socket-patch-cli/src/commands/rollback.rs @@ -2899,7 +2899,10 @@ async fn holds_patched_bytes(target: &CopyTarget, files: &HashMap { if socket_patch_core::hash::git_sha256::compute_git_sha256_from_bytes(&bytes) == info.after_hash @@ -3477,6 +3480,28 @@ mod tests { } } + #[cfg(unix)] + #[tokio::test] + async fn superseded_skip_never_blocks_on_a_fifo() { + // A FIFO where the record's file should be is unverifiable: the + // check must refuse it, not block in open(2), and must not skip. + let (tmp, target, files) = superseded_copy(b"patched by B"); + std::fs::remove_file(tmp.path().join("index.js")).unwrap(); + let made = std::process::Command::new("mkfifo") + .arg(tmp.path().join("index.js")) + .status() + .expect("run mkfifo"); + assert!(made.success()); + let result = refused("gradle_rollback_hash_mismatch: the before-blob for x does not hash"); + let skip = tokio::time::timeout( + std::time::Duration::from_secs(10), + superseded_record_skip(&target, &result, &files, &superseded_map()), + ) + .await + .expect("the patched-bytes probe must not block on a FIFO"); + assert!(skip.is_none()); + } + #[tokio::test] async fn superseded_skip_leaves_other_failures_and_records_alone() { let (_tmp, target, files) = superseded_copy(b"patched by B");