From 7d8ffdcca8e3831b4eec75109a86d6c349d30b5e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 1 Oct 2026 17:26:40 +0000 Subject: [PATCH 1/3] Start fix for #332, #361 Assisted-by: Claude Code:claude-opus-5-5 From b3a739f77fa574a6db666f33309cb500572f1666 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 1 Oct 2026 17:34:53 +0000 Subject: [PATCH 2/3] Stop agent apply patching shared package stores PDM's symlink install cache and pnpm's global virtual store link a package directory into a store every project on the machine uses. Agent apply renamed the patched file inside that directory, so other projects were patched too, and a rollback in one project silently unpatched the rest. Apply now refuses a package whose real location is such a store, and rollback refuses whenever it would write there. The error names the store and how to get a private copy. Per-project stores reached through a symlink (node_modules/.pnpm, workspace links) are patched as before. Fixes #332, #361. Assisted-by: Claude Code:claude-opus-5-5 --- CHANGELOG.md | 9 + crates/socket-patch-core/src/patch/apply.rs | 164 ++++++++++ crates/socket-patch-core/src/patch/mod.rs | 1 + .../socket-patch-core/src/patch/rollback.rs | 65 ++++ .../src/patch/shared_store.rs | 279 ++++++++++++++++++ docs/ecosystems.md | 9 + 6 files changed, 527 insertions(+) create mode 100644 crates/socket-patch-core/src/patch/shared_store.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index b03e92f8d..bb44c106e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -183,6 +183,15 @@ limits, and required install commands. unparseable (hosted) or refused as `vendor_lockfile_version_unsupported` (vendored). The lock now keeps its BOM, indent and line endings, and the undo is byte-exact (#324). +- Agent mode no longer patches other projects through a store they share. + PDM 2.0–2.12 with `install.cache` and `cache_method = symlink` links + `site-packages/` into its package cache, and pnpm's global virtual + store (`enableGlobalVirtualStore`) links `node_modules/` into + `/links`. `apply` (also `-g`) wrote the patch into that shared + directory, so every project using it was patched, and a `rollback` in one + project silently unpatched the rest. `apply` and `rollback` now fail on + such a package, naming the store and how to get a private copy + (#332, #361). ### Maintenance diff --git a/crates/socket-patch-core/src/patch/apply.rs b/crates/socket-patch-core/src/patch/apply.rs index 8ac188828..4a1ad85e8 100644 --- a/crates/socket-patch-core/src/patch/apply.rs +++ b/crates/socket-patch-core/src/patch/apply.rs @@ -811,6 +811,16 @@ async fn apply_package_patch_at( sidecar: None, }; + // A package dir that resolves into a store other projects link to + // (PDM's symlink cache, pnpm's global virtual store) is not ours to + // patch: the rename below would land in the shared dir and patch every + // project using it. Refused in every state, dry run and already-patched + // included, so this project never records the shared copy as its patch. + if let Some(store) = crate::patch::shared_store::shared_store_of(pkg_path).await { + result.error = Some(store.refusal("patch")); + return result; + } + // First, verify all files for (file_name, file_info) in files_in_order(files) { // SECURITY: reject any manifest key that would escape the package dir @@ -3489,4 +3499,158 @@ mod tests { Some("store copy /store/pkg@1.0.0_peer failed to patch: boom") ); } + + /// Lay out one shared store package with `index.js` (original bytes) + /// and link it into two projects' install dirs, the way pnpm's global + /// virtual store (#361) and PDM's symlink cache (#332) do. Returns + /// (root, [project A link, project B link], blobs dir, files, original, + /// patched). + #[cfg(unix)] + fn shared_store_fixture( + pdm: bool, + ) -> ( + tempfile::TempDir, + [std::path::PathBuf; 2], + std::path::PathBuf, + HashMap, + Vec, + Vec, + ) { + use crate::patch::shared_store::test_support::{make_pdm_cache_entry, make_pnpm_gvs}; + let root = tempfile::tempdir().unwrap(); + let store_pkg = if pdm { + make_pdm_cache_entry(root.path()) + } else { + make_pnpm_gvs(root.path()) + }; + let original = b"original shared bytes".to_vec(); + let patched = b"PATCHED shared bytes".to_vec(); + std::fs::write(store_pkg.join("index.js"), &original).unwrap(); + let links = ["a", "b"].map(|p| { + let install_dir = if pdm { + root.path() + .join(p) + .join(".venv/lib/python3.11/site-packages") + } else { + root.path().join(p).join("node_modules") + }; + std::fs::create_dir_all(&install_dir).unwrap(); + let link = install_dir.join(store_pkg.file_name().unwrap()); + std::os::unix::fs::symlink(&store_pkg, &link).unwrap(); + link + }); + let blobs = root.path().join("blobs"); + std::fs::create_dir_all(&blobs).unwrap(); + let before_hash = compute_git_sha256_from_bytes(&original); + let after_hash = compute_git_sha256_from_bytes(&patched); + std::fs::write(blobs.join(&before_hash), &original).unwrap(); + std::fs::write(blobs.join(&after_hash), &patched).unwrap(); + let mut files = HashMap::new(); + files.insert( + "index.js".to_string(), + PatchFileInfo { + before_hash, + after_hash, + }, + ); + (root, links, blobs, files, original, patched) + } + + /// #361 / #332: agent apply must not write through a package directory + /// that is a link into a store shared with other projects. It fails + /// closed (dry run included), and the other project keeps its bytes. + #[cfg(unix)] + #[tokio::test] + async fn test_apply_refuses_shared_store_package_dir() { + for (pdm, purl) in [ + (false, "pkg:npm/left-pad@1.3.0"), + (true, "pkg:pypi/urllib3@1.26.18"), + ] { + let (_root, [a, b], blobs, files, original, _patched) = shared_store_fixture(pdm); + let sources = PatchSources::blobs_only(&blobs); + for dry_run in [true, false] { + let result = apply_package_patch( + purl, + &a, + &files, + &sources, + None, + dry_run, + MismatchPolicy::Warn, + ) + .await; + assert!(!result.success, "{purl} dry_run={dry_run}: must refuse"); + let err = result.error.unwrap_or_default(); + assert!( + err.contains(crate::patch::shared_store::SHARED_STORE_REFUSAL_MARKER), + "{purl}: {err}" + ); + assert!(result.files_patched.is_empty()); + } + assert_eq!( + std::fs::read(b.join("index.js")).unwrap(), + original, + "{purl}" + ); + } + } + + /// The same store package already patched (by another project, or by + /// an apply before this guard existed) is refused too: this project + /// does not own the shared copy, so it must not record it as patched. + #[cfg(unix)] + #[tokio::test] + async fn test_apply_refuses_already_patched_shared_store_package_dir() { + let (_root, [a, _b], blobs, files, _original, patched) = shared_store_fixture(false); + std::fs::write(a.join("index.js"), &patched).unwrap(); + let result = apply_package_patch( + "pkg:npm/left-pad@1.3.0", + &a, + &files, + &PatchSources::blobs_only(&blobs), + None, + false, + MismatchPolicy::Warn, + ) + .await; + assert!(!result.success); + } + + /// A per-project pnpm store reached through a symlink is still patched. + #[cfg(unix)] + #[tokio::test] + async fn test_apply_patches_through_per_project_pnpm_link() { + let root = tempfile::tempdir().unwrap(); + let nm = root.path().join("node_modules"); + let real = nm.join(".pnpm/left-pad@1.3.0/node_modules/left-pad"); + std::fs::create_dir_all(&real).unwrap(); + let original = b"original".to_vec(); + let patched = b"patched!".to_vec(); + std::fs::write(real.join("index.js"), &original).unwrap(); + std::os::unix::fs::symlink(&real, nm.join("left-pad")).unwrap(); + let blobs = root.path().join("blobs"); + std::fs::create_dir_all(&blobs).unwrap(); + let after_hash = compute_git_sha256_from_bytes(&patched); + std::fs::write(blobs.join(&after_hash), &patched).unwrap(); + let mut files = HashMap::new(); + files.insert( + "index.js".to_string(), + PatchFileInfo { + before_hash: compute_git_sha256_from_bytes(&original), + after_hash, + }, + ); + let result = apply_package_patch( + "pkg:npm/left-pad@1.3.0", + &nm.join("left-pad"), + &files, + &PatchSources::blobs_only(&blobs), + None, + false, + MismatchPolicy::Warn, + ) + .await; + assert!(result.success, "{:?}", result.error); + assert_eq!(std::fs::read(real.join("index.js")).unwrap(), patched); + } } diff --git a/crates/socket-patch-core/src/patch/mod.rs b/crates/socket-patch-core/src/patch/mod.rs index ba709c6b1..b6f636cb4 100644 --- a/crates/socket-patch-core/src/patch/mod.rs +++ b/crates/socket-patch-core/src/patch/mod.rs @@ -8,4 +8,5 @@ pub mod package; pub(crate) mod path_safety; pub mod redirect; pub mod rollback; +pub mod shared_store; pub mod sidecars; diff --git a/crates/socket-patch-core/src/patch/rollback.rs b/crates/socket-patch-core/src/patch/rollback.rs index 571a4098c..7457da20f 100644 --- a/crates/socket-patch-core/src/patch/rollback.rs +++ b/crates/socket-patch-core/src/patch/rollback.rs @@ -437,6 +437,16 @@ async fn rollback_package_patch_at( .files_verified .iter() .all(|v| v.status == VerifyRollbackStatus::AlreadyOriginal); + // Restoring bytes into a package dir shared with other projects (PDM's + // symlink cache, pnpm's global virtual store) would silently unpatch + // them. Refused whenever a write would happen, dry run included; an + // already-original shared copy needs no write and passes. + if !all_original { + if let Some(store) = crate::patch::shared_store::shared_store_of(pkg_path).await { + result.error = Some(store.refusal("roll back")); + return result; + } + } if all_original || dry_run { result.success = true; return result; @@ -2505,4 +2515,59 @@ mod tests { "an already-original primary must still heal a patched twin" ); } + + /// #361 / #332: rollback in one project must not restore the original + /// bytes into a package directory shared with other projects (that + /// would silently unpatch them). It fails closed, dry run included, + /// and leaves the shared bytes as they are. + #[cfg(unix)] + #[tokio::test] + async fn test_rollback_refuses_shared_store_package_dir() { + use crate::patch::shared_store::test_support::{make_pdm_cache_entry, make_pnpm_gvs}; + for (pdm, purl) in [ + (false, "pkg:npm/left-pad@1.3.0"), + (true, "pkg:pypi/urllib3@1.26.18"), + ] { + let root = tempfile::tempdir().unwrap(); + let store_pkg = if pdm { + make_pdm_cache_entry(root.path()) + } else { + make_pnpm_gvs(root.path()) + }; + let original = b"original shared bytes".to_vec(); + let patched = b"PATCHED shared bytes".to_vec(); + std::fs::write(store_pkg.join("index.js"), &patched).unwrap(); + let install_dir = root.path().join("a").join("install"); + std::fs::create_dir_all(&install_dir).unwrap(); + let link = install_dir.join(store_pkg.file_name().unwrap()); + std::os::unix::fs::symlink(&store_pkg, &link).unwrap(); + let blobs = root.path().join("blobs"); + std::fs::create_dir_all(&blobs).unwrap(); + let before_hash = compute_git_sha256_from_bytes(&original); + std::fs::write(blobs.join(&before_hash), &original).unwrap(); + let mut files = HashMap::new(); + files.insert( + "index.js".to_string(), + PatchFileInfo { + before_hash, + after_hash: compute_git_sha256_from_bytes(&patched), + }, + ); + for dry_run in [true, false] { + let result = rollback_package_patch(purl, &link, &files, &blobs, dry_run).await; + assert!(!result.success, "{purl} dry_run={dry_run}: must refuse"); + let err = result.error.unwrap_or_default(); + assert!( + err.contains(crate::patch::shared_store::SHARED_STORE_REFUSAL_MARKER), + "{purl}: {err}" + ); + } + assert_eq!(std::fs::read(store_pkg.join("index.js")).unwrap(), patched); + + // Already original: nothing to write, so nothing to refuse. + std::fs::write(store_pkg.join("index.js"), &original).unwrap(); + let result = rollback_package_patch(purl, &link, &files, &blobs, false).await; + assert!(result.success, "{purl}: {:?}", result.error); + } + } } diff --git a/crates/socket-patch-core/src/patch/shared_store.rs b/crates/socket-patch-core/src/patch/shared_store.rs new file mode 100644 index 000000000..498c72869 --- /dev/null +++ b/crates/socket-patch-core/src/patch/shared_store.rs @@ -0,0 +1,279 @@ +//! Recognizing package directories that live in a store shared across +//! projects. +//! +//! Agent-mode apply and rollback commit each file with a stage + `rename(2)` +//! in the file's parent directory. That isolates a hardlinked or symlinked +//! *file* (pnpm's content-addressable `files/`, the bun / uv caches, Go's +//! module cache), but not a package *directory* that is itself a symlink +//! into a store every project on the machine links to: the rename then lands +//! inside the shared directory, patching (or, on rollback, unpatching) every +//! other project that uses it. Two package managers install that way: +//! +//! * **PDM's package cache** (`install.cache = true` with +//! `install.cache_method = symlink`, PDM 2.0–2.12): `site-packages/` +//! is a directory symlink into `/packages//lib/`. +//! Each cache entry carries a `referrers` file listing the environments +//! that use it. +//! * **pnpm's global virtual store** (`enableGlobalVirtualStore`): +//! `node_modules/` is a symlink (a junction on Windows) into +//! `/v/links/…/node_modules/`, beside the store's +//! `files/` content directory. +//! +//! Detection is positive and marker-based, on the package directory's real +//! path: a per-project store reached through a symlink (pnpm's +//! `node_modules/.pnpm`, a relocated `virtualStoreDir`, a workspace link) +//! carries neither marker and is patched as before. + +use std::path::{Path, PathBuf}; + +/// The substring every shared-store refusal carries, so callers and tests +/// can recognize it without matching the full sentence. +pub const SHARED_STORE_REFUSAL_MARKER: &str = "shared by other projects"; + +/// A cross-project store a package directory resolves into. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SharedStoreKind { + /// `/packages//lib`, PDM's symlink install cache. + PdmPackageCache, + /// `/v/links`, pnpm's global virtual store. + PnpmGlobalVirtualStore, +} + +/// Where a package directory really lives, when that is a shared store. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SharedStore { + pub kind: SharedStoreKind, + /// The package directory's real (canonical) path. + pub real_path: PathBuf, +} + +impl SharedStore { + /// The refusal message for `action` ("patch" or "roll back"), naming + /// the store and how to get a private copy instead. + pub fn refusal(&self, action: &str) -> String { + let (what, remedy) = match self.kind { + SharedStoreKind::PdmPackageCache => ( + "PDM's package cache (install.cache with cache_method = symlink)", + "run `pdm config install.cache_method hardlink` (or turn \ + install.cache off) and reinstall the package", + ), + SharedStoreKind::PnpmGlobalVirtualStore => ( + "pnpm's global virtual store (enableGlobalVirtualStore)", + "set enableGlobalVirtualStore to false and reinstall", + ), + }; + format!( + "Refusing to {action} {path}: it is in {what}, which is \ + {SHARED_STORE_REFUSAL_MARKER} on this machine, so the change would \ + reach them too. To patch only this project, {remedy}, or use \ + `scan --mode hosted` / `--mode vendored`", + path = self.real_path.display(), + ) + } +} + +/// Classify `pkg_path`: `Some` when its real location is inside a shared +/// store (see the module docs). A path that does not exist, or cannot be +/// resolved, is `None`; the normal verify step reports a missing package. +pub async fn shared_store_of(pkg_path: &Path) -> Option { + let pkg_path = pkg_path.to_path_buf(); + tokio::task::spawn_blocking(move || shared_store_of_blocking(&pkg_path)) + .await + .ok() + .flatten() +} + +fn shared_store_of_blocking(pkg_path: &Path) -> Option { + let real = std::fs::canonicalize(pkg_path).ok()?; + for dir in real.ancestors() { + let Some(name) = dir.file_name().and_then(|n| n.to_str()) else { + continue; + }; + let parent = dir.parent(); + let parent_name = parent.and_then(|p| p.file_name()).and_then(|n| n.to_str()); + + // pnpm: /v/links, with the store's `files/` beside it. + if name == "links" + && parent_name.is_some_and(is_pnpm_store_version_dir) + && parent.is_some_and(|p| p.join("files").is_dir()) + { + return Some(SharedStore { + kind: SharedStoreKind::PnpmGlobalVirtualStore, + real_path: real.clone(), + }); + } + + // PDM: /packages//lib/…, the entry carrying its + // `referrers` registry. + if name == "lib" + && dir != real + && parent.is_some_and(|entry| { + entry.join("referrers").is_file() + && entry + .parent() + .and_then(|p| p.file_name()) + .is_some_and(|n| n == "packages") + }) + { + return Some(SharedStore { + kind: SharedStoreKind::PdmPackageCache, + real_path: real.clone(), + }); + } + } + None +} + +/// `v3`, `v10`, `v11`, …: the layout-version directory of a pnpm store. +fn is_pnpm_store_version_dir(name: &str) -> bool { + name.strip_prefix('v') + .is_some_and(|n| !n.is_empty() && n.bytes().all(|b| b.is_ascii_digit())) +} + +#[cfg(test)] +pub(crate) mod test_support { + use std::path::{Path, PathBuf}; + + /// `/store/v10/{files,links/@/left-pad/1.3.0//node_modules/left-pad}`. + pub(crate) fn make_pnpm_gvs(root: &Path) -> PathBuf { + let v = root.join("store").join("v10"); + std::fs::create_dir_all(v.join("files")).unwrap(); + std::fs::create_dir_all(v.join("index")).unwrap(); + let pkg = v + .join("links") + .join("@") + .join("left-pad") + .join("1.3.0") + .join("abc123") + .join("node_modules") + .join("left-pad"); + std::fs::create_dir_all(&pkg).unwrap(); + pkg + } + + /// `/pdm-cache/packages/urllib3-1.26.18-py2.py3-none-any/{referrers,lib/urllib3}`. + pub(crate) fn make_pdm_cache_entry(root: &Path) -> PathBuf { + let entry = root + .join("pdm-cache") + .join("packages") + .join("urllib3-1.26.18-py2.py3-none-any"); + let pkg = entry.join("lib").join("urllib3"); + std::fs::create_dir_all(&pkg).unwrap(); + std::fs::write(entry.join("referrers"), "/somewhere/else\n").unwrap(); + pkg + } +} + +#[cfg(test)] +mod tests { + use super::test_support::*; + use super::*; + + #[tokio::test] + async fn pnpm_global_virtual_store_is_shared() { + let dir = tempfile::tempdir().unwrap(); + let pkg = make_pnpm_gvs(dir.path()); + let got = shared_store_of(&pkg).await.expect("shared"); + assert_eq!(got.kind, SharedStoreKind::PnpmGlobalVirtualStore); + assert_eq!(got.real_path, std::fs::canonicalize(&pkg).unwrap()); + } + + #[tokio::test] + async fn pdm_package_cache_is_shared() { + let dir = tempfile::tempdir().unwrap(); + let pkg = make_pdm_cache_entry(dir.path()); + let got = shared_store_of(&pkg).await.expect("shared"); + assert_eq!(got.kind, SharedStoreKind::PdmPackageCache); + // A file-level entry beside the package (a top-level module) is + // inside the shared `lib/` too. + let entry_lib = pkg.parent().unwrap().to_path_buf(); + std::fs::write(entry_lib.join("six.py"), "x").unwrap(); + assert!(shared_store_of(&entry_lib.join("six.py")).await.is_some()); + } + + /// pnpm's per-project virtual store and plain directories carry no + /// shared-store marker. + #[tokio::test] + async fn per_project_layouts_are_not_shared() { + let dir = tempfile::tempdir().unwrap(); + let pnpm = dir + .path() + .join("node_modules") + .join(".pnpm") + .join("left-pad@1.3.0") + .join("node_modules") + .join("left-pad"); + std::fs::create_dir_all(&pnpm).unwrap(); + assert_eq!(shared_store_of(&pnpm).await, None); + + // A `links` dir without the store's `files/` sibling, or under a + // non-version parent, is an ordinary directory. + let links = dir.path().join("v10").join("links").join("pkg"); + std::fs::create_dir_all(&links).unwrap(); + assert_eq!(shared_store_of(&links).await, None); + let other = dir.path().join("vendor").join("links").join("pkg"); + std::fs::create_dir_all(&other).unwrap(); + std::fs::create_dir_all(dir.path().join("vendor").join("files")).unwrap(); + assert_eq!(shared_store_of(&other).await, None); + + // A site-packages `lib/` without the PDM entry markers. + let venv = dir + .path() + .join(".venv") + .join("lib") + .join("python3.11") + .join("site-packages") + .join("six"); + std::fs::create_dir_all(&venv).unwrap(); + assert_eq!(shared_store_of(&venv).await, None); + let no_referrers = dir + .path() + .join("packages") + .join("x-1.0-py3-none-any") + .join("lib") + .join("x"); + std::fs::create_dir_all(&no_referrers).unwrap(); + assert_eq!(shared_store_of(&no_referrers).await, None); + + // Missing paths are not classified. + assert_eq!(shared_store_of(&dir.path().join("missing")).await, None); + } + + #[cfg(unix)] + #[tokio::test] + async fn symlinked_package_dir_resolves_to_the_store() { + let dir = tempfile::tempdir().unwrap(); + let pkg = make_pnpm_gvs(dir.path()); + let nm = dir.path().join("project").join("node_modules"); + std::fs::create_dir_all(&nm).unwrap(); + std::os::unix::fs::symlink(&pkg, nm.join("left-pad")).unwrap(); + assert!(shared_store_of(&nm.join("left-pad")).await.is_some()); + + // A per-project `.pnpm` link is not shared. + let private = nm + .join(".pnpm") + .join("is-odd@3.0.1") + .join("node_modules") + .join("is-odd"); + std::fs::create_dir_all(&private).unwrap(); + std::os::unix::fs::symlink(&private, nm.join("is-odd")).unwrap(); + assert_eq!(shared_store_of(&nm.join("is-odd")).await, None); + } + + #[test] + fn refusal_names_store_and_remedy() { + let s = SharedStore { + kind: SharedStoreKind::PdmPackageCache, + real_path: PathBuf::from("/c/packages/x/lib/x"), + }; + let msg = s.refusal("patch"); + assert!(msg.contains(SHARED_STORE_REFUSAL_MARKER), "{msg}"); + assert!(msg.contains("/c/packages/x/lib/x"), "{msg}"); + assert!(msg.contains("install.cache_method hardlink"), "{msg}"); + let s = SharedStore { + kind: SharedStoreKind::PnpmGlobalVirtualStore, + real_path: PathBuf::from("/s/v10/links/x"), + }; + assert!(s.refusal("roll back").contains("enableGlobalVirtualStore")); + } +} diff --git a/docs/ecosystems.md b/docs/ecosystems.md index 00263964f..58761e501 100644 --- a/docs/ecosystems.md +++ b/docs/ecosystems.md @@ -149,6 +149,15 @@ moved it to, as recorded in `node_modules/.modules.yaml`), vlt's notably pnpm's global virtual store (`enableGlobalVirtualStore`, under the pnpm store directory), is not walked: other projects on the machine load the same files, so patching it in place would patch them as well. +For the same reason, agent-mode `apply` and `rollback` fail on a direct +dependency whose `node_modules/` link resolves into that store +(`/v/links`) instead of writing through it. PDM's symlink install +cache gets the same treatment: with `install.cache` and +`cache_method = symlink` (PDM 2.0–2.12), `site-packages/` links into +`/packages//lib`, and that package is refused too. The error +names the store and how to get a private copy (disable the global virtual +store, or `pdm config install.cache_method hardlink`, then reinstall), or +use hosted or vendored mode. Every command that looks for installed npm copies walks these same trees, not only `scan`. A package installed only under a pruned directory is therefore From 0f84373c4c0d4c33ead5bfdd372955546f637446 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 1 Oct 2026 18:25:50 +0000 Subject: [PATCH 3/3] Check PyPI package dirs for shared stores too A PyPI patch is rooted at site-packages with file keys like urllib3/response.py, so the PDM cache link sits below the root and the first guard, which only looked at the root, never saw it. Apply and rollback now classify the directory of every patched file, so a PDM symlink-cache package is refused as intended. Refs #332. Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-core/src/patch/apply.rs | 39 ++++++----- .../socket-patch-core/src/patch/rollback.rs | 20 ++++-- .../src/patch/shared_store.rs | 64 +++++++++++++++++++ 3 files changed, 104 insertions(+), 19 deletions(-) diff --git a/crates/socket-patch-core/src/patch/apply.rs b/crates/socket-patch-core/src/patch/apply.rs index 4a1ad85e8..da0982883 100644 --- a/crates/socket-patch-core/src/patch/apply.rs +++ b/crates/socket-patch-core/src/patch/apply.rs @@ -816,7 +816,12 @@ async fn apply_package_patch_at( // patch: the rename below would land in the shared dir and patch every // project using it. Refused in every state, dry run and already-patched // included, so this project never records the shared copy as its patch. - if let Some(store) = crate::patch::shared_store::shared_store_of(pkg_path).await { + if let Some(store) = crate::patch::shared_store::shared_store_of_patch_dirs( + pkg_path, + files.keys().map(String::as_str), + ) + .await + { result.error = Some(store.refusal("patch")); return result; } @@ -3503,14 +3508,17 @@ mod tests { /// Lay out one shared store package with `index.js` (original bytes) /// and link it into two projects' install dirs, the way pnpm's global /// virtual store (#361) and PDM's symlink cache (#332) do. Returns - /// (root, [project A link, project B link], blobs dir, files, original, - /// patched). + /// (root, [project A, project B] package roots, file key, blobs dir, + /// files, original, patched). The package root is what the crawlers + /// hand apply: the linked `node_modules/` for npm, but the + /// `site-packages` dir for PyPI, whose keys are `/`. #[cfg(unix)] fn shared_store_fixture( pdm: bool, ) -> ( tempfile::TempDir, [std::path::PathBuf; 2], + String, std::path::PathBuf, HashMap, Vec, @@ -3526,7 +3534,7 @@ mod tests { let original = b"original shared bytes".to_vec(); let patched = b"PATCHED shared bytes".to_vec(); std::fs::write(store_pkg.join("index.js"), &original).unwrap(); - let links = ["a", "b"].map(|p| { + let roots = ["a", "b"].map(|p| { let install_dir = if pdm { root.path() .join(p) @@ -3537,8 +3545,13 @@ mod tests { std::fs::create_dir_all(&install_dir).unwrap(); let link = install_dir.join(store_pkg.file_name().unwrap()); std::os::unix::fs::symlink(&store_pkg, &link).unwrap(); - link + if pdm { + install_dir + } else { + link + } }); + let key = if pdm { "urllib3/index.js" } else { "index.js" }.to_string(); let blobs = root.path().join("blobs"); std::fs::create_dir_all(&blobs).unwrap(); let before_hash = compute_git_sha256_from_bytes(&original); @@ -3547,13 +3560,13 @@ mod tests { std::fs::write(blobs.join(&after_hash), &patched).unwrap(); let mut files = HashMap::new(); files.insert( - "index.js".to_string(), + key.clone(), PatchFileInfo { before_hash, after_hash, }, ); - (root, links, blobs, files, original, patched) + (root, roots, key, blobs, files, original, patched) } /// #361 / #332: agent apply must not write through a package directory @@ -3566,7 +3579,7 @@ mod tests { (false, "pkg:npm/left-pad@1.3.0"), (true, "pkg:pypi/urllib3@1.26.18"), ] { - let (_root, [a, b], blobs, files, original, _patched) = shared_store_fixture(pdm); + let (_root, [a, b], key, blobs, files, original, _patched) = shared_store_fixture(pdm); let sources = PatchSources::blobs_only(&blobs); for dry_run in [true, false] { let result = apply_package_patch( @@ -3587,11 +3600,7 @@ mod tests { ); assert!(result.files_patched.is_empty()); } - assert_eq!( - std::fs::read(b.join("index.js")).unwrap(), - original, - "{purl}" - ); + assert_eq!(std::fs::read(b.join(&key)).unwrap(), original, "{purl}"); } } @@ -3601,8 +3610,8 @@ mod tests { #[cfg(unix)] #[tokio::test] async fn test_apply_refuses_already_patched_shared_store_package_dir() { - let (_root, [a, _b], blobs, files, _original, patched) = shared_store_fixture(false); - std::fs::write(a.join("index.js"), &patched).unwrap(); + let (_root, [a, _b], key, blobs, files, _original, patched) = shared_store_fixture(false); + std::fs::write(a.join(key), &patched).unwrap(); let result = apply_package_patch( "pkg:npm/left-pad@1.3.0", &a, diff --git a/crates/socket-patch-core/src/patch/rollback.rs b/crates/socket-patch-core/src/patch/rollback.rs index 7457da20f..73797f61c 100644 --- a/crates/socket-patch-core/src/patch/rollback.rs +++ b/crates/socket-patch-core/src/patch/rollback.rs @@ -442,7 +442,12 @@ async fn rollback_package_patch_at( // them. Refused whenever a write would happen, dry run included; an // already-original shared copy needs no write and passes. if !all_original { - if let Some(store) = crate::patch::shared_store::shared_store_of(pkg_path).await { + if let Some(store) = crate::patch::shared_store::shared_store_of_patch_dirs( + pkg_path, + files.keys().map(String::as_str), + ) + .await + { result.error = Some(store.refusal("roll back")); return result; } @@ -2541,20 +2546,27 @@ mod tests { std::fs::create_dir_all(&install_dir).unwrap(); let link = install_dir.join(store_pkg.file_name().unwrap()); std::os::unix::fs::symlink(&store_pkg, &link).unwrap(); + // What the crawlers hand rollback: the linked package dir for + // npm, but `site-packages` (keys `/`) for PyPI. + let (pkg_root, key) = if pdm { + (install_dir.clone(), "urllib3/index.js") + } else { + (link.clone(), "index.js") + }; let blobs = root.path().join("blobs"); std::fs::create_dir_all(&blobs).unwrap(); let before_hash = compute_git_sha256_from_bytes(&original); std::fs::write(blobs.join(&before_hash), &original).unwrap(); let mut files = HashMap::new(); files.insert( - "index.js".to_string(), + key.to_string(), PatchFileInfo { before_hash, after_hash: compute_git_sha256_from_bytes(&patched), }, ); for dry_run in [true, false] { - let result = rollback_package_patch(purl, &link, &files, &blobs, dry_run).await; + let result = rollback_package_patch(purl, &pkg_root, &files, &blobs, dry_run).await; assert!(!result.success, "{purl} dry_run={dry_run}: must refuse"); let err = result.error.unwrap_or_default(); assert!( @@ -2566,7 +2578,7 @@ mod tests { // Already original: nothing to write, so nothing to refuse. std::fs::write(store_pkg.join("index.js"), &original).unwrap(); - let result = rollback_package_patch(purl, &link, &files, &blobs, false).await; + let result = rollback_package_patch(purl, &pkg_root, &files, &blobs, false).await; assert!(result.success, "{purl}: {:?}", result.error); } } diff --git a/crates/socket-patch-core/src/patch/shared_store.rs b/crates/socket-patch-core/src/patch/shared_store.rs index 498c72869..4c8eea314 100644 --- a/crates/socket-patch-core/src/patch/shared_store.rs +++ b/crates/socket-patch-core/src/patch/shared_store.rs @@ -83,6 +83,42 @@ pub async fn shared_store_of(pkg_path: &Path) -> Option { .flatten() } +/// Classify every directory a patch writes into: `pkg_path` itself and +/// the parent of each (normalized, package-relative) file key. The package +/// root is not always the package directory: a PyPI patch is rooted at +/// `site-packages` with keys like `urllib3/response.py`, so the directory +/// symlink into PDM's cache sits *below* the root. A parent that does not +/// exist yet (a patch adding a file under a new subdir) is classified by +/// its nearest existing ancestor at or below `pkg_path`. Keys that escape +/// the package dir are skipped; the caller refuses them on its own. +pub async fn shared_store_of_patch_dirs<'a>( + pkg_path: &Path, + file_keys: impl IntoIterator, +) -> Option { + let pkg_path = pkg_path.to_path_buf(); + let keys: Vec = file_keys + .into_iter() + .map(crate::patch::apply::normalize_file_path) + .filter(|k| crate::patch::apply::is_safe_relative_subpath(k)) + .map(str::to_string) + .collect(); + tokio::task::spawn_blocking(move || { + let mut seen = std::collections::HashSet::new(); + let dirs = std::iter::once(pkg_path.clone()).chain(keys.iter().filter_map(|key| { + let mut dir = pkg_path.join(key).parent()?.to_path_buf(); + while dir != pkg_path && !dir.exists() { + dir = dir.parent()?.to_path_buf(); + } + Some(dir) + })); + dirs.filter(|dir| seen.insert(dir.clone())) + .find_map(|dir| shared_store_of_blocking(&dir)) + }) + .await + .ok() + .flatten() +} + fn shared_store_of_blocking(pkg_path: &Path) -> Option { let real = std::fs::canonicalize(pkg_path).ok()?; for dir in real.ancestors() { @@ -260,6 +296,34 @@ mod tests { assert_eq!(shared_store_of(&nm.join("is-odd")).await, None); } + /// A PyPI patch is rooted at `site-packages` (keys `/`), so + /// the directory link into PDM's cache sits below the root and is found + /// through the keys, including a key under a subdir that does not exist + /// yet. A top-level module key (`six.py`) is classified by its parent, + /// `site-packages`, which is private. + #[cfg(unix)] + #[tokio::test] + async fn patch_dirs_find_a_link_below_the_package_root() { + let dir = tempfile::tempdir().unwrap(); + let pkg = make_pdm_cache_entry(dir.path()); + let site = dir.path().join("venv").join("site-packages"); + std::fs::create_dir_all(&site).unwrap(); + std::os::unix::fs::symlink(&pkg, site.join("urllib3")).unwrap(); + + assert_eq!(shared_store_of(&site).await, None); + for key in ["urllib3/response.py", "urllib3/new/dir/added.py"] { + let got = shared_store_of_patch_dirs(&site, [key]).await; + assert_eq!( + got.map(|s| s.kind), + Some(SharedStoreKind::PdmPackageCache), + "{key}" + ); + } + assert_eq!(shared_store_of_patch_dirs(&site, ["six.py"]).await, None); + // An escaping key is ignored here (apply refuses it separately). + assert_eq!(shared_store_of_patch_dirs(&site, ["../x/y.py"]).await, None); + } + #[test] fn refusal_names_store_and_remedy() { let s = SharedStore {