From a21e4d06b4483eae29d2b63c10f7563e2df19c23 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 00:57:59 +0000 Subject: [PATCH 1/3] Start sweep of scan/hosted nuget and takeover Assisted-by: Claude Code:claude-opus-5-5 From a7a87a3f9958192e749ba01146d8bb1c4127177c Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 01:04:12 +0000 Subject: [PATCH 2/3] Inline takeover's single-caller helpers Inline revert_keeps_wiring, residual_takeover_warning, drifted_takeover_warning and unwires_alias_copy into their only callers in Takeover::stage and Takeover::unpinned. The residual-ref check now uses vendor::RESIDUAL_REFERENCE_CODE instead of a copy of the literal, KEPT_VENDORED is private, and the cargo probe filters purl_parts by type instead of a starts_with + then + flatten chain. Bare issue references are dropped from comments here and in nuget.rs. Output bytes, codes and skip reasons are unchanged. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/commands/scan/hosted/nuget.rs | 3 +- .../src/commands/scan/hosted/takeover.rs | 157 +++++++----------- 2 files changed, 64 insertions(+), 96 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/scan/hosted/nuget.rs b/crates/socket-patch-cli/src/commands/scan/hosted/nuget.rs index 7527bcab1..b85f8844d 100644 --- a/crates/socket-patch-cli/src/commands/scan/hosted/nuget.rs +++ b/crates/socket-patch-cli/src/commands/scan/hosted/nuget.rs @@ -1,5 +1,4 @@ -//! Read-only check of NuGet's global packages folder for hosted redirects -//! (#352). +//! Read-only check of NuGet's global packages folder for hosted redirects. //! //! A hosted NuGet patch keeps the upstream id and version, and NuGet //! restores a package already extracted into its global packages folder diff --git a/crates/socket-patch-cli/src/commands/scan/hosted/takeover.rs b/crates/socket-patch-cli/src/commands/scan/hosted/takeover.rs index d7410f5a9..02c43db58 100644 --- a/crates/socket-patch-cli/src/commands/scan/hosted/takeover.rs +++ b/crates/socket-patch-cli/src/commands/scan/hosted/takeover.rs @@ -43,11 +43,11 @@ use socket_patch_core::patch::redirect::RewriteWarning; use socket_patch_core::utils::group_commit::{GroupCommit, Savepoint}; use socket_patch_core::utils::purl::{purl_parts, strip_purl_qualifiers}; use socket_patch_core::utils::purl_key::PurlKey; -use socket_patch_core::vendor::{RevertOutcome, VendorEntry, VendorState}; +use socket_patch_core::vendor::{RevertOutcome, VendorEntry, VendorState, RESIDUAL_REFERENCE_CODE}; /// The warning that announces a staged takeover the run did not complete: /// the purl stays vendored. -pub(super) const KEPT_VENDORED: &str = "redirect_takeover_kept_vendored"; +const KEPT_VENDORED: &str = "redirect_takeover_kept_vendored"; /// The skip reason of a staged takeover whose purl the hosted rewrite did /// not pin, when no rewriter warning names the cause. @@ -138,10 +138,8 @@ impl Takeover { // npm purl in this state falls through to the rewriters' own // per-flavor diagnostics.) None => { - let Some((_, name, version)) = purl - .starts_with("pkg:cargo/") - .then(|| purl_parts(purl)) - .flatten() + let Some((_, name, version)) = + purl_parts(purl).filter(|(ty, _, _)| ty == "cargo") else { continue; }; @@ -172,14 +170,14 @@ impl Takeover { } } } - // NON-UTF-8 PRE-CHECK (#721) — the GUARD's undecodable-file rule + // NON-UTF-8 PRE-CHECK — the GUARD's undecodable-file rule // (`engine::undecodable_guard`), checked before any revert is // staged (and under --dry-run too), on the project as it is on disk. if !out.attempts.is_empty() { let view = socket_patch_core::vendor::lock_inventory::ProjectView::Disk(&common.cwd); let read = socket_patch_core::hosted::engine::read_candidate_files( &view, - &std::collections::BTreeSet::new(), + &BTreeSet::new(), candidates, ) .await; @@ -257,15 +255,46 @@ impl Takeover { } else if let Some(residual) = residual_only_keep(&outcome) { // Nothing drifted: another project file (an exported // requirements file) still installs from the vendored - // wheel (#1184). Name it, not a drift. - Some(residual_takeover_warning(&purl, &residual)) - } else if revert_keeps_wiring(&outcome) { + // wheel. Name it, not a drift. + let pin = purl + .strip_prefix("pkg:pypi/") + .map(|rest| rest.split(['?', '#']).next().unwrap_or(rest)) + .and_then(|rest| rest.split_once('@')) + .map_or_else( + || "the registry release".to_string(), + |(name, version)| format!("`{name}=={version}`"), + ); + Some(serde_json::json!({ + "code": "redirect_vendored_revert_failed", + "detail": format!( + "{purl} is vendored and another project file still installs from its \ + vendored wheel ({residual}), so it is left in place; NOT switched to hosted — \ + first replace the wheel path in that file with {pin} by hand (the lock is \ + still vendored, so a re-export now would name the wheel again), re-run \ + `scan --mode hosted`, and only then re-export the file from the rewired lock" + ), + })) + } else if outcome.kept_artifact + || outcome.drift_skipped() + || outcome + .warnings + .iter() + .any(|w| w.code == RESIDUAL_REFERENCE_CODE) + { // A wiring record drifted and was left in place, so the // project may still resolve through the vendored artifact // and the ledger entry holds the only recorded originals // (the RevertOutcome contract): keep both and refuse, // exactly as `vendor --revert` reports it skipped. - Some(drifted_takeover_warning(&purl)) + Some(serde_json::json!({ + "code": "redirect_vendored_revert_failed", + "detail": format!( + "{purl} is vendored and part of its vendored wiring was edited since \ + vendoring, so it is left in place; NOT switched to hosted — restore or \ + remove that wiring (`socket-patch vendor --revert` lists it), then re-run \ + `scan --mode hosted`" + ), + })) } else { None }; @@ -299,8 +328,11 @@ impl Takeover { /// `npm:` alias entry the vendored wiring had repointed, which the /// hosted rewrite leaves on the registry (`alias_skipped`, the /// rewrite's `RewriteResult::alias_skipped_entries`). Committing that - /// takeover would un-patch the alias copy (#1158), so it is retracted - /// like a purl with no pin at all. + /// takeover would un-patch the alias copy, so it is retracted like a + /// purl with no pin at all. Both sides key an entry by its lock key + /// line as `formats::yarn::blocks` reads it; an alias entry the vendored + /// wiring never touched was unpatched before the run, so it does not + /// count. pub(super) fn unpinned( &self, confirmed: &[(String, String)], @@ -310,7 +342,14 @@ impl Takeover { .iter() .filter(|s| { !confirmed.iter().any(|(_, uuid)| *uuid == s.uuid) - || unwires_alias_copy(&s.entry, alias_skipped.get(&s.uuid)) + || alias_skipped.get(&s.uuid).is_some_and(|skipped| { + s.entry.wiring.iter().any(|w| { + std::path::Path::new(&w.file) + .file_name() + .is_some_and(|f| f == "yarn.lock") + && w.key.as_ref().is_some_and(|k| skipped.contains(k)) + }) + }) }) .map(|s| s.uuid.clone()) .collect() @@ -435,11 +474,11 @@ const LANDED_PIN_ADVISORIES: &[&str] = &[ /// Why the rewrite did not pin a staged purl: the skip reason, and the /// rewriter warnings that say so. In order: for a PyPI entry, the reach the /// hosted rewriter lacks (`preflight_pypi_takeover`: a requirements entry -/// wired outside the root file #699, a uv entry pinned down from another -/// locked version #723, a Poetry 0.x lock #945); a rewriter warning naming -/// the package; the rewrite's first warning (a lock-level refusal names no -/// package), skipping [`LANDED_PIN_ADVISORIES`] and the uv < 0.5.6 -/// advisories (#1569); else [`NOT_PINNED`]. +/// wired outside the root file, a uv entry pinned down from another locked +/// version, a Poetry 0.x lock); a rewriter warning naming the package; the +/// rewrite's first warning (a lock-level refusal names no package), +/// skipping [`LANDED_PIN_ADVISORIES`] and the uv < 0.5.6 advisories; else +/// [`NOT_PINNED`]. async fn explain( root: &std::path::Path, entry: &VendorEntry, @@ -451,7 +490,7 @@ async fn explain( } // The uv < 0.5.6 advisories ride on a pin that landed, and their // wording ("wheel", "uv", "sync") can read as another package's name, - // so neither pass may pick one (#1569). + // so neither pass may pick one. let candidates = || { warnings.iter().filter(|w| { !socket_patch_core::patch::redirect::UV_OLD_RELEASE_ADVISORIES @@ -470,24 +509,6 @@ async fn explain( } } -/// Whether `entry`'s vendored wiring repointed a yarn.lock entry among -/// `skipped`, the alias entries the hosted rewrite left untouched for its -/// uuid. Both sides key an entry by its lock key line as the shared -/// `formats::yarn::blocks` scanner reads it. An alias entry the vendored wiring -/// never touched (yarn berry vendoring skips aliases too) was unpatched -/// before the run, so the takeover does not change it. -fn unwires_alias_copy(entry: &VendorEntry, skipped: Option<&BTreeSet>) -> bool { - let Some(skipped) = skipped else { - return false; - }; - entry.wiring.iter().any(|w| { - std::path::Path::new(&w.file) - .file_name() - .is_some_and(|f| f == "yarn.lock") - && w.key.as_ref().is_some_and(|k| skipped.contains(k)) - }) -} - /// The [`KEPT_VENDORED`] warning for `purl`, naming the `code` that kept it. fn kept_vendored(purl: &str, code: &str) -> serde_json::Value { serde_json::json!({ @@ -509,7 +530,7 @@ fn is_berry_entry(entry: &VendorEntry) -> bool { /// it is BEFORE any yarn-berry entry's revert. The revert re-renders /// `package.json` in its majority line ending, so a mixed manifest would /// pass the hosted rewriter's own check afterwards, while both modes refuse -/// to rewrite a mixed one (#628): the takeover keeps such a package +/// to rewrite a mixed one: the takeover keeps such a package /// vendored. The gate itself is the rewriter's /// (`preflight_yarn_berry_hosted`, over the shared `berry_gates`). async fn berry_project_gate(common: &crate::args::GlobalArgs) -> Option { @@ -554,17 +575,6 @@ fn names_package(detail: &str, name: &str) -> bool { }) } -/// Whether a takeover revert left vendored wiring in place: a drift-skipped -/// record, or a reverted file that still references the artifact dir. -fn revert_keeps_wiring(outcome: &RevertOutcome) -> bool { - outcome.kept_artifact - || outcome.drift_skipped() - || outcome - .warnings - .iter() - .any(|w| w.code == "vendor_revert_residual_reference") -} - /// The `vendor_revert_residual_reference` detail of a revert whose ONLY /// keep signal is a file that still references the artifact. fn residual_only_keep(outcome: &RevertOutcome) -> Option { @@ -574,7 +584,7 @@ fn residual_only_keep(outcome: &RevertOutcome) -> Option { outcome .warnings .iter() - .find(|w| w.code == socket_patch_core::vendor::RESIDUAL_REFERENCE_CODE) + .find(|w| w.code == RESIDUAL_REFERENCE_CODE) .map(|w| { // `kept : still resolves through it, and deleting …`: // only the clause naming the file; the revert's own remedy is @@ -590,47 +600,6 @@ fn residual_only_keep(outcome: &RevertOutcome) -> Option { }) } -/// The refusal for a takeover whose vendored wheel another project file -/// still installs from. -/// -/// The order matters: the refusal leaves the lock vendored, so re-exporting -/// from it now would name the wheel again. The file is pinned by hand -/// first; only after the hosted scan has rewired the lock is a re-export -/// safe. -fn residual_takeover_warning(purl: &str, residual: &str) -> serde_json::Value { - let pin = purl - .strip_prefix("pkg:pypi/") - .map(|rest| rest.split(['?', '#']).next().unwrap_or(rest)) - .and_then(|rest| rest.split_once('@')) - .map_or_else( - || "the registry release".to_string(), - |(name, version)| format!("`{name}=={version}`"), - ); - serde_json::json!({ - "code": "redirect_vendored_revert_failed", - "detail": format!( - "{purl} is vendored and another project file still installs from its \ - vendored wheel ({residual}), so it is left in place; NOT switched to hosted — \ - first replace the wheel path in that file with {pin} by hand (the lock is \ - still vendored, so a re-export now would name the wheel again), re-run \ - `scan --mode hosted`, and only then re-export the file from the rewired lock" - ), - }) -} - -/// The refusal for a takeover whose vendored wiring drifted since vendoring. -fn drifted_takeover_warning(purl: &str) -> serde_json::Value { - serde_json::json!({ - "code": "redirect_vendored_revert_failed", - "detail": format!( - "{purl} is vendored and part of its vendored wiring was edited since \ - vendoring, so it is left in place; NOT switched to hosted — restore or \ - remove that wiring (`socket-patch vendor --revert` lists it), then re-run \ - `scan --mode hosted`" - ), - }) -} - #[cfg(test)] mod tests { use super::*; @@ -680,7 +649,7 @@ mod tests { assert_eq!(explained, vec![warn("redirect_lock_refused")]); } - /// #1569: another pin's uv advisory says "wheel" and "uv sync", whole + /// Another pin's uv advisory says "wheel" and "uv sync", whole /// words that are also package names; it never explains a staged /// package that was not pinned, by name or as the lock-wide fallback. #[tokio::test] From 3c9459acd531302fcfc6b55219c9dab7770c7a12 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 01:10:54 +0000 Subject: [PATCH 3/3] Apply review fixes to the takeover sweep Drop the residual-reference conjunct from the keep-wiring branch: it is reached only when residual_only_keep returned None, so a residual warning there implies drift_skipped, which the branch already checks. Fix that branch's comment to cover kept_artifact, keep the yarn alias rule as the named unwires_alias_copy predicate, and name the repeated refusal code REVERT_FAILED. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/commands/scan/hosted/takeover.rs | 59 ++++++++++--------- 1 file changed, 31 insertions(+), 28 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/scan/hosted/takeover.rs b/crates/socket-patch-cli/src/commands/scan/hosted/takeover.rs index 02c43db58..c96226437 100644 --- a/crates/socket-patch-cli/src/commands/scan/hosted/takeover.rs +++ b/crates/socket-patch-cli/src/commands/scan/hosted/takeover.rs @@ -49,6 +49,10 @@ use socket_patch_core::vendor::{RevertOutcome, VendorEntry, VendorState, RESIDUA /// the purl stays vendored. const KEPT_VENDORED: &str = "redirect_takeover_kept_vendored"; +/// The warning code of a takeover refused because its revert failed or +/// would keep vendored wiring. +const REVERT_FAILED: &str = "redirect_vendored_revert_failed"; + /// The skip reason of a staged takeover whose purl the hosted rewrite did /// not pin, when no rewriter warning names the cause. const NOT_PINNED: &str = "redirect_takeover_not_pinned"; @@ -157,7 +161,7 @@ impl Takeover { "vendored_revert_failed", )); out.warnings.push(serde_json::json!({ - "code": "redirect_vendored_revert_failed", + "code": REVERT_FAILED, "detail": format!( "{purl} has socket-owned vendored `[patch.crates-io]` \ wiring but no usable vendored ledger entry \ @@ -244,7 +248,7 @@ impl Takeover { crate::commands::vendor::dispatch_revert_one(&entry, &common.cwd, false).await; let refusal = if !outcome.success { Some(serde_json::json!({ - "code": "redirect_vendored_revert_failed", + "code": REVERT_FAILED, "detail": format!( "{purl} is vendored and its vendored state could not be \ reverted ({}); NOT switched to hosted — run `socket-patch vendor \ @@ -265,7 +269,7 @@ impl Takeover { |(name, version)| format!("`{name}=={version}`"), ); Some(serde_json::json!({ - "code": "redirect_vendored_revert_failed", + "code": REVERT_FAILED, "detail": format!( "{purl} is vendored and another project file still installs from its \ vendored wheel ({residual}), so it is left in place; NOT switched to hosted — \ @@ -274,20 +278,14 @@ impl Takeover { `scan --mode hosted`, and only then re-export the file from the rewired lock" ), })) - } else if outcome.kept_artifact - || outcome.drift_skipped() - || outcome - .warnings - .iter() - .any(|w| w.code == RESIDUAL_REFERENCE_CODE) - { - // A wiring record drifted and was left in place, so the - // project may still resolve through the vendored artifact - // and the ledger entry holds the only recorded originals - // (the RevertOutcome contract): keep both and refuse, - // exactly as `vendor --revert` reports it skipped. + } else if outcome.kept_artifact || outcome.drift_skipped() { + // A wiring record drifted or the revert kept the artifact, + // so the project may still resolve through it and the + // ledger entry holds the only recorded originals (the + // RevertOutcome contract): keep both and refuse, exactly as + // `vendor --revert` reports it skipped. Some(serde_json::json!({ - "code": "redirect_vendored_revert_failed", + "code": REVERT_FAILED, "detail": format!( "{purl} is vendored and part of its vendored wiring was edited since \ vendoring, so it is left in place; NOT switched to hosted — restore or \ @@ -329,10 +327,7 @@ impl Takeover { /// hosted rewrite leaves on the registry (`alias_skipped`, the /// rewrite's `RewriteResult::alias_skipped_entries`). Committing that /// takeover would un-patch the alias copy, so it is retracted like a - /// purl with no pin at all. Both sides key an entry by its lock key - /// line as `formats::yarn::blocks` reads it; an alias entry the vendored - /// wiring never touched was unpatched before the run, so it does not - /// count. + /// purl with no pin at all. pub(super) fn unpinned( &self, confirmed: &[(String, String)], @@ -342,14 +337,9 @@ impl Takeover { .iter() .filter(|s| { !confirmed.iter().any(|(_, uuid)| *uuid == s.uuid) - || alias_skipped.get(&s.uuid).is_some_and(|skipped| { - s.entry.wiring.iter().any(|w| { - std::path::Path::new(&w.file) - .file_name() - .is_some_and(|f| f == "yarn.lock") - && w.key.as_ref().is_some_and(|k| skipped.contains(k)) - }) - }) + || alias_skipped + .get(&s.uuid) + .is_some_and(|skipped| unwires_alias_copy(&s.entry, skipped)) }) .map(|s| s.uuid.clone()) .collect() @@ -509,6 +499,19 @@ async fn explain( } } +/// Whether `entry`'s vendored wiring repointed a yarn.lock entry among +/// `skipped`. Both sides key an entry by its lock key line as +/// `formats::yarn::blocks` reads it; an alias entry the vendored wiring +/// never touched was unpatched before the run, so it does not count. +fn unwires_alias_copy(entry: &VendorEntry, skipped: &BTreeSet) -> bool { + entry.wiring.iter().any(|w| { + std::path::Path::new(&w.file) + .file_name() + .is_some_and(|f| f == "yarn.lock") + && w.key.as_ref().is_some_and(|k| skipped.contains(k)) + }) +} + /// The [`KEPT_VENDORED`] warning for `purl`, naming the `code` that kept it. fn kept_vendored(purl: &str, code: &str) -> serde_json::Value { serde_json::json!({