Skip to content

Commit c05805e

Browse files
mikolalysenkoclaude
andcommitted
fix(pnpm): bind vendor edits to name@VERSION — multi-version vendoring corrupted the lock
Every "ours" probe in the pnpm backend matched ANY same-name .socket/vendor path, so a project vendoring the same package at several versions (Flowise: three fast-xml-parser, five minimatch patches) had each version's edit treat its siblings' entries as its own stale wiring: override values were clobbered to the wrong tarball and packages/ snapshots rekeys spliced duplicated mapping keys — which pnpm hard-rejects (ERR_PNPM_BROKEN_LOCKFILE), discovered live when repair's reconstruction re-dispatched all versions in sequence. - EditCtx::is_ours / both is_ours_key block probes / the override classification + lock-side mirror check now require the vendor path's leaf to be THIS name-version.tgz (any uuid — stale-uuid refresh unchanged); sibling-version vendored entries are skipped as coexisting. - edit_packages/edit_snapshot_rekey fail closed when BOTH the registry-keyed and our file:-keyed entry exist (a half-edited lock): refusing beats splicing a duplicate key. - Regression tests: multi-version vendor coexistence (per-section duplicate-key audit), integrity-drift refresh stays single-keyed, half-drifted duplicate guard. Live-verified on Flowise end to end: scan --vendor (16/16) → pnpm install --frozen-lockfile → rm -rf .socket → repair (16/16 reconstructed from the lockfile alone) → frozen install again, exit 0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent c92f6b2 commit c05805e

1 file changed

Lines changed: 155 additions & 8 deletions

File tree

crates/socket-patch-core/src/patch/vendor/pnpm_lock.rs

Lines changed: 155 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -550,10 +550,15 @@ impl EditCtx<'_> {
550550
format!("{}@{}", self.name, self.spec)
551551
}
552552

553-
/// Does `value` point into `.socket/vendor/npm/` (ours — any uuid; a
554-
/// stale uuid is rewritten to the current one with `original: None`)?
553+
/// Does `value` point at OUR vendored tarball for THIS name@version
554+
/// (any uuid — a stale uuid is rewritten to the current one with
555+
/// `original: None`)? The leaf binding is load-bearing: a project can
556+
/// vendor the SAME package at several versions, and a name-only match
557+
/// would let one version's edit clobber another's entries.
555558
fn is_ours(&self, value: &str) -> bool {
556-
parse_vendor_path(value).is_some_and(|p| p.eco == "npm")
559+
parse_vendor_path(value).is_some_and(|p| {
560+
p.eco == "npm" && p.leaf == super::npm_common::tgz_rel_leaf(self.name, self.version)
561+
})
557562
}
558563

559564
/// The per-importer `specifier:` spelling: re-relativized for nested
@@ -617,6 +622,15 @@ fn is_vendor_value(value: &str) -> bool {
617622
parse_vendor_path(value).is_some_and(|p| p.eco == "npm")
618623
}
619624

