From 93f86d59be9ba511b99e9023b687ee410da28563 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Fri, 14 Aug 2026 06:53:14 -0700 Subject: [PATCH] fix(hosted): refuse pnpm v5/v6 lock keys fail-closed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pnpm redirect grammar only matches lockfileVersion 9 packages keys (name@version, optionally quoted or /-prefixed). pnpm v6 embeds resolved peers in the key itself (/name@1.0.0(peer@2.0.0):) and v5.x uses path-style keys (/name/1.0.0:, peers suffixed _peer@ver), so those entries silently escaped the rewrite. Worst case was fail-open: a v6 lock holding BOTH /pkg@1.0.0: and /pkg@1.0.0(peer@2.0.0): got the plain entry rewritten, the dep was counted redirected, recorded, and attested via --vex, while every dependent resolving through the peered entry still installed the unpatched upstream tarball with no warning at all. Pure-peered v6 and v5.x deps degraded to a bare entry-not-found warning that never named the offending key. Detect v5/v6-grammar keys for the target name@version before rewriting and refuse the dep outright across the whole lock set — nothing rewritten, nothing confirmed — with a redirect_pnpm_unsupported_lock_key warning naming each unmatched key and the lock it lives in, plus the remedy (regenerate with pnpm >=9). v9 locks are unaffected: their peer-suffixed snapshots: keys never start with / and carry no resolution. The docs matrix now states the pnpm v9 constraint for hosted mode. Co-authored-by: Claude Fable 5 --- .../src/patch/redirect/mod.rs | 293 ++++++++++++++++++ docs/ecosystems.md | 8 +- 2 files changed, 300 insertions(+), 1 deletion(-) diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index 0ff40eb5..e329db84 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -691,6 +691,47 @@ fn rewrite_pnpm_lock( // that begin with `@` (`'@scope/name@1.0.0':` — YAML forbids a plain // scalar starting with `@`); v6 keys start with `/` and are unquoted. let key = regex::escape(&fname) + "@" + ®ex::escape(&dep.version); + // Legacy pnpm lock grammars this rewriter cannot repoint: lockfile- + // Version 6 embeds resolved peers in the `packages:` key itself + // (`/name@1.0.0(peer@2.0.0):`) and v5.x separates the version with a + // slash (`/name/1.0.0:`, peers suffixed `_peer@2.0.0`). Both carry + // their own `resolution:` block the pattern below never matches. + // Rewriting AROUND them is fail-open: a v6 lock holding both + // `/pkg@1.0.0:` and `/pkg@1.0.0(peer@2.0.0):` would get the plain + // entry rewritten — confirming and attesting the dep — while every + // dependent resolving through the peered entry still installs the + // unpatched upstream tarball. So when ANY such key exists for this + // dep in ANY lock, refuse the dep outright (no rewrite anywhere), + // naming the unmatched keys. v9 is unaffected: its peer-suffixed + // `snapshots:` keys never start with `/` (and carry no resolution). + let legacy_pat = String::from(r"(?m)^ {2}(/") + + &key + + r"\([^:\n]*|/" + + ®ex::escape(&fname) + + "/" + + ®ex::escape(&dep.version) + + r"(?:[_(][^:\n]*)?):"; + let legacy_re = Regex::new(&legacy_pat).unwrap(); + let mut legacy_keys: Vec = Vec::new(); + for (lock_key, content, _) in &contents { + for caps in legacy_re.captures_iter(content) { + legacy_keys.push(format!("{} in {lock_key}", &caps[1])); + } + } + if !legacy_keys.is_empty() { + result.warnings.push(RewriteWarning { + code: "redirect_pnpm_unsupported_lock_key".into(), + detail: format!( + "{fname}@{} resolves through pnpm v5/v6 lock key(s) the \ + redirect grammar cannot repoint: {}; left unredirected — \ + regenerate the lock with pnpm >=9 (lockfileVersion 9) \ + and re-run", + dep.version, + legacy_keys.join(", ") + ), + }); + continue; + } let pat = String::from(r"(?m)(^ {2}(?:'") + &key + r"'|/?" @@ -3682,4 +3723,256 @@ snapshots: second.edits ); } + + /// pnpm lockfileVersion 6 embeds resolved peers in the `packages:` key + /// itself, so one name@version can appear as BOTH `/pkg@1.0.0:` and + /// `/pkg@1.0.0(peer@2.0.0):`. Rewriting only the plain entry is silent + /// fail-open: the dep is confirmed and attested while every dependent + /// resolving through the peered entry still installs the unpatched + /// upstream tarball. The whole dep must be refused with a warning naming + /// the unmatched key — nothing rewritten, nothing confirmed. + #[test] + fn pnpm_v6_mixed_plain_and_peered_is_refused() { + let lock = "lockfileVersion: '6.0' + +dependencies: + left-pad: + specifier: 1.3.0 + version: 1.3.0 + +packages: + + /left-pad@1.3.0: + resolution: {integrity: sha512-UPSTREAM==} + dev: false + + /left-pad@1.3.0(react@18.2.0): + resolution: {integrity: sha512-UPSTREAM==} + peerDependencies: + react: '*' + dev: false +"; + let mut files = BTreeMap::new(); + files.insert("pnpm-lock.yaml".to_string(), lock.to_string()); + let url = "http://patch.test/left-pad-1.3.0.tgz"; + let overrides = vec![npm_override("left-pad", "1.3.0", url, "sha512-PATCHED==")]; + let r = rewrite_registry_redirect(&files, &overrides); + assert!( + r.files.is_empty() && r.edits.is_empty(), + "a partially-matchable v6 lock must not be rewritten at all: files={:?} edits={:?}", + r.files.keys(), + r.edits + ); + let warning = r + .warnings + .iter() + .find(|w| w.code == "redirect_pnpm_unsupported_lock_key") + .unwrap_or_else(|| panic!("refusal warning expected: {:?}", r.warnings)); + assert!( + warning.detail.contains("/left-pad@1.3.0(react@18.2.0)"), + "warning must name the unmatched peered key: {}", + warning.detail + ); + assert!( + !r.warnings + .iter() + .any(|w| w.code == "redirect_pnpm_entry_not_found"), + "the refusal replaces entry-not-found, not stacks on it: {:?}", + r.warnings + ); + } + + /// A v6 dep resolved ONLY through peer-suffixed keys previously degraded + /// to a bare `entry_not_found`; the refusal must instead name the exact + /// key the grammar cannot repoint so the operator knows the lock (not the + /// dep) is the problem. + #[test] + fn pnpm_v6_pure_peered_key_is_refused_by_name() { + let lock = "lockfileVersion: '6.0' + +packages: + + /@socktest/pkg@1.0.0(react@18.2.0): + resolution: {integrity: sha512-UPSTREAM==} + dev: false +"; + let mut files = BTreeMap::new(); + files.insert("pnpm-lock.yaml".to_string(), lock.to_string()); + let overrides = vec![npm_override( + "@socktest/pkg", + "1.0.0", + "http://patch.test/socktest-pkg-1.0.0.tgz", + "sha512-PATCHED==", + )]; + let r = rewrite_registry_redirect(&files, &overrides); + assert!(r.files.is_empty() && r.edits.is_empty()); + let warning = r + .warnings + .iter() + .find(|w| w.code == "redirect_pnpm_unsupported_lock_key") + .unwrap_or_else(|| panic!("refusal warning expected: {:?}", r.warnings)); + assert!( + warning + .detail + .contains("/@socktest/pkg@1.0.0(react@18.2.0)"), + "warning must name the unmatched key: {}", + warning.detail + ); + assert!( + !r.warnings + .iter() + .any(|w| w.code == "redirect_pnpm_entry_not_found"), + "{:?}", + r.warnings + ); + } + + /// pnpm lockfileVersion 5.x keys are path-style (`/name/version:`, peers + /// suffixed `_peer@ver`) — the rewrite grammar never matches them, so the + /// dep must be refused with the keys named rather than silently reported + /// as a missing entry. + #[test] + fn pnpm_v5_path_style_keys_are_refused_by_name() { + let lock = "lockfileVersion: 5.4 + +specifiers: + left-pad: 1.3.0 + +dependencies: + left-pad: 1.3.0 + +packages: + + /left-pad/1.3.0: + resolution: {integrity: sha512-UPSTREAM==} + dev: false + + /left-pad/1.3.0_react@18.2.0: + resolution: {integrity: sha512-UPSTREAM==} + dev: false +"; + let mut files = BTreeMap::new(); + files.insert("pnpm-lock.yaml".to_string(), lock.to_string()); + let overrides = vec![npm_override( + "left-pad", + "1.3.0", + "http://patch.test/left-pad-1.3.0.tgz", + "sha512-PATCHED==", + )]; + let r = rewrite_registry_redirect(&files, &overrides); + assert!(r.files.is_empty() && r.edits.is_empty()); + let warning = r + .warnings + .iter() + .find(|w| w.code == "redirect_pnpm_unsupported_lock_key") + .unwrap_or_else(|| panic!("refusal warning expected: {:?}", r.warnings)); + assert!( + warning.detail.contains("/left-pad/1.3.0") + && warning.detail.contains("/left-pad/1.3.0_react@18.2.0"), + "warning must name both v5 keys: {}", + warning.detail + ); + assert!( + !r.warnings + .iter() + .any(|w| w.code == "redirect_pnpm_entry_not_found"), + "{:?}", + r.warnings + ); + } + + /// When a dep lives in a rewritable v9 lock AND a legacy lock in the same + /// set (e.g. a Rush nested lock still on pnpm 7), rewriting just the v9 + /// lock would confirm the dep while the legacy lock keeps installing + /// upstream. The refusal must cover the WHOLE set: no lock rewritten. + #[test] + fn pnpm_legacy_lock_in_set_refuses_the_dep_everywhere() { + let v9_lock = "lockfileVersion: '9.0' + +packages: + + left-pad@1.3.0: + resolution: {integrity: sha512-UPSTREAM==} +"; + let v5_lock = "lockfileVersion: 5.4 + +packages: + + /left-pad/1.3.0: + resolution: {integrity: sha512-UPSTREAM==} +"; + let mut files = BTreeMap::new(); + files.insert("pnpm-lock.yaml".to_string(), v9_lock.to_string()); + files.insert( + "common/config/rush/pnpm-lock.yaml".to_string(), + v5_lock.to_string(), + ); + let overrides = vec![npm_override( + "left-pad", + "1.3.0", + "http://patch.test/left-pad-1.3.0.tgz", + "sha512-PATCHED==", + )]; + let r = rewrite_registry_redirect(&files, &overrides); + assert!( + r.files.is_empty() && r.edits.is_empty(), + "no lock in the set may be rewritten while a legacy key survives: files={:?}", + r.files.keys() + ); + let warning = r + .warnings + .iter() + .find(|w| w.code == "redirect_pnpm_unsupported_lock_key") + .unwrap_or_else(|| panic!("refusal warning expected: {:?}", r.warnings)); + assert!( + warning + .detail + .contains("/left-pad/1.3.0 in common/config/rush/pnpm-lock.yaml"), + "warning must name the key AND the lock it lives in: {}", + warning.detail + ); + } + + /// A v6 lock whose target dep has ONLY a plain `/name@version:` key (no + /// peered sibling anywhere) stays rewritable — the refusal must not + /// overreach to every v6 lock. + #[test] + fn pnpm_v6_plain_key_without_peered_sibling_still_rewrites() { + let lock = "lockfileVersion: '6.0' + +packages: + + /left-pad@1.3.0: + resolution: {integrity: sha512-UPSTREAM==} + dev: false + + /other-dep@2.0.0(react@18.2.0): + resolution: {integrity: sha512-OTHER==} + dev: false +"; + let mut files = BTreeMap::new(); + files.insert("pnpm-lock.yaml".to_string(), lock.to_string()); + let url = "http://patch.test/left-pad-1.3.0.tgz"; + let overrides = vec![npm_override("left-pad", "1.3.0", url, "sha512-PATCHED==")]; + let r = rewrite_registry_redirect(&files, &overrides); + let out = r.files.get("pnpm-lock.yaml").unwrap_or_else(|| { + panic!( + "plain v6 key must still be rewritten; warnings={:?}", + r.warnings + ) + }); + assert!( + out.contains(&format!( + " /left-pad@1.3.0:\n resolution: {{integrity: sha512-PATCHED==, tarball: {url}}}" + )), + "{out}" + ); + assert!( + !r.warnings + .iter() + .any(|w| w.code.starts_with("redirect_pnpm_")), + "an unrelated dep's peered key must not trip the refusal: {:?}", + r.warnings + ); + } } diff --git a/docs/ecosystems.md b/docs/ecosystems.md index 1313c4eb..937ae807 100644 --- a/docs/ecosystems.md +++ b/docs/ecosystems.md @@ -14,7 +14,7 @@ The backticked slug in each row is the value `-e`/`--ecosystems` accepts (e.g. | Ecosystem | agent (`--mode agent`) | vendored (`--mode vendored`) | hosted (`--mode hosted`) | |-----------|------------------------|------------------------------|--------------------------| -| npm (`npm`) — pnpm / yarn / berry / bun | ✅ any install layout; `setup` postinstall hook | ✅ five lockfile flavors: package-lock, yarn classic, yarn berry (node-modules linker; PnP refused), pnpm v9, bun `bun.lock` (binary `bun.lockb` refused with a `--save-text-lockfile` pointer). Rush monorepos refused (`vendor_rush_unsupported`) — see [Rush notes](#npm-rush-monorepos) | ✅ package-lock / npm-shrinkwrap, pnpm-lock.yaml, yarn classic, yarn berry, bun — berry and bun carry constraints, see [npm hosted-mode notes](#npm-hosted-mode-notes) | +| npm (`npm`) — pnpm / yarn / berry / bun | ✅ any install layout; `setup` postinstall hook | ✅ five lockfile flavors: package-lock, yarn classic, yarn berry (node-modules linker; PnP refused), pnpm v9, bun `bun.lock` (binary `bun.lockb` refused with a `--save-text-lockfile` pointer). Rush monorepos refused (`vendor_rush_unsupported`) — see [Rush notes](#npm-rush-monorepos) | ✅ package-lock / npm-shrinkwrap, pnpm-lock.yaml (pnpm v9), yarn classic, yarn berry, bun — pnpm, berry, and bun carry constraints, see [npm hosted-mode notes](#npm-hosted-mode-notes) | | PyPI (`pypi`) — uv / poetry / pdm / pipenv / pip | ✅ `.pth` startup hook via `setup` | ✅ five lockfile flavors: uv, poetry, pdm, pipenv (lock rewired, but pipenv doesn't hash-check file entries — `vendor_integrity_unverified` warning; the committed wheel bytes are the protection), and requirements.txt (consumed by pip or `uv pip`) | ✅ requirements.txt + uv.lock. **poetry / pdm / pipenv locks are not rewritten** — use vendored | | Cargo (`cargo`) | ✅ in-place + `.cargo-checksum.json` rewrite (shared registry-cache caveat — see [Cargo: shared registry cache](#cargo-shared-registry-cache)) | ✅ `[patch.crates-io]` path entry | ✅ per-patch sparse registry (`[registries.socket-patch-]` + Cargo.lock source/checksum) | | RubyGems (`gem`) | ✅ Bundler plugin via `setup` | ✅ Gemfile + Gemfile.lock path pair | ✅ per-dep `source` block; the `CHECKSUMS` pin needs bundler ≥ 2.6 (older locks get a `redirect_gem_no_checksums_section` warning) | @@ -35,6 +35,12 @@ The backticked slug in each row is the value `-e`/`--ecosystems` accepts (e.g. ## npm hosted-mode notes +- **pnpm** — lockfileVersion 9 (`pnpm >=9`). Older lock grammars carry `packages:` keys + the rewrite cannot repoint — v6 embeds resolved peers in the key itself + (`/name@1.0.0(peer@2.0.0)`) and v5.x is path-style (`/name/1.0.0`). A dep that + resolves through any such key is refused outright + (`redirect_pnpm_unsupported_lock_key` names the key and the lock), never partially + rewritten: regenerate the lock with pnpm ≥ 9 and re-run. - **yarn berry** — the redirect edits the `yarn.lock` entry only (cacheKey `10c0` / yarn 4), and `.yarnrc.yml`'s `compressionLevel` must stay 0. The node-modules linker is e2e-covered; PnP is untested for hosted — the lock rewrite fires, but PnP's