Fix vendored revert deleting through a symlinked vendor dir (#664)

[Mikola Lysenko (mikolalysenko)]

LLM Description written by Claude Code:claude-opus-5-5

Fixes #664

Summary

If two projects share one vendor store through a symlinked .socket/vendor/npm (or any <eco>/<uuid> level), rolling back one project deleted the other project's committed artifacts, and the rollback still exited success. The other project's next frozen or offline install then failed. Vendoring also wrote through the link. Now socket-patch refuses to write, delete or redownload through a linked vendor dir, and says which path is the link.

Root cause

Vendor staging creates .socket/vendor/<eco>/<uuid>/ itself and never writes symlinks, so a linked level isn't ours. vendor::path::sweep_vendor_dirs already applied that rule, but the write and delete paths didn't:

• every backend wrote its unit through the link;
• every backend's revert removed the unit with utils::socket_dir::remove_tree_and_prune, which followed the link (npm, pnpm, yarn classic, yarn berry, bun, vlt, composer, nuget, maven, gem, pypi). So this wasn't yarn classic only;
• vendor::redownload::restore (repair) and vendor::prestage::sweep_stale also went through a linked eco dir.

Changes

• vendor::path::vendor_dir_symlink: one check returning the first linked level among .socket/vendor, <eco> and <uuid>.
• dispatch_vendor_one refuses vendor_dir_symlink_unsupported before any backend write. dispatch_revert_one_opts, which handles vendor --revert, rollback, remove and the vendored→hosted takeover, fails before any lock edit or delete. The lock and ledger stay as they were.
• remove_tree_and_prune refuses to delete when dir or any level between it and stop_dir is a link. This protects every caller even if a dispatcher misses the check. Links at or above .socket (for example /tmp -> /private/tmp) are still allowed.
• For maven and jvm entries the check also covers the JVM vendor trees .socket/vendor/maven2 and .socket/vendor/gradle (one shared constant, jvm::apply::VENDOR_TREES), because a jvm entry is reverted by the maven backend.
• The vendor loop refuses a linked vendor dir before a hosted→vendored takeover restores the upstream entry, so the refusal leaves the hosted patch wired. The service prefetch plan skips those purls too, so no download grant is spent on them. Test: in_process_vendor::hosted_to_vendor_conversion::hosted_then_vendor_over_a_linked_vendor_dir_keeps_the_hosted_wiring.
• redownload::restore refuses before downloading. prestage::sweep_stale and sweep_vendor_dirs skip a linked .socket/vendor / eco dir.
• New row in the CLI_CONTRACT error table for vendor_dir_symlink_unsupported.

A whole-.socket symlink, which also shares state.json, is out of scope, as the issue notes.

Test evidence

Per-issue checklist:

• Vendored yarn classic writes and deletes through a symlinked .socket/vendor/npm dir, so rollback in one project deletes another project's vendored tarballs and breaks its frozen install #664: in_process_rollback_vendored::rollback_never_deletes_through_a_shared_vendor_dir_link and vendor_refuses_a_linked_vendor_dir_before_writing. Both fail on main (rollback exits 0 and the shared tarball is deleted; vendor stages into the link target) and pass with the fix. The real-yarn twin e2e_vendor_yarn_classic_build::yarn_classic_rollback_keeps_a_shared_vendor_store_intact replays the issue's repro: A's rollback fails naming the link, and B's yarn install --frozen-lockfile --offline with an empty cache still installs the patched bytes.
• Unit tests per layer: path::vendor_dir_symlink_finds_the_outermost_linked_level, path::sweep_never_follows_a_symlinked_vendor_dir, socket_dir::remove_tree_and_prune_never_deletes_through_a_link, socket_dir::remove_tree_and_prune_allows_a_linked_project_path, prestage::sweep_never_follows_a_linked_ecosystem_dir, redownload::linked_vendor_dir_refuses_without_downloading.

Commands run locally on 736712a:

• cargo fmt --all -- --check: ok. cargo clippy --workspace --all-features -- -D warnings: ok.
• cargo test -p socket-patch-core --all-features --lib: 4848 passed, 4 failed. The 4 failures are read-only-permission tests that can't fail when run as root (this sandbox is uid 0): copy_tree::relax_loop_must_not_traverse_symlinked_root, vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry, pypi_poetry::wire_write_failure_maps_error_and_leaves_lock_untouched, pypi_requirements::wire_failure_rolls_back_already_written_files. None of them touch the changed code; CI runs as non-root.
• cargo test -p socket-patch-cli --all-features with --lib (834), --test in_process_rollback_vendored (8), --test e2e_vendor_yarn_classic_build (12, SOCKET_PATCH_YARN_E2E_REQUIRED=1, yarn 1.22.22), --test in_process_vendor (104), --test vendor_eject (4), --test vendor_jvm_cli (12), --test scan_vendor_e2e (33): all pass.
• I couldn't build the full cargo test --workspace here: linking every test binary uses up this session's disk allowance. CI runs the full suite.
• Wrapper tests weren't run because npm/, pypi/ and gem/ are unchanged; they only dispatch to the binary.

🤖 Generated with Claude Code

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

Ready for review at f67ff4d501.

• CI: 97/97 green (3 skipped).
• Bugbot: reviewed this head, no findings; the 3 earlier threads on vendor/path.rs are resolved.
• Reviewer focus: remove_tree_and_prune now refuses to delete through any linked level below .socket; this affects every backend revert path.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Codex follow-up review of 356084b59046566a837329989c00cbe18deaa9be: both confirmed data-loss paths are fixed; ready to merge as-is from this review.

The author's correction refuses linked vendor directories before hosted restoration, excludes those targets from service prefetch, and retains the dispatcher backstop. The latest commit removes unrelated formatting changes, leaving 11 files in the PR. All 130 files in that cleanup reproduce the previously validated 9129b34a bytes after formatting; no functional change was introduced. No replacement patch or additional push was needed.

Validation retained from the tested source:

• 194 repository CLI tests passed on 9129b34a: 67 vendor/preview unit, 106 vendor, eight rollback, 12 JVM, and one required real Yarn test. 47 core boundary tests also passed on the equivalent core source.
• The original built-CLI failure now preserves the hosted URL, lock and .npmrc. A separate 12-case scan/get matrix covers wet/dry runs across vendor-root, ecosystem and UUID links, preserving project bytes, links and external sentinels. Dry-run previews keep their documented ledger classification.
• Yarn 1.22.22 verifies that A's rollback refuses the shared-store link and B still installs patched bytes with a fresh cache, frozen lockfile and offline mode.
• Independent review and exact Git hashes cover the current 11 files; the PR diff and merge with main 045d7ec7 are clean. Production/test Clippy passed on the equivalent source with the existing macOS unused-variable allowance and, for test helpers, four existing categories allowed: duplicate_mod, type_complexity, needless_borrow, cloned_ref_to_slice_refs.

411 successful checks, 7 skipped, and 11 successful workflows (1 additional workflow skipped). Bugbot is clear on this commit; no unresolved review threads or new actionable findings. Ready for review has been restored. GitHub still requires the normal human approval before merge.

[Mikola Lysenko (mikolalysenko)]

Fixed the Codex P1 in 03a17de. The vendor loop now refuses a linked vendor dir with vendor_dir_symlink_unsupported before the hosted→vendored takeover restores anything. The service prefetch plan leaves those purls out too, so no download grant is spent on them. Dispatch keeps the same check as a backstop.

Regression test: in_process_vendor::hosted_to_vendor_conversion::hosted_then_vendor_over_a_linked_vendor_dir_keeps_the_hosted_wiring. It runs a hosted scan on a package-lock project, links .socket/vendor/npm, then vendors. It asserts that the takeover never runs, package-lock.json and .npmrc stay byte-identical, and nothing is written into the link's target. It fails on f67ff4d (vendor_takeover_reverted_redirect is emitted) and passes on 03a17de.

Ran locally on 03a17de: fmt, clippy -D warnings, and CLI --lib (834), in_process_vendor (105), in_process_rollback_vendored (8), in_process_vendor_bun_takeover (20), mode_migration_{npm,cargo,vlt} (15/5/24), vendor_jvm_cli (12), vendor_eject (4), scan_vendor_e2e (33): all pass. covgap_commands_vendor has 3 read-only-directory tests that fail only because this sandbox runs as root; they fail identically on f67ff4d, where CI passed them.

To whichever session is preparing its own correction: this push covers the ordering case, so please rebase onto 03a17de rather than force-pushing over it.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

The second Codex point (background prefetch extracting through a linked unit) is covered by the prefetch-plan exclusion in 03a17de. 9129b34 adds vendor::service_plan_tests::the_plan_leaves_out_a_package_whose_vendor_dir_is_linked: three locked composer packages, the middle one's .socket/vendor/composer/<uuid> linked to another project's dir. The plan holds only the other two. The test fails with the exclusion removed (all three planned) and passes with it.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

356084b removes the unrelated formatting churn Codex flagged. Main isn't rustfmt-clean under the pinned 1.93.1 toolchain (cargo fmt --check on main reports 498 hunks), so my cargo fmt --all in 5db2fd3 had reformatted 127 files this fix doesn't touch. Those files are now byte-identical to main, and the 11 files the fix does edit carry only its own hunks. The PR goes from 138 files to 11 files, +751/−14, with no code change; CI doesn't run rustfmt.

Re-ran on 356084b: clippy -D warnings; CLI --lib (835), in_process_vendor (105), in_process_rollback_vendored (8), in_process_vendor_bun_takeover (20), mode_migration_npm (15), vendor_jvm_cli (12), vendor_eject (4), scan_vendor_e2e (33), and e2e_vendor_yarn_classic_build (12, real yarn 1.22.22) all pass. Core --lib: 4848 pass. The 4 that fail are the same read-only-permission tests that fail only as root.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 356084b. Configure here.}