From 32f7b994fe769d71bd098d59903a86b1cd005753 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 30 Oct 2026 04:24:07 +0000 Subject: [PATCH 1/4] Start fix for #1878 Assisted-by: Claude Code:claude-opus-5-5 From 1e057b9fe95fa4b161cfd7c48baa42aceb34de70 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 30 Oct 2026 04:33:56 +0000 Subject: [PATCH 2/4] Word the Pipenv stale-install remedy once Hosted scan and vendored pypi each told the user how to reinstall a release Pipenv kept installed, and the copies drifted: hosted asked for `socket-patch vex --product ` with the placeholder left in, hand-split the purl and fell back to a literal ``. Both now call core's pypi_pipenv::stale_install_detail, which sorts the stale sites into the three classes (inherited base site, a venv copy hiding the base one, the venv's own copy) and words each remedy. Vendored text is unchanged; hosted keeps its warning code and lead sentence, names the package via parse_pypi_purl, and ends with the runnable `socket-patch vex` that vendored already used. `--product` takes the project's product purl, not the patched package's, so filling in the package purl would have been wrong. Fixes #1878 Assisted-by: Claude Code:claude-opus-5-5 --- .../src/commands/scan/hosted/python.rs | 123 +++++-------- crates/socket-patch-core/src/utils/purl.rs | 2 +- crates/socket-patch-core/src/vendor/pypi.rs | 67 ++----- .../src/vendor/pypi_pipenv.rs | 163 +++++++++++++++++- 4 files changed, 218 insertions(+), 137 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/scan/hosted/python.rs b/crates/socket-patch-cli/src/commands/scan/hosted/python.rs index 70c097e46..9a22b4cfa 100644 --- a/crates/socket-patch-cli/src/commands/scan/hosted/python.rs +++ b/crates/socket-patch-cli/src/commands/scan/hosted/python.rs @@ -4,7 +4,7 @@ use std::collections::{BTreeMap, BTreeSet}; use socket_patch_core::crawlers::PythonCrawler; use socket_patch_core::manifest::schema::PatchRecord; -use socket_patch_core::utils::purl::strip_purl_qualifiers; +use socket_patch_core::utils::purl::{parse_pypi_purl, strip_purl_qualifiers}; use socket_patch_core::vex::verify::judge_installed_record; use super::StaleInstallOutcome; @@ -145,52 +145,16 @@ pub(super) async fn stale_install_warnings( let remedy = if let Some(env) = hatch_env { socket_patch_core::crawlers::hatch_env::stale_install_remedy(&env.name) } else if pipenv_purls.contains(&purl) { - let name = strip_purl_qualifiers(&purl) - .strip_prefix("pkg:pypi/") - .and_then(|rest| rest.split('@').next()) - .unwrap_or("") - .to_string(); - if inherited.contains(&site) { - let remedy = - socket_patch_core::vendor::pypi_pipenv::base_site_stale_install_remedy( - pipenv_lock.as_ref(), - &name, - &site.display().to_string(), - ); - format!( - "{remedy}; then `socket-patch vex --product ` re-verifies the \ - installed files." - ) - } else if let Some(base) = - socket_patch_core::crawlers::python_crawler::inherited_same_release( - &site, &name, &version, - ) - .await - { - let remedy = - socket_patch_core::vendor::pypi_pipenv::shadowing_stale_install_remedy( - pipenv_lock.as_ref(), - &name, - &site.display().to_string(), - &base.display().to_string(), - ); - format!( - "{remedy}; then `socket-patch vex --product ` re-verifies the \ - installed files." - ) - } else { - let remedy = socket_patch_core::vendor::pypi_pipenv::stale_install_remedy( - pipenv_lock.as_ref(), - &name, - ); - format!( - "Pipenv does not reinstall a release that is already present (`pipenv \ - install`, `pipenv install --deploy` and `pipenv sync` all keep those \ - bytes), so the rewritten Pipfile.lock only protects fresh installs. \ - {remedy}; then `socket-patch vex --product ` re-verifies the \ - installed files." - ) - } + let name = parse_pypi_purl(strip_purl_qualifiers(&purl)) + .map_or_else(|| purl.clone(), |(name, _)| name.to_string()); + socket_patch_core::vendor::pypi_pipenv::stale_install_detail( + pipenv_lock.as_ref(), + &name, + &version, + std::slice::from_ref(&site), + &inherited, + ) + .await } else { "Reinstall from the rewritten lock in this interpreter and verify with \ `socket-patch vex`. Poetry before 1.4 may keep same-version packages: \ @@ -361,12 +325,13 @@ mod tests { assert_eq!(out.warnings[0]["code"], "redirect_pypi_stale_install"); } - /// #790: the Pipenv remedy names the lock category that pins the - /// package. Plain `pipenv sync` installs only `default`, so for a - /// `[dev-packages]` entry it uninstalled the package and left it - /// uninstalled. + /// #1878: hosted words the Pipenv remedy through core's shared + /// `stale_install_detail`, as vendored does: the lock categories pick + /// the `pipenv sync` arguments (core's tests cover each category), and + /// the re-verify step names a runnable command, with no `` or + /// `` placeholder left in. #[tokio::test] - async fn pipenv_remedy_resyncs_the_category_that_pins_the_package() { + async fn pipenv_remedy_is_the_shared_core_wording() { let tmp = tempfile::tempdir().unwrap(); let site = tmp.path().join("site-packages"); std::fs::create_dir_all(site.join("six-1.16.0.dist-info")).unwrap(); @@ -380,42 +345,34 @@ mod tests { let confirmed = vec![(purl.to_string(), "six-uuid".to_string())]; let ledger = BTreeMap::from([("k".into(), record("six-uuid", "six.py", b"patched"))]); let pipenv = BTreeSet::from(["six-uuid".to_string()]); - let detail = |lock: &str| { - let lock = lock.to_string(); - let (common, confirmed, ledger, pipenv) = (&common, &confirmed, &ledger, &pipenv); - async move { - let out = - stale_install_warnings(common, confirmed, pipenv, Some(&lock), ledger).await; - assert_eq!(out.warnings.len(), 1); - out.warnings[0]["detail"].as_str().unwrap().to_string() - } - }; - - let develop = detail( - r#"{"_meta": {"pipfile-spec": 6}, "default": {}, "develop": {"six": {"file": "x"}}}"#, - ) - .await; - assert!( - develop.contains("`pipenv run pip uninstall -y six && pipenv sync --dev`"), - "{develop}" - ); - assert!( - develop.contains("`pipenv --rm && pipenv sync --dev`"), - "{develop}" - ); - - let docs = detail( - r#"{"_meta": {"pipfile-spec": 6}, "default": {"requests": {}}, "docs": {"six": {}}}"#, + let text = + r#"{"_meta": {"pipfile-spec": 6}, "default": {}, "develop": {"six": {"file": "x"}}}"#; + let out = stale_install_warnings(&common, &confirmed, &pipenv, Some(text), &ledger).await; + assert_eq!(out.warnings.len(), 1); + let detail = out.warnings[0]["detail"].as_str().unwrap(); + let lock: serde_json::Value = serde_json::from_str(text).unwrap(); + let shared = socket_patch_core::vendor::pypi_pipenv::stale_install_detail( + Some(&lock), + "six", + "1.16.0", + std::slice::from_ref(&site), + &Default::default(), ) .await; - assert!( - docs.contains("-y six && pipenv sync --categories \"docs\"`"), - "{docs}" + assert_eq!( + detail, + format!( + "{purl} was switched to a hosted patch, but installed files in {} still differ \ + from the patched hashes. {shared} The installed files were left unchanged.", + site.display() + ) ); assert!( - docs.contains("`pipenv --rm && pipenv sync --categories \"packages docs\"`"), - "{docs}" + detail.contains("`pipenv run pip uninstall -y six && pipenv sync --dev`"), + "{detail}" ); + assert!(!detail.contains(""), "{detail}"); + assert!(!detail.contains(""), "{detail}"); } /// #1450: a `--site-packages` venv imports the upstream release from its diff --git a/crates/socket-patch-core/src/utils/purl.rs b/crates/socket-patch-core/src/utils/purl.rs index 08edd82c3..3eaa30961 100644 --- a/crates/socket-patch-core/src/utils/purl.rs +++ b/crates/socket-patch-core/src/utils/purl.rs @@ -237,7 +237,7 @@ fn parse_namespaced<'a>(purl: &'a str, prefix: &str) -> Option `Some(("requests", "2.28.0"))` -pub(crate) fn parse_pypi_purl(purl: &str) -> Option<(Cow<'_, str>, Cow<'_, str>)> { +pub fn parse_pypi_purl(purl: &str) -> Option<(Cow<'_, str>, Cow<'_, str>)> { parse_name_version(purl, "pkg:pypi/") } diff --git a/crates/socket-patch-core/src/vendor/pypi.rs b/crates/socket-patch-core/src/vendor/pypi.rs index 5d6d1c44b..6e5d93988 100644 --- a/crates/socket-patch-core/src/vendor/pypi.rs +++ b/crates/socket-patch-core/src/vendor/pypi.rs @@ -802,61 +802,24 @@ async fn pipenv_stale_install_warning( let (name, version) = parse_pypi_purl(strip_purl_qualifiers(purl)) .map(|(name, version)| (name.to_string(), version.to_string())) .unwrap_or_else(|| (purl.to_string(), String::new())); - let join = |dirs: &[&std::path::PathBuf]| { - dirs.iter() - .map(|d| d.display().to_string()) - .collect::>() - .join(", ") - }; - let listed = join(&stale_dirs.iter().collect::>()); + let listed = stale_dirs + .iter() + .map(|d| d.display().to_string()) + .collect::>() + .join(", "); // A base interpreter's copy a `--site-packages` venv inherits survives // both the uninstall-and-sync and the recreate-and-sync remedies (#1450). let inherited = crate::crawlers::python_crawler::inherited_base_site_dirs(&sites).await; - let (base, own): (Vec<_>, Vec<_>) = stale_dirs.iter().partition(|d| inherited.contains(*d)); - // A venv copy that hides the same release in its base interpreter: the - // uninstall-and-sync remedy would only expose that base copy. - let mut shadowing = Vec::new(); - let mut plain = Vec::new(); - for dir in own { - match crate::crawlers::python_crawler::inherited_same_release(dir, &name, &version).await { - Some(base_dir) => shadowing.push((dir, base_dir)), - None => plain.push(dir), - } - } - let own = plain; - let mut detail = - format!("{purl}: the UNPATCHED upstream release is still installed in {listed}."); - let several = [!own.is_empty(), !shadowing.is_empty(), !base.is_empty()] - .iter() - .filter(|kind| **kind) - .count() - > 1; - if !own.is_empty() { - let remedy = super::pypi_pipenv::stale_install_remedy(Some(lock), &name); - let lead = if !several { - String::new() - } else { - format!("For {}: ", join(&own)) - }; - detail.push_str(&format!( - " {lead}Pipenv does not reinstall a release that is already present (`pipenv install`, `pipenv install --deploy` and `pipenv sync` all keep those bytes), so the wired Pipfile.lock only protects fresh installs. {remedy}." - )); - } - for (dir, base_dir) in &shadowing { - let remedy = super::pypi_pipenv::shadowing_stale_install_remedy( - Some(lock), - &name, - &dir.display().to_string(), - &base_dir.display().to_string(), - ); - detail.push_str(&format!(" {remedy}.")); - } - if !base.is_empty() { - let remedy = - super::pypi_pipenv::base_site_stale_install_remedy(Some(lock), &name, &join(&base)); - detail.push_str(&format!(" {remedy}.")); - } - detail.push_str(" Then `socket-patch vex` re-verifies the installed files."); + let remedy = super::pypi_pipenv::stale_install_detail( + Some(lock), + &name, + &version, + &stale_dirs, + &inherited, + ) + .await; + let detail = + format!("{purl}: the UNPATCHED upstream release is still installed in {listed}. {remedy}"); Some(VendorWarning::new("pypi_pipenv_stale_install", detail)) } diff --git a/crates/socket-patch-core/src/vendor/pypi_pipenv.rs b/crates/socket-patch-core/src/vendor/pypi_pipenv.rs index 872571949..d1f9d3c0b 100644 --- a/crates/socket-patch-core/src/vendor/pypi_pipenv.rs +++ b/crates/socket-patch-core/src/vendor/pypi_pipenv.rs @@ -2,7 +2,8 @@ //! Vendoring requires Pipenv 2018 or later; earlier installers cannot consume //! portable relative wheel paths with integrity hashes. -use std::path::Path; +use std::collections::HashSet; +use std::path::{Path, PathBuf}; use std::sync::Arc; use serde_json::{Map, Value}; @@ -950,6 +951,75 @@ pub fn shadowing_stale_install_remedy( ) } +/// The remedy text for upstream copies of `name` `version` still installed +/// in `sites`, shared by the hosted (`redirect_pypi_stale_install`) and +/// vendored (`pypi_pipenv_stale_install`) warnings. Each site falls in one +/// class: a base interpreter dir in `inherited` that a `--site-packages` +/// virtualenv imports (#1450), a virtualenv copy that hides the same release +/// in its base interpreter, or the virtualenv's own copy. Each class gets +/// its remedy (the own copies led by `For :` when more than one class +/// is present), and the text ends with the command that re-verifies. +pub async fn stale_install_detail( + lock: Option<&Value>, + name: &str, + version: &str, + sites: &[PathBuf], + inherited: &HashSet, +) -> String { + let join = |dirs: &[&PathBuf]| { + dirs.iter() + .map(|d| d.display().to_string()) + .collect::>() + .join(", ") + }; + let (base, own): (Vec<&PathBuf>, Vec<&PathBuf>) = + sites.iter().partition(|d| inherited.contains(*d)); + // A venv copy that hides the same release in its base interpreter: the + // uninstall-and-sync remedy would only expose that base copy. + let mut shadowing = Vec::new(); + let mut plain = Vec::new(); + for dir in own { + match crate::crawlers::python_crawler::inherited_same_release(dir, name, version).await { + Some(base_dir) => shadowing.push((dir, base_dir)), + None => plain.push(dir), + } + } + let several = [!plain.is_empty(), !shadowing.is_empty(), !base.is_empty()] + .iter() + .filter(|kind| **kind) + .count() + > 1; + let mut parts = Vec::new(); + if !plain.is_empty() { + let remedy = stale_install_remedy(lock, name); + let lead = if several { + format!("For {}: ", join(&plain)) + } else { + String::new() + }; + parts.push(format!( + "{lead}Pipenv does not reinstall a release that is already present (`pipenv install`, `pipenv install --deploy` and `pipenv sync` all keep those bytes), so the wired Pipfile.lock only protects fresh installs. {remedy}." + )); + } + for (dir, base_dir) in &shadowing { + let remedy = shadowing_stale_install_remedy( + lock, + name, + &dir.display().to_string(), + &base_dir.display().to_string(), + ); + parts.push(format!("{remedy}.")); + } + if !base.is_empty() { + parts.push(format!( + "{}.", + base_site_stale_install_remedy(lock, name, &join(&base)) + )); + } + parts.push("Then `socket-patch vex` re-verifies the installed files.".to_string()); + parts.join(" ") +} + /// The install-over command both base-interpreter remedies share. fn install_over_command(lock: Option<&Value>, name: &str) -> String { let (package, _) = lock_sections(lock, name); @@ -998,6 +1068,97 @@ mod tests { use crate::formats::pipenv::format_entry; use crate::vendor::state::VendorArtifact; + /// #1878: hosted and vendored word the Pipenv stale-install remedy + /// through one helper. Each site class gets its own remedy: the + /// virtualenv's own copy (uninstall and sync, led by `For :` when + /// other classes are present), a virtualenv copy that hides the same + /// release in its base interpreter, and an inherited base site dir + /// (both install over the copy inside the virtualenv). + #[tokio::test] + async fn stale_install_detail_words_each_site_class() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + let venv = root.join(".venv"); + let base = root.join("base"); + let (home, venv_site, base_site) = if cfg!(windows) { + ( + base.clone(), + venv.join("Lib").join("site-packages"), + base.join("Lib").join("site-packages"), + ) + } else { + ( + base.join("bin"), + venv.join("lib").join("python3.12").join("site-packages"), + base.join("lib").join("python3.12").join("site-packages"), + ) + }; + std::fs::create_dir_all(&home).unwrap(); + for site in [&venv_site, &base_site] { + let di = site.join("six-1.16.0.dist-info"); + std::fs::create_dir_all(&di).unwrap(); + std::fs::write( + di.join("METADATA"), + "Metadata-Version: 2.1\nName: six\nVersion: 1.16.0\n", + ) + .unwrap(); + } + std::fs::write(base_site.parent().unwrap().join("os.py"), "").unwrap(); + std::fs::write( + venv.join("pyvenv.cfg"), + format!( + "home = {}\ninclude-system-site-packages = true\nversion = 3.12.4\n", + home.display() + ), + ) + .unwrap(); + let plain = root.join("plain").join("site-packages"); + std::fs::create_dir_all(&plain).unwrap(); + let lock: Value = serde_json::from_str( + r#"{"_meta": {"pipfile-spec": 6}, "default": {"six": {"version": "==1.16.0"}}}"#, + ) + .unwrap(); + let inherited = HashSet::from([base_site.clone()]); + let detail = |sites: Vec| { + let (lock, inherited) = (&lock, &inherited); + async move { stale_install_detail(Some(lock), "six", "1.16.0", &sites, inherited).await } + }; + let own = format!( + "Pipenv does not reinstall a release that is already present (`pipenv install`, \ + `pipenv install --deploy` and `pipenv sync` all keep those bytes), so the wired \ + Pipfile.lock only protects fresh installs. {}.", + stale_install_remedy(Some(&lock), "six") + ); + let shadow = format!( + "{}.", + shadowing_stale_install_remedy( + Some(&lock), + "six", + &venv_site.display().to_string(), + &base_site.display().to_string(), + ) + ); + let inherit = format!( + "{}.", + base_site_stale_install_remedy(Some(&lock), "six", &base_site.display().to_string()) + ); + let tail = "Then `socket-patch vex` re-verifies the installed files."; + + assert_eq!(detail(vec![plain.clone()]).await, format!("{own} {tail}")); + assert_eq!( + detail(vec![venv_site.clone()]).await, + format!("{shadow} {tail}") + ); + assert_eq!( + detail(vec![base_site.clone()]).await, + format!("{inherit} {tail}") + ); + assert_eq!( + detail(vec![plain.clone(), venv_site.clone(), base_site.clone()]).await, + format!("For {}: {own} {shadow} {inherit} {tail}", plain.display()) + ); + } + /// A whole lock as Pipenv writes it, `json.dumps(indent=4, /// sort_keys=True) + "\n"`, through the same renderer the splices use. fn pipenv_dumps(value: &Value) -> String { From d35ba44d5593c70725d54f03890d1ff0b8ca4936 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 30 Oct 2026 04:52:28 +0000 Subject: [PATCH 3/4] Keep hosted wording and guard remedy names Review follow-ups for the shared Pipenv remedy: hosted again says the "rewritten" Pipfile.lock (vendor says "wired"), a purl without a version falls back to the bare package name instead of the whole purl, and a name that is not a distribution name prints as `` rather than landing unquoted in the shell commands users copy. The hosted test checks the named-category arguments again, and both callers share one directory-list formatter. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/commands/scan/hosted/python.rs | 35 ++++++++++-- crates/socket-patch-core/src/utils/purl.rs | 2 +- crates/socket-patch-core/src/vendor/pypi.rs | 7 +-- .../src/vendor/pypi_pipenv.rs | 53 ++++++++++++++++--- 4 files changed, 81 insertions(+), 16 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/scan/hosted/python.rs b/crates/socket-patch-cli/src/commands/scan/hosted/python.rs index 9a22b4cfa..e796bd651 100644 --- a/crates/socket-patch-cli/src/commands/scan/hosted/python.rs +++ b/crates/socket-patch-cli/src/commands/scan/hosted/python.rs @@ -4,7 +4,7 @@ use std::collections::{BTreeMap, BTreeSet}; use socket_patch_core::crawlers::PythonCrawler; use socket_patch_core::manifest::schema::PatchRecord; -use socket_patch_core::utils::purl::{parse_pypi_purl, strip_purl_qualifiers}; +use socket_patch_core::utils::purl::{purl_parts, strip_purl_qualifiers}; use socket_patch_core::vex::verify::judge_installed_record; use super::StaleInstallOutcome; @@ -145,14 +145,20 @@ pub(super) async fn stale_install_warnings( let remedy = if let Some(env) = hatch_env { socket_patch_core::crawlers::hatch_env::stale_install_remedy(&env.name) } else if pipenv_purls.contains(&purl) { - let name = parse_pypi_purl(strip_purl_qualifiers(&purl)) - .map_or_else(|| purl.clone(), |(name, _)| name.to_string()); + let name = purl_parts(&purl) + .map(|(_, name, _)| name) + .or_else(|| { + let rest = strip_purl_qualifiers(&purl).strip_prefix("pkg:pypi/")?; + Some(rest.split('@').next().unwrap_or(rest).to_string()) + }) + .unwrap_or_default(); socket_patch_core::vendor::pypi_pipenv::stale_install_detail( pipenv_lock.as_ref(), &name, &version, std::slice::from_ref(&site), &inherited, + "rewritten", ) .await } else { @@ -357,6 +363,7 @@ mod tests { "1.16.0", std::slice::from_ref(&site), &Default::default(), + "rewritten", ) .await; assert_eq!( @@ -371,8 +378,30 @@ mod tests { detail.contains("`pipenv run pip uninstall -y six && pipenv sync --dev`"), "{detail}" ); + assert!( + detail.contains("so the rewritten Pipfile.lock only protects fresh installs"), + "{detail}" + ); + assert!( + detail.contains("Then `socket-patch vex` re-verifies the installed files."), + "{detail}" + ); assert!(!detail.contains(""), "{detail}"); assert!(!detail.contains(""), "{detail}"); + + // #790: a named category reaches the hosted remedy too. + let docs = + r#"{"_meta": {"pipfile-spec": 6}, "default": {"requests": {}}, "docs": {"six": {}}}"#; + let out = stale_install_warnings(&common, &confirmed, &pipenv, Some(docs), &ledger).await; + let detail = out.warnings[0]["detail"].as_str().unwrap(); + assert!( + detail.contains("-y six && pipenv sync --categories \"docs\"`"), + "{detail}" + ); + assert!( + detail.contains("`pipenv --rm && pipenv sync --categories \"packages docs\"`"), + "{detail}" + ); } /// #1450: a `--site-packages` venv imports the upstream release from its diff --git a/crates/socket-patch-core/src/utils/purl.rs b/crates/socket-patch-core/src/utils/purl.rs index 3eaa30961..08edd82c3 100644 --- a/crates/socket-patch-core/src/utils/purl.rs +++ b/crates/socket-patch-core/src/utils/purl.rs @@ -237,7 +237,7 @@ fn parse_namespaced<'a>(purl: &'a str, prefix: &str) -> Option `Some(("requests", "2.28.0"))` -pub fn parse_pypi_purl(purl: &str) -> Option<(Cow<'_, str>, Cow<'_, str>)> { +pub(crate) fn parse_pypi_purl(purl: &str) -> Option<(Cow<'_, str>, Cow<'_, str>)> { parse_name_version(purl, "pkg:pypi/") } diff --git a/crates/socket-patch-core/src/vendor/pypi.rs b/crates/socket-patch-core/src/vendor/pypi.rs index 6e5d93988..8759d8d4b 100644 --- a/crates/socket-patch-core/src/vendor/pypi.rs +++ b/crates/socket-patch-core/src/vendor/pypi.rs @@ -802,11 +802,7 @@ async fn pipenv_stale_install_warning( let (name, version) = parse_pypi_purl(strip_purl_qualifiers(purl)) .map(|(name, version)| (name.to_string(), version.to_string())) .unwrap_or_else(|| (purl.to_string(), String::new())); - let listed = stale_dirs - .iter() - .map(|d| d.display().to_string()) - .collect::>() - .join(", "); + let listed = super::pypi_pipenv::join_dirs(&stale_dirs); // A base interpreter's copy a `--site-packages` venv inherits survives // both the uninstall-and-sync and the recreate-and-sync remedies (#1450). let inherited = crate::crawlers::python_crawler::inherited_base_site_dirs(&sites).await; @@ -816,6 +812,7 @@ async fn pipenv_stale_install_warning( &version, &stale_dirs, &inherited, + "wired", ) .await; let detail = diff --git a/crates/socket-patch-core/src/vendor/pypi_pipenv.rs b/crates/socket-patch-core/src/vendor/pypi_pipenv.rs index d1f9d3c0b..7309d5524 100644 --- a/crates/socket-patch-core/src/vendor/pypi_pipenv.rs +++ b/crates/socket-patch-core/src/vendor/pypi_pipenv.rs @@ -959,18 +959,26 @@ pub fn shadowing_stale_install_remedy( /// in its base interpreter, or the virtualenv's own copy. Each class gets /// its remedy (the own copies led by `For :` when more than one class /// is present), and the text ends with the command that re-verifies. +/// `lock_change` says what this run did to Pipfile.lock (`wired` by vendor, +/// `rewritten` by hosted). A `name` that is not a distribution name prints +/// as ``: it lands unquoted in shell commands the user copies. pub async fn stale_install_detail( lock: Option<&Value>, name: &str, version: &str, sites: &[PathBuf], inherited: &HashSet, + lock_change: &str, ) -> String { - let join = |dirs: &[&PathBuf]| { - dirs.iter() - .map(|d| d.display().to_string()) - .collect::>() - .join(", ") + let join = |dirs: &[&PathBuf]| join_dirs(dirs.iter().copied()); + let name = if !name.is_empty() + && name + .chars() + .all(|c| c.is_ascii_alphanumeric() || matches!(c, '.' | '_' | '-')) + { + name + } else { + "" }; let (base, own): (Vec<&PathBuf>, Vec<&PathBuf>) = sites.iter().partition(|d| inherited.contains(*d)); @@ -998,7 +1006,7 @@ pub async fn stale_install_detail( String::new() }; parts.push(format!( - "{lead}Pipenv does not reinstall a release that is already present (`pipenv install`, `pipenv install --deploy` and `pipenv sync` all keep those bytes), so the wired Pipfile.lock only protects fresh installs. {remedy}." + "{lead}Pipenv does not reinstall a release that is already present (`pipenv install`, `pipenv install --deploy` and `pipenv sync` all keep those bytes), so the {lock_change} Pipfile.lock only protects fresh installs. {remedy}." )); } for (dir, base_dir) in &shadowing { @@ -1020,6 +1028,14 @@ pub async fn stale_install_detail( parts.join(" ") } +/// `dirs` as the comma-separated list the stale-install warnings print. +pub(crate) fn join_dirs<'a>(dirs: impl IntoIterator) -> String { + dirs.into_iter() + .map(|d| d.display().to_string()) + .collect::>() + .join(", ") +} + /// The install-over command both base-interpreter remedies share. fn install_over_command(lock: Option<&Value>, name: &str) -> String { let (package, _) = lock_sections(lock, name); @@ -1121,7 +1137,9 @@ mod tests { let inherited = HashSet::from([base_site.clone()]); let detail = |sites: Vec| { let (lock, inherited) = (&lock, &inherited); - async move { stale_install_detail(Some(lock), "six", "1.16.0", &sites, inherited).await } + async move { + stale_install_detail(Some(lock), "six", "1.16.0", &sites, inherited, "wired").await + } }; let own = format!( "Pipenv does not reinstall a release that is already present (`pipenv install`, \ @@ -1157,6 +1175,27 @@ mod tests { detail(vec![plain.clone(), venv_site.clone(), base_site.clone()]).await, format!("For {}: {own} {shadow} {inherit} {tail}", plain.display()) ); + + // A decoded purl name that is not a distribution name never reaches + // the copy-paste shell commands. + let unsafe_name = stale_install_detail( + Some(&lock), + "six;rm -rf ~", + "1.16.0", + std::slice::from_ref(&plain), + &inherited, + "rewritten", + ) + .await; + assert!(!unsafe_name.contains("rm -rf"), "{unsafe_name}"); + assert!( + unsafe_name.contains("`pipenv run pip uninstall -y && pipenv sync`"), + "{unsafe_name}" + ); + assert!( + unsafe_name.contains("so the rewritten Pipfile.lock"), + "{unsafe_name}" + ); } /// A whole lock as Pipenv writes it, `json.dumps(indent=4, From e7151f5887baff4b825f4d1d5e7b8ee94405bca9 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 30 Oct 2026 07:20:50 +0000 Subject: [PATCH 4/4] Refuse option-like names in Pipenv remedy A decoded purl name such as `--pre` passed the remedy's name guard and would be printed into `pip uninstall -y --pre`, where pip reads it as an option. Names must now start and end with a letter or digit, as PEP 508 requires; anything else prints as ``. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/vendor/pypi_pipenv.rs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/crates/socket-patch-core/src/vendor/pypi_pipenv.rs b/crates/socket-patch-core/src/vendor/pypi_pipenv.rs index 7309d5524..5ce7fae92 100644 --- a/crates/socket-patch-core/src/vendor/pypi_pipenv.rs +++ b/crates/socket-patch-core/src/vendor/pypi_pipenv.rs @@ -971,7 +971,11 @@ pub async fn stale_install_detail( lock_change: &str, ) -> String { let join = |dirs: &[&PathBuf]| join_dirs(dirs.iter().copied()); - let name = if !name.is_empty() + // PEP 508: letters, digits, `.`, `_` and `-`, starting and ending with a + // letter or digit (so a `-`-led name can't read as a pip option). + let edge = |c: Option| c.is_some_and(|c| c.is_ascii_alphanumeric()); + let name = if edge(name.chars().next()) + && edge(name.chars().last()) && name .chars() .all(|c| c.is_ascii_alphanumeric() || matches!(c, '.' | '_' | '-')) @@ -1188,6 +1192,18 @@ mod tests { ) .await; assert!(!unsafe_name.contains("rm -rf"), "{unsafe_name}"); + // Nor does a name pip would read as an option. + let option = stale_install_detail( + Some(&lock), + "--pre", + "1.16.0", + std::slice::from_ref(&plain), + &inherited, + "rewritten", + ) + .await; + assert!(!option.contains("--pre"), "{option}"); + assert!(option.contains("uninstall -y &&"), "{option}"); assert!( unsafe_name.contains("`pipenv run pip uninstall -y && pipenv sync`"), "{unsafe_name}"