625+
/// A vendor value belonging to THIS `name@version`'s tarball (any uuid).
626+
/// The leaf binding matters: a project can vendor the same package at
627+
/// several versions, and edits must never treat a SIBLING version's
628+
/// override/entry as their own.
629+
fn vendor_value_is_for(value: &str, name: &str, version: &str) -> bool {
630+
parse_vendor_path(value)
631+
.is_some_and(|p| p.eco == "npm" && p.leaf == super::npm_common::tgz_rel_leaf(name, version))
632+
}
633+
620634
/// How the package.json `pnpm.overrides` table relates to the package
621635
/// being vendored. The lock's `overrides:` section must mirror this map
622636
/// key-for-key (pnpm hard-checks the two and fails
@@ -673,13 +687,18 @@ fn classify_pkg_override(
673687
if override_key_name(key) != name {
674688
continue;
675689
}
690+
let value_str = value.as_str().unwrap_or("");
691+
// A SIBLING version's vendored override coexists — not ours to
692+
// touch (and not a conflict): skip it entirely.
693+
if is_vendor_value(value_str) && !vendor_value_is_for(value_str, name, version) {
694+
continue;
695+
}
676696
if found.is_some() {
677697
return Err(format!(
678698
"package.json carries more than one pnpm override for `{name}`; vendoring \
679699
cannot pick one — remove the extras first"
680700
));
681701
}
682-
let value_str = value.as_str().unwrap_or("");
683702
let classified = if key.contains('>') {
684703
None
685704
} else if is_vendor_value(value_str) {
@@ -727,6 +746,10 @@ fn check_lock_override(
727746
if override_key_name(&key) != name {
728747
continue;
729748
}
749+
// A sibling version's vendored override coexists — skip it.
750+
if is_vendor_value(&rest) && !vendor_value_is_for(&rest, name, version) {
751+
continue;
752+
}
730753
if key != effective_key {
731754
return Err(format!(
732755
"{PNPM_LOCK} carries an override key `{key}` for `{name}` that does not \
@@ -1018,7 +1041,7 @@ fn edit_packages(
10181041
} else if block
10191042
.key
10201043
.strip_prefix(&ours_prefix)
1021-
.is_some_and(|rest| parse_vendor_path(rest).is_some_and(|p| p.eco == "npm"))
1044+
.is_some_and(|rest| ctx.is_ours(rest))
10221045
{
10231046
has_ours = true;
10241047
}
@@ -1038,7 +1061,7 @@ fn edit_packages(
10381061
let is_ours_key = block
10391062
.key
10401063
.strip_prefix(&ours_prefix)
1041-
.is_some_and(|rest| parse_vendor_path(rest).is_some_and(|p| p.eco == "npm"));
1064+
.is_some_and(|rest| ctx.is_ours(rest));
10421065
if !is_registry && !is_ours_key {
10431066
i = block.end;
10441067
continue;
@@ -1125,7 +1148,7 @@ fn edit_snapshot_rekey(
11251148
} else if block
11261149
.key
11271150
.strip_prefix(&ours_prefix)
1128-
.is_some_and(|rest| parse_vendor_path(rest).is_some_and(|p| p.eco == "npm"))
1151+
.is_some_and(|rest| ctx.is_ours(rest))
11291152
{
11301153
has_ours = true;
11311154
}
@@ -1144,7 +1167,7 @@ fn edit_snapshot_rekey(
11441167
let is_ours_key = block
11451168
.key
11461169
.strip_prefix(&ours_prefix)
1147-
.is_some_and(|rest| parse_vendor_path(rest).is_some_and(|p| p.eco == "npm"));
1170+
.is_some_and(|rest| ctx.is_ours(rest));
11481171
if !is_registry && !is_ours_key {
11491172
i = block.end;
11501173
continue;
@@ -2697,6 +2720,130 @@ snapshots:
26972720
assert_eq!(fx.read(PACKAGE_JSON).await, pkg_before, "pkg untouched");
26982721
}
26992722

2723+
/// Two VERSIONS of the same package vendored in sequence: each edit
2724+
/// must bind to its own version's entries — a name-only "ours" match
2725+
/// would let the second vendor clobber/rekey the first one's blocks
2726+
/// (live-debugged on Flowise: identical duplicated mapping keys).
2727+
#[tokio::test]
2728+
async fn multi_version_vendor_does_not_clobber_sibling_entries() {
2729+
let fx = fixture_with(P1_BEFORE_PKG, P1_BEFORE_LOCK).await;
2730+
let (r1, e1, _) = expect_done(fx.vendor(false).await);
2731+
assert!(r1.success);
2732+
assert!(e1.is_some());
2733+
let tgz_13 = fx.rel_tgz();
2734+
2735+
// Vendor left-pad@1.2.0 under a DIFFERENT uuid (the `left-pad-old`
2736+
// npm: alias resolves it in the same lock).
2737+
let uuid2 = "22222222-3333-4444-8555-666666666666";
2738+
let installed2 = fx.root().join("node_modules/left-pad-old");
2739+
tokio::fs::create_dir_all(&installed2).await.unwrap();
2740+
tokio::fs::write(
2741+
installed2.join("package.json"),
2742+
br#"{"name":"left-pad","version":"1.2.0"}"#,
2743+
)
2744+
.await
2745+
.unwrap();
2746+
tokio::fs::write(installed2.join("index.js"), ORIG_INDEX)
2747+
.await
2748+
.unwrap();
2749+
let mut record2 = fx.record.clone();
2750+
record2.uuid = uuid2.to_string();
2751+
let blobs = fx.root().join(".socket/blobs");
2752+
let sources = PatchSources::blobs_only(&blobs);
2753+
let outcome = vendor_pnpm(
2754+
"pkg:npm/left-pad@1.2.0",
2755+
&installed2,
2756+
fx.root(),
2757+
&record2,
2758+
&sources,
2759+
"2026-06-09T00:00:00Z",
2760+
false,
2761+
false,
2762+
)
2763+
.await;
2764+
let (r2, e2, _) = expect_done(outcome);
2765+
assert!(r2.success, "{:?}", r2.error);
2766+
assert!(e2.is_some());
2767+
2768+
let lock = fx.read(PNPM_LOCK).await;
2769+
let key13 = format!(" left-pad@file:{tgz_13}:");
2770+
let key12 = format!(" left-pad@file:.socket/vendor/npm/{uuid2}/left-pad-1.2.0.tgz:");
2771+
// Both versions' packages + snapshots blocks exist exactly once
2772+
// each (snapshot entries may be inline `key: {}`).
2773+
for (key, label) in [(&key13, "1.3.0"), (&key12, "1.2.0")] {
2774+
assert_eq!(
2775+
lock.lines().filter(|l| l.starts_with(key.as_str())).count(),
2776+
2, // packages + snapshots
2777+
"{label} entries intact:\n{lock}"
2778+
);
2779+
}
2780+
// No duplicated mapping keys within a section (what pnpm
2781+
// hard-rejects): each section's 2-space keys are unique.
2782+
for section in ["overrides", "packages", "snapshots"] {
2783+
let Some((start, end)) = section_bounds(&split_lines(&lock), section) else {
2784+
continue;
2785+
};
2786+
let lines = split_lines(&lock);
2787+
let mut keys: Vec<String> = lines[start + 1..end]
2788+
.iter()
2789+
.filter_map(|l| parse_key_line(l, 2).map(|(k, _, _)| k))
2790+
.collect();
2791+
let total = keys.len();
2792+
keys.sort_unstable();
2793+
keys.dedup();
2794+
assert_eq!(total, keys.len(), "duplicated keys in {section}:\n{lock}");
2795+
}
2796+
}
2797+
2798+
/// Re-vendor over a wired lock whose recorded integrity DRIFTED (e.g.
2799+
/// the artifact was rebuilt from a differently-shaped source): the
2800+
/// stale-ours refresh must REPLACE the file:-keyed blocks, never
2801+
/// duplicate them.
2802+
#[tokio::test]
2803+
async fn integrity_drift_refresh_never_duplicates_keys() {
2804+
let fx = fixture_with(P1_BEFORE_PKG, P1_BEFORE_LOCK).await;
2805+
let (_, entry, _) = expect_done(fx.vendor(false).await);
2806+
assert!(entry.is_some());
2807+
2808+
// Simulate drift: the lock records a DIFFERENT integrity for OUR
2809+
// file: entry (only) than the tarball the next run will pack.
2810+
let lock = fx.read(PNPM_LOCK).await;
2811+
let drifted = lock
2812+
.lines()
2813+
.map(|l| {
2814+
if l.contains("tarball: file:.socket") {
2815+
l.replace("integrity: sha512-", "integrity: sha512-DRIFT")
2816+
} else {
2817+
l.to_string()
2818+
}
2819+
})
2820+
.collect::<Vec<_>>()
2821+
.join("\n");
2822+
assert_ne!(drifted, lock);
2823+
tokio::fs::write(fx.root().join(PNPM_LOCK), &drifted)
2824+
.await
2825+
.unwrap();
2826+
2827+
let (result, _, _) = expect_done(fx.vendor(false).await);
2828+
assert!(result.success, "{:?}", result.error);
2829+
let healed = fx.read(PNPM_LOCK).await;
2830+
let ours_key = format!(" left-pad@file:{}:", fx.rel_tgz());
2831+
let count = healed.lines().filter(|l| *l == ours_key.as_str()).count();
2832+
assert_eq!(
2833+
count, 1,
2834+
"exactly one file:-keyed packages/snapshots block per section; lock:
2835+
{healed}"
2836+
);
2837+
let snap_count = healed
2838+
.matches(&format!("left-pad@file:{}", fx.rel_tgz()))
2839+
.count();
2840+
assert!(
2841+
!healed.contains("sha512-DRIFT"),
2842+
"drifted integrity healed: {snap_count} refs
2843+
{healed}"
2844+
);
2845+
}
2846+
27002847
#[tokio::test]
27012848
async fn dry_run_writes_nothing() {
27022849
let fx = fixture_with(P1_BEFORE_PKG, P1_BEFORE_LOCK).await;

0 commit comments

Comments
 (0)