Skip to content

Commit 09c49a6

Browse files
Fix vendored revert deleting through a symlinked vendor dir (#664) (#666)
* Start fix for #664 Assisted-by: Claude Code:claude-opus-5-5 * Refuse vendoring through a linked vendor dir When two projects shared one vendor store through a symlinked .socket/vendor/<eco> dir, rolling back one project deleted the other project's committed artifacts, and the rollback still reported success. The other project's next frozen install then failed. socket-patch never creates links under .socket/vendor, so a linked .socket/vendor, <eco> or <uuid> dir is not ours. Vendoring now refuses it with vendor_dir_symlink_unsupported before writing, and every vendored revert fails on the same check before it edits a lock or deletes anything. The shared unit-removal helper also refuses to delete through a link, and the stale pre-stage sweep and the vendor dir sweep no longer follow a linked vendor dir. Fixes #664 Assisted-by: Claude Code:claude-opus-5-5 * Stop repair redownloading into a linked store repair's artifact redownload wrote into .socket/vendor/<eco>/<uuid> through a symlinked vendor dir, landing bytes in a store another project may own. It now refuses with vendor_dir_symlink_unsupported before downloading. Also adds a real yarn classic e2e for the two-project shared-store rollback from #664. Assisted-by: Claude Code:claude-opus-5-5 * Expect the shared code for a linked JVM vendor dir The JVM symlink-escape test expected its backend's own build_file_outside_root reason for a linked .socket/vendor. The new up-front gate now refuses that case for every ecosystem with vendor_dir_symlink_unsupported, still before writing anything. The .mvn and module-dir cases keep their JVM reason. Assisted-by: Claude Code:claude-opus-5-5 * Check the maven dirs for jvm vendor entries too A jvm ledger entry is reverted by the maven backend, and maven-family entries can own files in the JVM repository tree under .socket/vendor/maven2. The linked-dir check only looked at .socket/vendor for jvm entries, so a linked maven or maven2 dir got past dispatch and the lock was restored before the delete was refused. The check now covers maven, maven/<uuid> and maven2 for both. Assisted-by: Claude Code:claude-opus-5-5 * Check the gradle vendor tree for linked dirs too Gradle JVM entries write and revert files under .socket/vendor/gradle, so a shared linked gradle dir could still be deleted through by a rollback. The JVM vendor trees are now one constant that both the tree-write allowlist and the linked-dir check read, so maven and jvm entries check maven2 and gradle. Assisted-by: Claude Code:claude-opus-5-5 * Refuse a linked vendor dir before a hosted takeover The linked-dir guard ran inside dispatch, after the vendor loop had already restored a live hosted pin's upstream entry. A scan --mode vendored over a hosted project with a linked .socket/vendor/npm then failed with the hosted patch already unwired: the lock and .npmrc had changed and no vendored artifact replaced them. The loop now refuses with vendor_dir_symlink_unsupported before the takeover, and the service prefetch plan leaves such purls out, so no download grant is spent on them. Dispatch keeps the same check as a backstop. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AHr7bqqwKnGKwkSCCcdWrN * Test that the prefetch plan skips a linked vendor dir Pins the prefetch half of the ordering fix: plan_service_downloads leaves out a package whose vendor unit is a link, so a prefetch running ahead of the loop never stages or extracts an archive through it. The test fails without the exclusion and passes with it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AHr7bqqwKnGKwkSCCcdWrN * Drop the unrelated rustfmt churn from this branch Main is not rustfmt-clean under the pinned 1.93.1 toolchain, so the cargo fmt --all in 5db2fd3 reformatted 127 files this fix never touches, and reformatted untouched code in the 11 files it does. This puts those files back to main's bytes and keeps only the fix's own edits. CI does not run rustfmt, and the code is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AHr7bqqwKnGKwkSCCcdWrN --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6dd293f commit 09c49a6

11 files changed

Lines changed: 751 additions & 14 deletions

File tree

‎crates/socket-patch-cli/CLI_CONTRACT.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1200,6 +1200,7 @@ Every `--json` invocation emits a single JSON object that follows the **unified
12001200
| `eject_rollback_failed` | top-level `errorCode` | vendor eject (v5.0): putting the pre-eject snapshot back failed; the detail names the files to `git checkout --`; exit 1. |
12011201
| `offline_eject_unavailable` | top-level `errorCode` | vendor eject under `--offline` / `SOCKET_OFFLINE` (v5.0): records and registry entries cannot be fetched offline; zero network requests, nothing touched, exit 1. |
12021202
| `hosted_wiring_contested` | top-level `errorCode` (list: warning when it can still list) | rollback / remove / vendor eject / list (v5.0): a lockfile mentions a recognized hosted patch uuid that discovery rejected (or a pin with no lockfile), so the hosted set is not known exactly; refused with nothing touched, exit 1. Remedy: fix or `git checkout` the named lockfile. |
1203+
| `vendor_dir_symlink_unsupported` | `failed` | vendor / scan / get `--mode vendored` (every ecosystem): `.socket/vendor`, `.socket/vendor/<eco>` or the patch's `<uuid>` dir is a symlink or junction. socket-patch creates those directories itself and never writes links, so a linked one is not ours; its target may be another project's vendor store. Refused before any write. The vendored revert (`vendor --revert`, `rollback`, `remove`, the vendored → hosted takeover) fails on the same check with the same detail before it edits a lock or deletes anything, so it never deletes another project's artifacts through the link. The detail names the linked path. Remedy: replace the link with a real directory and re-run. |
12031204
| `vendor_yarn_berry_cache_unsupported` | `failed` | vendor (yarn berry): lock `cacheKey ≠ 10c0` or non-default `.yarnrc.yml` `compressionLevel` — the cache-zip checksum is not reproducible. |
12041205
| `vendor_yarn_berry_mixed_line_endings` | `failed` | vendor (yarn berry): `yarn.lock` or the root `package.json` mixes CRLF and LF line endings (or holds a bare CR) — no single ending can be kept, and yarn rewrites such a file wholesale on its next install (a mixed lock also fails `--immutable`, YN0028). Refused before any write; `yarn install` normalizes the files. A uniformly CRLF pair is vendored in CRLF. A hosted→vendored takeover (`vendor`, `scan`/`get --mode vendored`) raises this — and the berry `vendor_yarn_berry_cache_unsupported` gates — BEFORE restoring the hosted pin's upstream entry (dry run too), so a refused purl stays hosted. |
12051206
| `vendor_override_conflict` | `failed` | vendor (pnpm/yarn-berry): a user-authored override/resolution for the package already exists. |

‎crates/socket-patch-cli/src/commands/vendor.rs‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,14 @@ fn refusal_is_benign(code: &str) -> bool {
113113
matches!(code, "vendor_unsupported_ecosystem" | "already_vendored")
114114
}
115115

116+
/// The `vendor_dir_symlink_unsupported` detail when `purl`'s vendor dir
117+
/// (`.socket/vendor`, its ecosystem dir, or the `uuid` unit) is a link.
118+
fn linked_vendor_dir_refusal(project_root: &Path, purl: &str, uuid: &str) -> Option<String> {
119+
let eco = ecosystem_dir_for_purl(purl)?;
120+
vendor::path::vendor_dir_symlink(project_root, eco, Some(uuid))
121+
.map(|link| vendor::path::vendor_dir_symlink_detail(&link))
122+
}
123+
116124
/// Dispatch one purl to its ecosystem backend. `pkg_path` is the crawler's
117125
/// installed location (site-packages root for pypi, the package dir
118126
/// otherwise), or a fetched artifact the backend materialises only if it
@@ -133,6 +141,16 @@ pub(crate) async fn dispatch_vendor_one(
133141
installed_sites: &vendor::pypi::InstalledSiteListings,
134142
) -> Option<VendorOutcome> {
135143
let eco = ecosystem_dir_for_purl(purl)?;
144+
// Before any backend write: a linked vendor dir is never ours, and the
145+
// unit would land in (and a later revert delete from) its target. The
146+
// vendor loop refuses it earlier still, before a hosted takeover; this
147+
// is the backstop for every other caller.
148+
if let Some(detail) = linked_vendor_dir_refusal(project_root, purl, &record.uuid) {
149+
return Some(VendorOutcome::Refused {
150+
code: "vendor_dir_symlink_unsupported",
151+
detail,
152+
});
153+
}
136154

137155
const SERVICE_ECOSYSTEMS: &[&str] = &[
138156
"npm", "pypi", "cargo", "golang", "composer", "gem", "nuget", "maven",
@@ -238,6 +256,13 @@ pub(crate) async fn dispatch_revert_one_opts(
238256
project_root: &Path,
239257
opts: RevertOpts,
240258
) -> RevertOutcome {
259+
// Before any lock edit or delete: the unit removal would reach through
260+
// a linked vendor dir into another project's artifacts (#664).
261+
if let Some(link) =
262+
vendor::path::vendor_dir_symlink(project_root, &entry.ecosystem, Some(&entry.uuid))
263+
{
264+
return RevertOutcome::failed(vendor::path::vendor_dir_symlink_detail(&link));
265+
}
241266
match entry.ecosystem.as_str() {
242267
"npm" => vendor::npm_flavor::revert_npm_any_opts(entry, project_root, opts).await,
243268
"pypi" => vendor::pypi::revert_pypi_opts(entry, project_root, opts).await,
@@ -1904,6 +1929,10 @@ async fn plan_service_downloads(
19041929
if bun_refusal.is_some_and(|r| r.applies_to(candidate)) {
19051930
continue;
19061931
}
1932+
// The loop refuses a linked vendor dir; no grant on its behalf.
1933+
if linked_vendor_dir_refusal(cwd, candidate, &record.uuid).is_some() {
1934+
continue;
1935+
}
19071936
if takeover_blocked(candidate) {
19081937
continue;
19091938
}
@@ -2377,6 +2406,18 @@ pub(crate) async fn vendor_records_reusing(
23772406
report_vendor_failure(common, candidate, &refusal.detail);
23782407
continue;
23792408
}
2409+
// A linked vendor dir (#664) is refused before the takeover
2410+
// below can restore a live hosted pin's upstream entry: the
2411+
// refusal must leave the hosted patch wired.
2412+
if let Some(detail) = linked_vendor_dir_refusal(&common.cwd, candidate, &record.uuid) {
2413+
has_errors = true;
2414+
env.record(
2415+
PatchEvent::new(PatchAction::Failed, candidate.clone())
2416+
.with_error("vendor_dir_symlink_unsupported", detail.clone()),
2417+
);
2418+
report_vendor_failure(common, candidate, &detail);
2419+
continue;
2420+
}
23802421

23812422
// Cross-mode takeover: vendoring over a LIVE hosted pin must
23822423
// first restore the upstream registry entry (v5 keeps no hosted
@@ -3754,6 +3795,75 @@ mod plan_gate_tests {
37543795
order, and none for the package its backend refuses first"
37553796
);
37563797
}
3798+
3799+
/// A record whose patch dir is a link (#664) is refused by the loop
3800+
/// before dispatch, so the plan leaves it out: a prefetch running ahead
3801+
/// of the loop must never stage or extract an archive through the link.
3802+
#[cfg(unix)]
3803+
#[tokio::test]
3804+
async fn the_plan_leaves_out_a_package_whose_vendor_dir_is_linked() {
3805+
let tmp = tempfile::tempdir().unwrap();
3806+
let root = &tmp.path().join("project");
3807+
std::fs::create_dir_all(root).unwrap();
3808+
std::fs::write(root.join("composer.json"), r#"{"require":{}}"#).unwrap();
3809+
let names = ["psr/cache", "psr/container", "psr/log"];
3810+
let locked: Vec<serde_json::Value> = names
3811+
.iter()
3812+
.map(|name| {
3813+
serde_json::json!({
3814+
"name": name, "version": "1.0.0",
3815+
"dist": {"type": "zip", "url": format!("https://example.invalid/{name}.zip"),
3816+
"reference": "abc", "shasum": ""},
3817+
"type": "library"
3818+
})
3819+
})
3820+
.collect();
3821+
std::fs::write(
3822+
root.join("composer.lock"),
3823+
serde_json::to_vec_pretty(&serde_json::json!({
3824+
"content-hash": "x", "packages": locked, "packages-dev": []
3825+
}))
3826+
.unwrap(),
3827+
)
3828+
.unwrap();
3829+
let mut all_packages: Vec<(String, StagedSource)> = Vec::new();
3830+
let mut records: HashMap<String, PatchRecord> = HashMap::new();
3831+
for (name, uuid) in names.iter().zip([UUID_A, UUID_B, UUID_C]) {
3832+
let purl = format!("pkg:composer/{name}@1.0.0");
3833+
let dir = root.join("vendor").join(name);
3834+
std::fs::create_dir_all(&dir).unwrap();
3835+
all_packages.push((purl.clone(), StagedSource::Installed(dir)));
3836+
records.insert(purl, record(uuid));
3837+
}
3838+
let other = tmp.path().join("other-project-unit");
3839+
std::fs::create_dir_all(&other).unwrap();
3840+
let eco_dir = root.join(".socket/vendor/composer");
3841+
std::fs::create_dir_all(&eco_dir).unwrap();
3842+
std::os::unix::fs::symlink(&other, eco_dir.join(UUID_B)).unwrap();
3843+
3844+
let planned = plan_service_downloads(
3845+
root,
3846+
false,
3847+
&all_packages,
3848+
&HashMap::new(),
3849+
&records,
3850+
&VendorState::default(),
3851+
&HashSet::new(),
3852+
None,
3853+
&|_| false,
3854+
(
3855+
&tokio::sync::OnceCell::new(),
3856+
&vendor::pypi::InstalledSiteListings::default(),
3857+
),
3858+
)
3859+
.await;
3860+
let uuids: Vec<&str> = planned.iter().map(|d| d.uuid.as_str()).collect();
3861+
assert_eq!(
3862+
uuids,
3863+
vec![UUID_A, UUID_C],
3864+
"the linked package is never planned"
3865+
);
3866+
}
37573867
}
37583868

37593869
#[cfg(test)]

‎crates/socket-patch-cli/tests/e2e_vendor_yarn_classic_build.rs‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -858,3 +858,118 @@ fn yarn_classic_detached_scan_vendored_fresh_checkout_manifestless_vex() {
858858
.run(&fresh);
859859
drop(server);
860860
}
861+
862+
/// #664: two yarn classic projects whose `.socket/vendor/npm` links point
863+
/// at one shared store, both vendored with the same patch. `rollback` in
864+
/// project A used to delete the shared uuid dir through the link and
865+
/// report success, so project B's frozen offline install then failed
866+
/// ("Tarball is not in network and can not be located in cache"). The
867+
/// revert now refuses the linked dir before touching anything: A's
868+
/// rollback fails naming the link, and B still installs the patched
869+
/// bytes from the shared store with an empty yarn cache.
870+
#[cfg(unix)]
871+
#[test]
872+
fn yarn_classic_rollback_keeps_a_shared_vendor_store_intact() {
873+
if !require_yarn_classic("e2e_vendor_yarn_classic_build", |c| {
874+
cache_env::isolate(c);
875+
}) {
876+
return;
877+
}
878+
let tmp = tempfile::tempdir().unwrap();
879+
let cache = tmp.path().join("yarn-cache");
880+
let shared = tmp.path().join("mono/shared");
881+
let purl = format!("pkg:npm/{DEP}@{DEP_VERSION}");
882+
let mut projects = Vec::new();
883+
for name in ["a", "b"] {
884+
let proj = tmp.path().join("mono").join(name);
885+
std::fs::create_dir_all(&proj).unwrap();
886+
std::fs::write(
887+
proj.join("package.json"),
888+
format!(
889+
r#"{{"name":"{name}","version":"1.0.0","private":true,"dependencies":{{"{DEP}":"{DEP_VERSION}"}}}}"#
890+
),
891+
)
892+
.unwrap();
893+
let install = corepack(
894+
&proj,
895+
&yarn_classic(),
896+
&["install", "--no-progress"],
897+
&[("YARN_CACHE_FOLDER", cache.to_str().unwrap())],
898+
);
899+
if !install.status.success() {
900+
skip!(
901+
"fixture `yarn install` failed (registry unreachable?):\n{}",
902+
String::from_utf8_lossy(&install.stderr)
903+
);
904+
return;
905+
}
906+
let orig = std::fs::read(proj.join("node_modules").join(DEP).join("index.js")).unwrap();
907+
let patched: Vec<u8> = [MARKER.as_bytes(), orig.as_slice()].concat();
908+
stage_patch(&proj, &purl, "package/index.js", &orig, &patched);
909+
let cwd = proj.to_str().unwrap().to_string();
910+
let (code, stdout, stderr) =
911+
run_socket(&proj, &["vendor", "--json", "--offline", "--cwd", &cwd]);
912+
assert_eq!(code, 0, "vendor {name}:\n{stdout}\n{stderr}");
913+
914+
// Share the store: the first project's unit moves into it, and
915+
// both projects' eco dirs become links to it.
916+
let npm = proj.join(".socket/vendor/npm");
917+
if !shared.exists() {
918+
std::fs::rename(&npm, &shared).unwrap();
919+
} else {
920+
std::fs::remove_dir_all(&npm).unwrap();
921+
}
922+
std::os::unix::fs::symlink(&shared, &npm).unwrap();
923+
projects.push(proj);
924+
}
925+
let shared_tgz = shared.join(UUID).join(format!("{DEP}-{DEP_VERSION}.tgz"));
926+
assert!(
927+
shared_tgz.is_file(),
928+
"sanity: the shared store holds the unit"
929+
);
930+
931+
let a = &projects[0];
932+
let a_lock = std::fs::read(a.join("yarn.lock")).unwrap();
933+
let cwd = a.to_str().unwrap().to_string();
934+
let (code, stdout, stderr) = run_socket(
935+
a,
936+
&["rollback", "--json", "--yes", "--offline", "--cwd", &cwd],
937+
);
938+
assert_eq!(
939+
code, 1,
940+
"A's rollback must fail, not delete:\n{stdout}\n{stderr}"
941+
);
942+
assert!(
943+
stdout.contains(".socket/vendor/npm` is a symlink"),
944+
"the failure names the link:\n{stdout}"
945+
);
946+
assert!(shared_tgz.is_file(), "B's committed tarball survives");
947+
assert_eq!(std::fs::read(a.join("yarn.lock")).unwrap(), a_lock);
948+
949+
// B's fresh, frozen, offline install still resolves the shared tarball.
950+
let b = &projects[1];
951+
std::fs::remove_dir_all(b.join("node_modules")).unwrap();
952+
let empty_cache = tmp.path().join("empty-cache");
953+
std::fs::create_dir_all(&empty_cache).unwrap();
954+
let install = corepack(
955+
b,
956+
&yarn_classic(),
957+
&["install", "--frozen-lockfile", "--offline", "--no-progress"],
958+
&[("YARN_CACHE_FOLDER", empty_cache.to_str().unwrap())],
959+
);
960+
assert!(
961+
install.status.success(),
962+
"B's frozen offline install:\n{}",
963+
String::from_utf8_lossy(&install.stderr)
964+
);
965+
// yarn < 1.7 installs nothing for a `file:` tarball entry (see
966+
// `installs_file_tarballs`); the frozen install succeeding is the proof
967+
// there.
968+
if yarn_classic_vex::installs_file_tarballs(&yarn_classic_vex::yarn_classic_version()) {
969+
let installed = std::fs::read(b.join("node_modules").join(DEP).join("index.js")).unwrap();
970+
assert!(
971+
installed.starts_with(MARKER.as_bytes()),
972+
"B installs the patched bytes"
973+
);
974+
}
975+
}

0 commit comments

Comments
 (0)