Skip to content

Commit 8a50d5a

Browse files
Fix agent apply writing into shared package stores (#332, #361) (#486)
* Start fix for #332, #361 Assisted-by: Claude Code:claude-opus-5-5 * 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 * 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 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 795a535 commit 8a50d5a

6 files changed

Lines changed: 612 additions & 0 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,15 @@ limits, and required install commands.
200200
unparseable (hosted) or refused as `vendor_lockfile_version_unsupported`
201201
(vendored). The lock now keeps its BOM, indent and line endings, and the
202202
undo is byte-exact (#324).
203+
- Agent mode no longer patches other projects through a store they share.
204+
PDM 2.0–2.12 with `install.cache` and `cache_method = symlink` links
205+
`site-packages/<pkg>` into its package cache, and pnpm's global virtual
206+
store (`enableGlobalVirtualStore`) links `node_modules/<dep>` into
207+
`<store>/links`. `apply` (also `-g`) wrote the patch into that shared
208+
directory, so every project using it was patched, and a `rollback` in one
209+
project silently unpatched the rest. `apply` and `rollback` now fail on
210+
such a package, naming the store and how to get a private copy
211+
(#332, #361).
203212
- `vendor` under `--global` / `--global-prefix` (or `SOCKET_GLOBAL` /
204213
`SOCKET_GLOBAL_PREFIX`) is now a usage error (exit 2,
205214
`global_scope_unsupported`), like `scan` and `get` with `--mode vendored`.

‎crates/socket-patch-core/src/patch/apply.rs‎

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -811,6 +811,21 @@ async fn apply_package_patch_at(
811811
sidecar: None,
812812
};
813813

814+
// A package dir that resolves into a store other projects link to
815+
// (PDM's symlink cache, pnpm's global virtual store) is not ours to
816+
// patch: the rename below would land in the shared dir and patch every
817+
// project using it. Refused in every state, dry run and already-patched
818+
// included, so this project never records the shared copy as its patch.
819+
if let Some(store) = crate::patch::shared_store::shared_store_of_patch_dirs(
820+
pkg_path,
821+
files.keys().map(String::as_str),
822+
)
823+
.await
824+
{
825+
result.error = Some(store.refusal("patch"));
826+
return result;
827+
}
828+
814829
// First, verify all files
815830
for (file_name, file_info) in files_in_order(files) {
816831
// SECURITY: reject any manifest key that would escape the package dir
@@ -3489,4 +3504,162 @@ mod tests {
34893504
Some("store copy /store/pkg@1.0.0_peer failed to patch: boom")
34903505
);
34913506
}
3507+
3508+
/// Lay out one shared store package with `index.js` (original bytes)
3509+
/// and link it into two projects' install dirs, the way pnpm's global
3510+
/// virtual store (#361) and PDM's symlink cache (#332) do. Returns
3511+
/// (root, [project A, project B] package roots, file key, blobs dir,
3512+
/// files, original, patched). The package root is what the crawlers
3513+
/// hand apply: the linked `node_modules/<dep>` for npm, but the
3514+
/// `site-packages` dir for PyPI, whose keys are `<pkg>/<file>`.
3515+
#[cfg(unix)]
3516+
fn shared_store_fixture(
3517+
pdm: bool,
3518+
) -> (
3519+
tempfile::TempDir,
3520+
[std::path::PathBuf; 2],
3521+
String,
3522+
std::path::PathBuf,
3523+
HashMap<String, PatchFileInfo>,
3524+
Vec<u8>,
3525+
Vec<u8>,
3526+
) {
3527+
use crate::patch::shared_store::test_support::{make_pdm_cache_entry, make_pnpm_gvs};
3528+
let root = tempfile::tempdir().unwrap();
3529+
let store_pkg = if pdm {
3530+
make_pdm_cache_entry(root.path())
3531+
} else {
3532+
make_pnpm_gvs(root.path())
3533+
};
3534+
let original = b"original shared bytes".to_vec();
3535+
let patched = b"PATCHED shared bytes".to_vec();
3536+
std::fs::write(store_pkg.join("index.js"), &original).unwrap();
3537+
let roots = ["a", "b"].map(|p| {
3538+
let install_dir = if pdm {
3539+
root.path()
3540+
.join(p)
3541+
.join(".venv/lib/python3.11/site-packages")
3542+
} else {
3543+
root.path().join(p).join("node_modules")
3544+
};
3545+
std::fs::create_dir_all(&install_dir).unwrap();
3546+
let link = install_dir.join(store_pkg.file_name().unwrap());
3547+
std::os::unix::fs::symlink(&store_pkg, &link).unwrap();
3548+
if pdm {
3549+
install_dir
3550+
} else {
3551+
link
3552+
}
3553+
});
3554+
let key = if pdm { "urllib3/index.js" } else { "index.js" }.to_string();
3555+
let blobs = root.path().join("blobs");
3556+
std::fs::create_dir_all(&blobs).unwrap();
3557+
let before_hash = compute_git_sha256_from_bytes(&original);
3558+
let after_hash = compute_git_sha256_from_bytes(&patched);
3559+
std::fs::write(blobs.join(&before_hash), &original).unwrap();
3560+
std::fs::write(blobs.join(&after_hash), &patched).unwrap();
3561+
let mut files = HashMap::new();
3562+
files.insert(
3563+
key.clone(),
3564+
PatchFileInfo {
3565+
before_hash,
3566+
after_hash,
3567+
},
3568+
);
3569+
(root, roots, key, blobs, files, original, patched)
3570+
}
3571+
3572+
/// #361 / #332: agent apply must not write through a package directory
3573+
/// that is a link into a store shared with other projects. It fails
3574+
/// closed (dry run included), and the other project keeps its bytes.
3575+
#[cfg(unix)]
3576+
#[tokio::test]
3577+
async fn test_apply_refuses_shared_store_package_dir() {
3578+
for (pdm, purl) in [
3579+
(false, "pkg:npm/left-pad@1.3.0"),
3580+
(true, "pkg:pypi/urllib3@1.26.18"),
3581+
] {
3582+
let (_root, [a, b], key, blobs, files, original, _patched) = shared_store_fixture(pdm);
3583+
let sources = PatchSources::blobs_only(&blobs);
3584+
for dry_run in [true, false] {
3585+
let result = apply_package_patch(
3586+
purl,
3587+
&a,
3588+
&files,
3589+
&sources,
3590+
None,
3591+
dry_run,
3592+
MismatchPolicy::Warn,
3593+
)
3594+
.await;
3595+
assert!(!result.success, "{purl} dry_run={dry_run}: must refuse");
3596+
let err = result.error.unwrap_or_default();
3597+
assert!(
3598+
err.contains(crate::patch::shared_store::SHARED_STORE_REFUSAL_MARKER),
3599+
"{purl}: {err}"
3600+
);
3601+
assert!(result.files_patched.is_empty());
3602+
}
3603+
assert_eq!(std::fs::read(b.join(&key)).unwrap(), original, "{purl}");
3604+
}
3605+
}
3606+
3607+
/// The same store package already patched (by another project, or by
3608+
/// an apply before this guard existed) is refused too: this project
3609+
/// does not own the shared copy, so it must not record it as patched.
3610+
#[cfg(unix)]
3611+
#[tokio::test]
3612+
async fn test_apply_refuses_already_patched_shared_store_package_dir() {
3613+
let (_root, [a, _b], key, blobs, files, _original, patched) = shared_store_fixture(false);
3614+
std::fs::write(a.join(key), &patched).unwrap();
3615+
let result = apply_package_patch(
3616+
"pkg:npm/left-pad@1.3.0",
3617+
&a,
3618+
&files,
3619+
&PatchSources::blobs_only(&blobs),
3620+
None,
3621+
false,
3622+
MismatchPolicy::Warn,
3623+
)
3624+
.await;
3625+
assert!(!result.success);
3626+
}
3627+
3628+
/// A per-project pnpm store reached through a symlink is still patched.
3629+
#[cfg(unix)]
3630+
#[tokio::test]
3631+
async fn test_apply_patches_through_per_project_pnpm_link() {
3632+
let root = tempfile::tempdir().unwrap();
3633+
let nm = root.path().join("node_modules");
3634+
let real = nm.join(".pnpm/left-pad@1.3.0/node_modules/left-pad");
3635+
std::fs::create_dir_all(&real).unwrap();
3636+
let original = b"original".to_vec();
3637+
let patched = b"patched!".to_vec();
3638+
std::fs::write(real.join("index.js"), &original).unwrap();
3639+
std::os::unix::fs::symlink(&real, nm.join("left-pad")).unwrap();
3640+
let blobs = root.path().join("blobs");
3641+
std::fs::create_dir_all(&blobs).unwrap();
3642+
let after_hash = compute_git_sha256_from_bytes(&patched);
3643+
std::fs::write(blobs.join(&after_hash), &patched).unwrap();
3644+
let mut files = HashMap::new();
3645+
files.insert(
3646+
"index.js".to_string(),
3647+
PatchFileInfo {
3648+
before_hash: compute_git_sha256_from_bytes(&original),
3649+
after_hash,
3650+
},
3651+
);
3652+
let result = apply_package_patch(
3653+
"pkg:npm/left-pad@1.3.0",
3654+
&nm.join("left-pad"),
3655+
&files,
3656+
&PatchSources::blobs_only(&blobs),
3657+
None,
3658+
false,
3659+
MismatchPolicy::Warn,
3660+
)
3661+
.await;
3662+
assert!(result.success, "{:?}", result.error);
3663+
assert_eq!(std::fs::read(real.join("index.js")).unwrap(), patched);
3664+
}
34923665
}

‎crates/socket-patch-core/src/patch/mod.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,5 @@ pub mod package;
88
pub(crate) mod path_safety;
99
pub mod redirect;
1010
pub mod rollback;
11+
pub mod shared_store;
1112
pub mod sidecars;

‎crates/socket-patch-core/src/patch/rollback.rs‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,21 @@ async fn rollback_package_patch_at(
437437
.files_verified
438438
.iter()
439439
.all(|v| v.status == VerifyRollbackStatus::AlreadyOriginal);
440+
// Restoring bytes into a package dir shared with other projects (PDM's
441+
// symlink cache, pnpm's global virtual store) would silently unpatch
442+
// them. Refused whenever a write would happen, dry run included; an
443+
// already-original shared copy needs no write and passes.
444+
if !all_original {
445+
if let Some(store) = crate::patch::shared_store::shared_store_of_patch_dirs(
446+
pkg_path,
447+
files.keys().map(String::as_str),
448+
)
449+
.await
450+
{
451+
result.error = Some(store.refusal("roll back"));
452+
return result;
453+
}
454+
}
440455
if all_original || dry_run {
441456
result.success = true;
442457
return result;
@@ -2505,4 +2520,66 @@ mod tests {
25052520
"an already-original primary must still heal a patched twin"
25062521
);
25072522
}
2523+
2524+
/// #361 / #332: rollback in one project must not restore the original
2525+
/// bytes into a package directory shared with other projects (that
2526+
/// would silently unpatch them). It fails closed, dry run included,
2527+
/// and leaves the shared bytes as they are.
2528+
#[cfg(unix)]
2529+
#[tokio::test]
2530+
async fn test_rollback_refuses_shared_store_package_dir() {
2531+
use crate::patch::shared_store::test_support::{make_pdm_cache_entry, make_pnpm_gvs};
2532+
for (pdm, purl) in [
2533+
(false, "pkg:npm/left-pad@1.3.0"),
2534+
(true, "pkg:pypi/urllib3@1.26.18"),
2535+
] {
2536+
let root = tempfile::tempdir().unwrap();
2537+
let store_pkg = if pdm {
2538+
make_pdm_cache_entry(root.path())
2539+
} else {
2540+
make_pnpm_gvs(root.path())
2541+
};
2542+
let original = b"original shared bytes".to_vec();
2543+
let patched = b"PATCHED shared bytes".to_vec();
2544+
std::fs::write(store_pkg.join("index.js"), &patched).unwrap();
2545+
let install_dir = root.path().join("a").join("install");
2546+
std::fs::create_dir_all(&install_dir).unwrap();
2547+
let link = install_dir.join(store_pkg.file_name().unwrap());
2548+
std::os::unix::fs::symlink(&store_pkg, &link).unwrap();
2549+
// What the crawlers hand rollback: the linked package dir for
2550+
// npm, but `site-packages` (keys `<pkg>/<file>`) for PyPI.
2551+
let (pkg_root, key) = if pdm {
2552+
(install_dir.clone(), "urllib3/index.js")
2553+
} else {
2554+
(link.clone(), "index.js")
2555+
};
2556+
let blobs = root.path().join("blobs");
2557+
std::fs::create_dir_all(&blobs).unwrap();
2558+
let before_hash = compute_git_sha256_from_bytes(&original);
2559+
std::fs::write(blobs.join(&before_hash), &original).unwrap();
2560+
let mut files = HashMap::new();
2561+
files.insert(
2562+
key.to_string(),
2563+
PatchFileInfo {
2564+
before_hash,
2565+
after_hash: compute_git_sha256_from_bytes(&patched),
2566+
},
2567+
);
2568+
for dry_run in [true, false] {
2569+
let result = rollback_package_patch(purl, &pkg_root, &files, &blobs, dry_run).await;
2570+
assert!(!result.success, "{purl} dry_run={dry_run}: must refuse");
2571+
let err = result.error.unwrap_or_default();
2572+
assert!(
2573+
err.contains(crate::patch::shared_store::SHARED_STORE_REFUSAL_MARKER),
2574+
"{purl}: {err}"
2575+
);
2576+
}
2577+
assert_eq!(std::fs::read(store_pkg.join("index.js")).unwrap(), patched);
2578+
2579+
// Already original: nothing to write, so nothing to refuse.
2580+
std::fs::write(store_pkg.join("index.js"), &original).unwrap();
2581+
let result = rollback_package_patch(purl, &pkg_root, &files, &blobs, false).await;
2582+
assert!(result.success, "{purl}: {:?}", result.error);
2583+
}
2584+
}
25082585
}

0 commit comments

Comments
 (0)