From 7954a58bcf3d4df2f37e3149d16bb272475f74ae Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 02:23:15 +0000 Subject: [PATCH 1/3] Start fix for #1619 Assisted-by: Claude Code:claude-opus-5-5 From 25593a0253fbaa5e197332a15fbc7e49ebfed270 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 02:36:00 +0000 Subject: [PATCH 2/3] Keep pnpm 12.0-12.12 scoped tarball on restore pnpm 12.0.0 through 12.12.x record the tarball URL of every package installed from a scoped registry, because they compare it with the default registry's conventional URL instead of the scope's. Rollback, remove and the hosted to vendored takeover dropped that URL from the restored pnpm-lock.yaml, so the lock no longer matched what pnpm wrote. The restore now reads the pnpm minor from the same install record, package.json pin or rush.json pnpmVersion it already uses, and for those releases judges a scoped package's URL against the default registry, both when restoring the entry and when reading the lock's lockfileIncludeTarballUrl evidence. pnpm 11 and 12.13+ are unchanged. Fixes #1619 Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 2 +- .../tests/in_process_redirect.rs | 97 ++++++++ .../src/patch/redirect/upstream/npm.rs | 223 ++++++++++++++---- 3 files changed, 281 insertions(+), 41 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 7bea75a7c..c394b842a 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -1158,7 +1158,7 @@ v5.0 replaces v4's per-purl reverts and whole-ledger reverse replay (`revert_rem * **Scope.** The hosted pins are what lockfile discovery finds — `(purl, patch uuid, files wiring it)`, recognized only on `https://patch.socket.dev` or the `--patch-server-url` / `SOCKET_PATCH_SERVER_URL` origin. A scoped rollback (paths / identifiers / `--ecosystems`) restores exactly the pins in scope; each pin restores or refuses on its own (there is no whole-ledger replay, and a pre-v5 ledger's edits are never replayed). A pin discovery cannot see is out of reach: a lockless cargo `registry = "socket-patch-"` pin, a nuget exact-id mapping with no `packages.lock.json`, a gem wired only in the `Gemfile` (pre-bundler-2.6 mixed state) — restore those files from version control. * **What a restore does.** Every file wiring the pin is rewritten back to the DEFAULT UPSTREAM registry entry for `name@version`, re-resolving whatever the entry pins (tarball URL, integrity, checksum, hashes) from the public registry; only the hosted entries change and every other byte stays the file's own. A pin is **all-or-nothing**: refused in one of its files, it is restored in none of them, so no pin is left half hosted. Nothing reaches disk until every pin has resolved, and `--dry-run` resolves exactly like a wet run — registry lookups included — and skips only the write. Per format: - * **npm family** — `package-lock.json` / `npm-shrinkwrap.json`, `yarn.lock` (classic and berry), `pnpm-lock.yaml` / `shrinkwrap.yaml`, `bun.lock`: resolution + integrity (+ shasum where recorded) from the npm registry's version document (`SOCKET_NPM_REGISTRY`); a yarn berry lock whose settings name another registry reads that registry's document instead — the registry yarn itself resolves the package against (#1017): a scoped package's `npmScopes..npmRegistryServer`, else `YARN_NPM_REGISTRY_SERVER`, else `npmRegistryServer`, each from the closest `.yarnrc.yml` (`YARN_RC_FILENAME`) that sets it among the lock's directory, every parent directory and then the home directory, with a `${VAR:-default}` / `${VAR-default}` reference to an unset variable read as its default, as yarn reads it (a reference to a variable that is set is not expanded — the rc file must not choose which of the process's variables lands in a requested URL — and, like a reference yarn cannot expand or an `npmScopes` written as a flow mapping, falls back to the default registry with `upstream_registry_fallback`), so a mirror's off-path `dist.tarball` keeps its `::__archiveUrl=` binding (a berry entry the hosted pin keyed by its tarball URL also takes the version document's `bin` back, in place of the served tarball's own spelling the pin wrote, as yarn writes the `npm:` entry, #1131), and a pnpm lock whose sibling settings name a registry reads that registry's document — `.npmrc` `registry` (or, for a scoped name, `@scope:registry`); on pnpm 10 a pnpm-workspace.yaml `registries` map instead when present; on pnpm 11+ (or an unknown major) pnpm-workspace.yaml `registries."@scope"` / `registry` / `registries.default` too, ahead of the matching `.npmrc` key — so a mirror's `tarball:` comes back as pnpm recorded it and a URL conventional under that registry stays derived (falling back to the default registry, with `upstream_registry_fallback`, when the mirror can't be read; a value holding an unexpanded `${VAR}` is read as unset). Whether a restored pnpm entry gets its `tarball:` back follows `lockfileIncludeTarballUrl` as the pnpm that wrote the lock read it (#902), from the strongest evidence available: (1) the lock's own unpinned registry resolutions (a bare one proves it off; a URL pnpm could have derived proves it on; pnpm 11+'s env lockfile document does not count); else (2) the settings file the installed pnpm major reads (`node_modules/.modules.yaml` `packageManager`, else package.json `packageManager`; a pre-9 lock or shrinkwrap means pnpm <= 8): `.npmrc` `lockfile-include-tarball-url` on pnpm <= 9, pnpm-workspace.yaml `lockfileIncludeTarballUrl` on pnpm >= 11 (also assumed for a lock carrying an env lockfile document), the workspace file then `.npmrc` on pnpm 10; else (3) pnpm 10's reading. A Rush lock (`common/config/rush/pnpm-lock.yaml` or a subspace lock, with `rush.json` at the Rush root) takes its pnpm major from rush.json `pnpmVersion` instead of tier 2's install record and package.json pin. A 9.0 lock may come from pnpm 9, 10 or 11+, so when tier 3's reading differs from pnpm 9's (`.npmrc` only) or pnpm >= 11's (pnpm-workspace.yaml only) — e.g. `.npmrc` on with the workspace file silent, or the workspace file setting it with `.npmrc` silent or disagreeing — the restore follows pnpm 10 but warns `upstream_pnpm_tarball_setting_guessed` (once per lock, naming the entries); a URL pnpm records anyway (not derivable from the registry) never warns. A `bun.lock` 4-tuple's registry slot is rebuilt the way Bun writes it (#992): `""` for a package from registry.npmjs.org, otherwise the full tarball URL — Bun 1.1.39–1.3.6 read `""` as npmjs whatever the project configures. The registry is the one Bun resolves the package against: a scope's `.npmrc` `@scope:registry` or `bunfig.toml` `[install.scopes]` entry, else `BUN_CONFIG_REGISTRY` / `NPM_CONFIG_REGISTRY`, then the `.npmrc` `registry` or `bunfig.toml` `[install] registry`. Bun reads these from the files beside the lock and from the user's own (#1276): `$XDG_CONFIG_HOME/.npmrc` when it exists, else `~/.npmrc` (never `NPM_CONFIG_USERCONFIG`), and the global bunfig `$XDG_CONFIG_HOME/.bunfig.toml` when `XDG_CONFIG_HOME` is set, else `~/.bunfig.toml`. A key set beside the lock wins over the user's own file of the same kind. Between the two kinds, Bun ≤ 1.3 takes any `.npmrc` over any bunfig and Bun ≥ 1.4 any bunfig over any `.npmrc`. A lockfileVersion-2 `bun.lock` is Bun ≥ 1.4's (Bun 1.3 ignores it); when the kinds disagree on a package of a lockfileVersion-0/1 `bun.lock` (which Bun 1.4 keeps as is) or a `bun.lockb`, the restore refuses that pin rather than guess. Only the conventional token variables (`NPM_TOKEN`, `NODE_AUTH_TOKEN`, `BUN_AUTH_TOKEN`) expand in the project's files; the user's own files expand any variable, as Bun does. Its version document's `dist.tarball` fills the slot, and when it can't be read (`upstream_registry_fallback`) the default registry's conventional URL is re-based on it. The `bun.lockb` takeover restore records the same URL. Side settings: a project `.npmrc` that is exactly `allow-remote=all\n` is deleted once no root npm lock entry is hosted, otherwise a remaining top-level `allow-remote=all` warns `npm_allow_remote_left`; a `pnpm-workspace.yaml` that is exactly the scaffold hosted mode creates is deleted once `pnpm-lock.yaml` is no longer hosted (a keyless file hosted mode spliced the scaffold into gets the spliced lines removed, #1096), otherwise a remaining `trustLockfile: true` warns `pnpm_trust_lockfile_left`. **`bun.lockb` (binary)**: `rollback` and `remove` refuse it (the checkout remedy). The hosted → vendored takeover and the eject DO restore it, since the vendor ledger then records the rebuilt record as its pre-vendor original: the native codec turns each hosted remote-tarball record back into Bun's npm registry record for `name@version` (the registry's `dist.tarball` + `dist.integrity`, the package metadata hash re-derived, the hosted URL string dropped from the string pool). The hosted rewrite keeps the registry record's inactive bytes (padding, semver) in the tarball record, so a lock it wrote comes back byte for byte — early writers' uninitialized padding included; a record without them (an older socket-patch or a Bun re-save) is rebuilt the way Bun writes one, and refused for a prerelease/build version. A lock the hosted rewrite had to normalize is marked in the root package's resolution value bytes (which no Bun reader reads): a binary format 1 lock it promoted to format 2 is demoted back to its exact format-1 bytes (verified by promoting it again, otherwise refused), and a lock whose workspace dependency behaviors it normalized is refused with the `git checkout -- bun.lockb` remedy. + * **npm family** — `package-lock.json` / `npm-shrinkwrap.json`, `yarn.lock` (classic and berry), `pnpm-lock.yaml` / `shrinkwrap.yaml`, `bun.lock`: resolution + integrity (+ shasum where recorded) from the npm registry's version document (`SOCKET_NPM_REGISTRY`); a yarn berry lock whose settings name another registry reads that registry's document instead — the registry yarn itself resolves the package against (#1017): a scoped package's `npmScopes..npmRegistryServer`, else `YARN_NPM_REGISTRY_SERVER`, else `npmRegistryServer`, each from the closest `.yarnrc.yml` (`YARN_RC_FILENAME`) that sets it among the lock's directory, every parent directory and then the home directory, with a `${VAR:-default}` / `${VAR-default}` reference to an unset variable read as its default, as yarn reads it (a reference to a variable that is set is not expanded — the rc file must not choose which of the process's variables lands in a requested URL — and, like a reference yarn cannot expand or an `npmScopes` written as a flow mapping, falls back to the default registry with `upstream_registry_fallback`), so a mirror's off-path `dist.tarball` keeps its `::__archiveUrl=` binding (a berry entry the hosted pin keyed by its tarball URL also takes the version document's `bin` back, in place of the served tarball's own spelling the pin wrote, as yarn writes the `npm:` entry, #1131), and a pnpm lock whose sibling settings name a registry reads that registry's document — `.npmrc` `registry` (or, for a scoped name, `@scope:registry`); on pnpm 10 a pnpm-workspace.yaml `registries` map instead when present; on pnpm 11+ (or an unknown major) pnpm-workspace.yaml `registries."@scope"` / `registry` / `registries.default` too, ahead of the matching `.npmrc` key — so a mirror's `tarball:` comes back as pnpm recorded it and a URL conventional under that registry stays derived (falling back to the default registry, with `upstream_registry_fallback`, when the mirror can't be read; a value holding an unexpanded `${VAR}` is read as unset). pnpm 12.0–12.12 (named by the same install record, package.json pin or rush.json `pnpmVersion` as tier 2 below) compare a scoped package's URL with the default registry's rather than its scope registry's, so they record `tarball:` for every package from a scope registry other than the default one, and the restore gives it back, both in the entry and when reading the lock's evidence below (#1619); pnpm 12.13+ derive it from the scope registry like pnpm <= 11. Whether a restored pnpm entry gets its `tarball:` back follows `lockfileIncludeTarballUrl` as the pnpm that wrote the lock read it (#902), from the strongest evidence available: (1) the lock's own unpinned registry resolutions (a bare one proves it off; a URL pnpm could have derived proves it on; pnpm 11+'s env lockfile document does not count); else (2) the settings file the installed pnpm major reads (`node_modules/.modules.yaml` `packageManager`, else package.json `packageManager`; a pre-9 lock or shrinkwrap means pnpm <= 8): `.npmrc` `lockfile-include-tarball-url` on pnpm <= 9, pnpm-workspace.yaml `lockfileIncludeTarballUrl` on pnpm >= 11 (also assumed for a lock carrying an env lockfile document), the workspace file then `.npmrc` on pnpm 10; else (3) pnpm 10's reading. A Rush lock (`common/config/rush/pnpm-lock.yaml` or a subspace lock, with `rush.json` at the Rush root) takes its pnpm major from rush.json `pnpmVersion` instead of tier 2's install record and package.json pin. A 9.0 lock may come from pnpm 9, 10 or 11+, so when tier 3's reading differs from pnpm 9's (`.npmrc` only) or pnpm >= 11's (pnpm-workspace.yaml only) — e.g. `.npmrc` on with the workspace file silent, or the workspace file setting it with `.npmrc` silent or disagreeing — the restore follows pnpm 10 but warns `upstream_pnpm_tarball_setting_guessed` (once per lock, naming the entries); a URL pnpm records anyway (not derivable from the registry) never warns. A `bun.lock` 4-tuple's registry slot is rebuilt the way Bun writes it (#992): `""` for a package from registry.npmjs.org, otherwise the full tarball URL — Bun 1.1.39–1.3.6 read `""` as npmjs whatever the project configures. The registry is the one Bun resolves the package against: a scope's `.npmrc` `@scope:registry` or `bunfig.toml` `[install.scopes]` entry, else `BUN_CONFIG_REGISTRY` / `NPM_CONFIG_REGISTRY`, then the `.npmrc` `registry` or `bunfig.toml` `[install] registry`. Bun reads these from the files beside the lock and from the user's own (#1276): `$XDG_CONFIG_HOME/.npmrc` when it exists, else `~/.npmrc` (never `NPM_CONFIG_USERCONFIG`), and the global bunfig `$XDG_CONFIG_HOME/.bunfig.toml` when `XDG_CONFIG_HOME` is set, else `~/.bunfig.toml`. A key set beside the lock wins over the user's own file of the same kind. Between the two kinds, Bun ≤ 1.3 takes any `.npmrc` over any bunfig and Bun ≥ 1.4 any bunfig over any `.npmrc`. A lockfileVersion-2 `bun.lock` is Bun ≥ 1.4's (Bun 1.3 ignores it); when the kinds disagree on a package of a lockfileVersion-0/1 `bun.lock` (which Bun 1.4 keeps as is) or a `bun.lockb`, the restore refuses that pin rather than guess. Only the conventional token variables (`NPM_TOKEN`, `NODE_AUTH_TOKEN`, `BUN_AUTH_TOKEN`) expand in the project's files; the user's own files expand any variable, as Bun does. Its version document's `dist.tarball` fills the slot, and when it can't be read (`upstream_registry_fallback`) the default registry's conventional URL is re-based on it. The `bun.lockb` takeover restore records the same URL. Side settings: a project `.npmrc` that is exactly `allow-remote=all\n` is deleted once no root npm lock entry is hosted, otherwise a remaining top-level `allow-remote=all` warns `npm_allow_remote_left`; a `pnpm-workspace.yaml` that is exactly the scaffold hosted mode creates is deleted once `pnpm-lock.yaml` is no longer hosted (a keyless file hosted mode spliced the scaffold into gets the spliced lines removed, #1096), otherwise a remaining `trustLockfile: true` warns `pnpm_trust_lockfile_left`. **`bun.lockb` (binary)**: `rollback` and `remove` refuse it (the checkout remedy). The hosted → vendored takeover and the eject DO restore it, since the vendor ledger then records the rebuilt record as its pre-vendor original: the native codec turns each hosted remote-tarball record back into Bun's npm registry record for `name@version` (the registry's `dist.tarball` + `dist.integrity`, the package metadata hash re-derived, the hosted URL string dropped from the string pool). The hosted rewrite keeps the registry record's inactive bytes (padding, semver) in the tarball record, so a lock it wrote comes back byte for byte — early writers' uninitialized padding included; a record without them (an older socket-patch or a Bun re-save) is rebuilt the way Bun writes one, and refused for a prerelease/build version. A lock the hosted rewrite had to normalize is marked in the root package's resolution value bytes (which no Bun reader reads): a binary format 1 lock it promoted to format 2 is demoted back to its exact format-1 bytes (verified by promoting it again, otherwise refused), and a lock whose workspace dependency behaviors it normalized is refused with the `git checkout -- bun.lockb` remedy. * **vlt** — `vlt-lock.json`: slot [2] from the registry's `dist.integrity`, slot [3] per the lock's own convention (see the vlt hosted-mode contract); every hosted instance of the pin together. * **cargo** — `Cargo.lock` back on crates.io (source + the sparse index's checksum, `SOCKET_CRATES_INDEX`). When the lock already holds a crates.io block for the same `name@version` (a later dependent locked its own copy, #679), the Socket block merges into it instead: it is dropped (with a v1 lock's `[metadata]` line), and dependents' references take the spelling cargo writes, so no duplicate block is left that cargo cannot parse (#863), and no index lookup is needed; every `Cargo.toml` declaration loses its `registry = "socket-patch-"` pin (the shorthand the rewriter produced collapses back); every `[registries.socket-patch-]` block no manifest or lock still references leaves the project cargo config — including a superseded patch generation's block an earlier re-pin left behind (#864). A declaration it cannot unpin refuses. * **golang** — the hosted `replace` and the socket module's go.sum lines go; the upstream module's two go.sum lines come back, hashed from the module proxy (`SOCKET_GOPROXY`, else `GOPROXY` / `GONOPROXY` / `GOPRIVATE` as go reads them) and cross-checked against the checksum database (`SOCKET_GOSUMDB_URL`, else `sum.golang.org` unless `GOSUMDB=off` / `GONOSUMDB` / `GOPRIVATE` say go would not ask it). A `replace` the user had before the hosted run is not recorded anywhere, so the restore lands on the plain upstream module. diff --git a/crates/socket-patch-cli/tests/in_process_redirect.rs b/crates/socket-patch-cli/tests/in_process_redirect.rs index 56fa305e7..36244519f 100644 --- a/crates/socket-patch-cli/tests/in_process_redirect.rs +++ b/crates/socket-patch-cli/tests/in_process_redirect.rs @@ -6189,6 +6189,103 @@ async fn pnpm_rollback_reads_the_scope_registry_for_a_scoped_name() { } } +/// A v9 pnpm lock resolving `SCOPED_NAME@VERSION` with `resolution` next +/// to a bare npmjs `left-pad@1.3.0`, whose bare entry shows +/// `lockfileIncludeTarballUrl` off. +fn scoped_and_npmjs_pnpm_lock(resolution: &str) -> String { + format!( + "lockfileVersion: '9.0'\n\nimporters:\n .:\n dependencies:\n \ + '{SCOPED_NAME}':\n specifier: {VERSION}\n version: {VERSION}\n \ + left-pad:\n specifier: 1.3.0\n version: 1.3.0\n\n\ + packages:\n\n '{SCOPED_NAME}@{VERSION}':\n resolution: {resolution}\n\n \ + left-pad@1.3.0:\n resolution: {{integrity: sha512-LEFTPADleftpad==}}\n\n\ + snapshots:\n\n '{SCOPED_NAME}@{VERSION}': {{}}\n\n left-pad@1.3.0: {{}}\n" + ) +} + +/// #1619: pnpm 12.0–12.12 compare a scoped package's tarball with the +/// conventional URL under the default registry, not the scope's, so they +/// record `tarball:` for every package from a scoped registry even with +/// `lockfileIncludeTarballUrl` off. Rollback and remove of the scoped pin +/// must give it back, while pnpm 11 and 12.13+ (pnpm#16866) keep it bare. +/// The pnpm comes from `.modules.yaml` or package.json's `packageManager`, +/// and the scope registry from `.npmrc` or pnpm-workspace.yaml. +#[tokio::test] +#[serial] +async fn pnpm_12_restore_keeps_the_scoped_registry_tarball() { + let server = MockServer::start().await; + let scope_registry = format!("{}/scoped/", server.uri()); + let conventional = format!("{scope_registry}{SCOPED_NAME}/-/scoped-pkg-{VERSION}.tgz"); + Mock::given(method("GET")) + .and(path_regex(format!( + "^/scoped/@socktest%2[fF]scoped-pkg/{VERSION}$" + ))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "name": SCOPED_NAME, + "version": VERSION, + "dist": { "tarball": conventional, "integrity": "sha512-UPSTREAMupstream==" }, + }))) + .mount(&server) + .await; + let recorded = scoped_and_npmjs_pnpm_lock(&format!( + "{{integrity: sha512-UPSTREAMupstream==, tarball: {conventional}}}" + )); + let bare = scoped_and_npmjs_pnpm_lock("{integrity: sha512-UPSTREAMupstream==}"); + let pinned = scoped_and_npmjs_pnpm_lock(&format!( + "{{integrity: {PATCHED_SHA512}, tarball: {SCOPED_HOSTED_URL}}}" + )); + let npmrc = format!("@socktest:registry={scope_registry}\n"); + let workspace = format!("packages:\n - '.'\nregistries:\n '@socktest': {scope_registry}\n"); + // (pnpm, read from .modules.yaml rather than package.json, scope + // registry in pnpm-workspace.yaml rather than .npmrc, pristine lock) + for (pnpm, modules, in_workspace, pristine) in [ + ("12.12.1", true, false, &recorded), + ("12.12.1", true, true, &recorded), + ("12.0.0", false, false, &recorded), + ("12.11.2", false, true, &recorded), + ("12.13.0", true, false, &bare), + ("12.14.0", false, true, &bare), + ("11.28.6", true, false, &bare), + ] { + for command in ["rollback", "remove"] { + let tmp = tempfile::tempdir().unwrap(); + let package_manager = if modules { + std::fs::create_dir_all(tmp.path().join("node_modules")).unwrap(); + write_pnpm_modules_json(tmp.path(), pnpm); + String::new() + } else { + format!(r#", "packageManager": "pnpm@{pnpm}""#) + }; + std::fs::write( + tmp.path().join("package.json"), + format!( + r#"{{ "name": "consumer", "version": "0.0.0"{package_manager}, "dependencies": {{ "{SCOPED_NAME}": "{VERSION}", "left-pad": "1.3.0" }} }}"# + ), + ) + .unwrap(); + std::fs::write(tmp.path().join("pnpm-lock.yaml"), &pinned).unwrap(); + if in_workspace { + std::fs::write(tmp.path().join("pnpm-workspace.yaml"), &workspace).unwrap(); + } else { + std::fs::write(tmp.path().join(".npmrc"), &npmrc).unwrap(); + } + + let args: &[&str] = if command == "rollback" { + &["rollback"] + } else { + &["remove", "pkg:npm/%40socktest/scoped-pkg@1.0.0"] + }; + let (code, env) = hosted_unwind_json(tmp.path(), &server, "http://patch.test", args); + assert_eq!(code, Some(0), "pnpm {pnpm} {command}: {env:#}"); + let restored = std::fs::read_to_string(tmp.path().join("pnpm-lock.yaml")).unwrap(); + assert_eq!( + &restored, pristine, + "pnpm {pnpm} {command} (modules {modules}, workspace {in_workspace})" + ); + } + } +} + /// #919: when the `.npmrc` mirror cannot be read (here it answers 401), /// pnpm's restore falls back to the default registry's document, exits 0, /// and says so with `upstream_registry_fallback`. diff --git a/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs b/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs index 8ebb95001..169f735b9 100644 --- a/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs @@ -1167,6 +1167,9 @@ struct PnpmTarballPolicy { workspace: Option, npmrc: Option, major: Option, + /// The pnpm that wrote the lock records a scoped-registry package's + /// `tarball:` ([`pnpm_records_scoped_tarballs`]). + scoped_tarballs: bool, /// The `dir/` prefix those settings were read from /// ([`pnpm_settings_prefix`]), for the guess warning. settings_prefix: String, @@ -1181,6 +1184,41 @@ impl PnpmTarballPolicy { name, ) } + + /// Whether pnpm derives `name@version`'s `tarball` itself and so + /// leaves it out of the lock ([`registry_derives_tarball`]); + /// `from_project` says the version document came from + /// [`Self::registry`]. + fn derives_tarball( + &self, + name: &str, + version: &str, + tarball: &str, + from_project: bool, + ) -> bool { + let registry = pnpm_tarball_registry( + self.workspace.as_deref(), + self.npmrc.as_deref(), + self.major, + self.scoped_tarballs, + name, + ); + if registry == self.registry(name) { + return registry_derives_tarball( + registry.as_deref(), + from_project, + name, + version, + tarball, + ); + } + // pnpm compares with the default registry, not the scope one the + // document came from (#1619). + let own = registry + .as_deref() + .is_some_and(|r| non_default_registry(r).is_some()); + registry_derives_tarball(registry.as_deref(), own, name, version, tarball) + } } /// The pnpm major whose settings reading applies to the lock `text`: the @@ -1362,12 +1400,14 @@ fn pnpm_include_tarball( workspace: Option<&str>, npmrc: Option<&str>, pm_major: Option, + scoped_tarballs: bool, is_hosted: impl Fn(&str) -> bool, ) -> PnpmIncludeTarball { use super::super::npmrc::npmrc_top_level_value; let major = pnpm_settings_major(text, pm_major); - let registry = |name: &str| pnpm_lookup_registry(workspace, npmrc, major, name); + let registry = + |name: &str| pnpm_tarball_registry(workspace, npmrc, major, scoped_tarballs, name); if let Some(on) = pnpm_lock_tarball_evidence(text, registry, is_hosted) { return PnpmIncludeTarball { on, guess: None }; } @@ -1458,39 +1498,87 @@ fn pnpm_lock_tarball_evidence( derived.then_some(true) } -/// The major of a `pnpm@` package-manager spec (`.modules.yaml`'s -/// `packageManager`, package.json's corepack pin, `+sha…` suffix allowed). -fn pnpm_spec_major(spec: &str) -> Option { +/// The major and minor of a `pnpm@` package-manager spec +/// (`.modules.yaml`'s `packageManager`, package.json's corepack pin, +/// `+sha…` suffix allowed). +fn pnpm_spec_version(spec: &str) -> Option { let version = spec .trim() .trim_matches(['"', '\'']) .strip_prefix("pnpm@")?; - let major = version.split(['.', '+', '-']).next()?; - major.parse().ok() + pnpm_version(version) +} + +/// A pnpm release's major and minor (`None` when the version has none). +type PnpmVersion = (u32, Option); + +/// The major and minor of a bare pnpm version such as `12.12.1`. +fn pnpm_version(version: &str) -> Option { + let mut parts = version.trim().split(['.', '+', '-']); + let major = parts.next()?.parse().ok()?; + Some((major, parts.next().and_then(|minor| minor.parse().ok()))) +} + +/// pnpm 12.0–12.12 compare a scoped-registry package's tarball with the +/// conventional URL under the default registry rather than the scope's, so +/// they record `tarball:` for every package from a scoped registry other +/// than the default one; 12.13.0 derives it from the scope's (pnpm#16866, +/// #1619). +fn pnpm_records_scoped_tarballs(version: Option) -> bool { + matches!(version, Some((12, Some(minor))) if minor < 13) +} + +/// The registry pnpm compares `name`'s tarball with to decide whether it +/// derives it ([`pnpm_lookup_registry`]): the one it resolves `name` +/// against, except that a pnpm that [`pnpm_records_scoped_tarballs`] +/// compares a scoped name with the default registry, the one its unscoped +/// part resolves against. +fn pnpm_tarball_registry( + workspace: Option<&str>, + npmrc: Option<&str>, + major: Option, + scoped_tarballs: bool, + name: &str, +) -> Option { + let name = match name.split_once('/') { + Some((_, unscoped)) if scoped_tarballs && name.starts_with('@') => unscoped, + _ => name, + }; + pnpm_lookup_registry(workspace, npmrc, major, name) } /// The pnpm major an install record names: `node_modules/.modules.yaml`'s /// `packageManager` (JSON on pnpm 10+, a top-level YAML scalar before). pub(crate) fn modules_yaml_pnpm_major(text: &str) -> Option { + modules_yaml_pnpm_version(text).map(|(major, _)| major) +} + +/// [`modules_yaml_pnpm_major`] with the minor. +fn modules_yaml_pnpm_version(text: &str) -> Option { let text = crate::formats::text::strip_bom(text); if let Ok(value) = serde_json::from_str::(text) { return value .get("packageManager") .and_then(|v| v.as_str()) - .and_then(pnpm_spec_major); + .and_then(pnpm_spec_version); } yaml_top_level_value(text, "packageManager") .as_deref() - .and_then(pnpm_spec_major) + .and_then(pnpm_spec_version) } /// The pnpm major package.json's corepack `packageManager` pins. pub(crate) fn package_json_pnpm_major(text: &str) -> Option { + package_json_pnpm_version(text).map(|(major, _)| major) +} + +/// [`package_json_pnpm_major`] with the minor. +fn package_json_pnpm_version(text: &str) -> Option { serde_json::from_str::(crate::formats::text::strip_bom(text)) .ok()? .get("packageManager")? .as_str() - .and_then(pnpm_spec_major) + .and_then(pnpm_spec_version) } /// The Rush root (`""` or a `dir/` prefix) of a Rush lock path: the common @@ -1513,18 +1601,16 @@ fn rush_lock_root(rel: &str) -> Option<&str> { (root.is_empty() || root.ends_with('/')).then_some(root) } -/// The pnpm major rush.json's `pnpmVersion` names (rush.json is JSONC). -fn rush_json_pnpm_major(text: &str) -> Option { +/// The pnpm major and minor rush.json's `pnpmVersion` names (rush.json is +/// JSONC). +fn rush_json_pnpm_version(text: &str) -> Option { let text = crate::vendor::bun_lock_text::strip_jsonc(crate::formats::text::strip_bom(text)); - serde_json::from_str::(&text) - .ok()? - .get("pnpmVersion")? - .as_str()? - .trim() - .split('.') - .next()? - .parse() - .ok() + pnpm_version( + serde_json::from_str::(&text) + .ok()? + .get("pnpmVersion")? + .as_str()?, + ) } /// The text of `name` next to the lock (`dir_prefix`); `None` when absent @@ -1698,8 +1784,8 @@ async fn pnpm_tarball_policy( None => None, }; let rush = rush_json.is_some(); - let pm_major = if let Some(rush_json) = rush_json.as_deref() { - rush_json_pnpm_major(rush_json) + let pm_version = if let Some(rush_json) = rush_json.as_deref() { + rush_json_pnpm_version(rush_json) } else { // What installed the project (the workspace root's install // record, else the member's own), else what it pins. @@ -1708,24 +1794,27 @@ async fn pnpm_tarball_policy( installed = read_sibling(view, prefix, "node_modules/.modules.yaml") .await .as_deref() - .and_then(modules_yaml_pnpm_major); + .and_then(modules_yaml_pnpm_version); if installed.is_some() || dir_prefix == lock_prefix { break; } } match installed { - Some(major) => Some(major), + Some(version) => Some(version), None => read_sibling(view, &dir_prefix, "package.json") .await .as_deref() - .and_then(package_json_pnpm_major), + .and_then(package_json_pnpm_version), } }; + let pm_major = pm_version.map(|(major, _)| major); + let scoped_tarballs = pnpm_records_scoped_tarballs(pm_version); let include = pnpm_include_tarball( text, workspace.as_deref(), npmrc.as_deref(), pm_major, + scoped_tarballs, |url| ctx.hosted_uuid(url).is_some(), ); PnpmTarballPolicy { @@ -1735,6 +1824,7 @@ async fn pnpm_tarball_policy( workspace, npmrc, major: pnpm_settings_major(text, pm_major), + scoped_tarballs, settings_prefix: dir_prefix, } } @@ -1814,13 +1904,7 @@ pub(crate) async fn restore_pnpm_locks( }; // pnpm records `tarball:` under lockfileIncludeTarballUrl and for // a URL it cannot derive from the registry (#557). - let derived = registry_derives_tarball( - policy.registry(name).as_deref(), - found.from_project, - name, - version, - &dist.tarball, - ); + let derived = policy.derives_tarball(name, version, &dist.tarball, found.from_project); let restored = if policy.always || !derived { resolution.rewrite(integrity, &dist.tarball) } else { @@ -2711,10 +2795,12 @@ mod tests { use super::{ berry_lookup_registry, berry_registry_locator, bun_env_registry, bun_lookup_registry, bun_registry_slot, bun_tarball_url, expand_url_env, modules_yaml_pnpm_major, - non_default_registry, package_json_pnpm_major, pnpm_include_tarball, - pnpm_lock_tarball_evidence, pnpm_lookup_registry, pnpm_tarball_guess_warning, - registry_derives_tarball, rush_json_pnpm_major, rush_lock_root, split_userinfo, - yaml_top_level_value, yarn_expand_env, PnpmTarballGuess, ProjectDist, + modules_yaml_pnpm_version, non_default_registry, package_json_pnpm_major, + package_json_pnpm_version, pnpm_include_tarball, pnpm_lock_tarball_evidence, + pnpm_lookup_registry, pnpm_records_scoped_tarballs, pnpm_spec_version, + pnpm_tarball_guess_warning, pnpm_tarball_registry, registry_derives_tarball, + rush_json_pnpm_version, rush_lock_root, split_userinfo, yaml_top_level_value, + yarn_expand_env, PnpmTarballGuess, ProjectDist, }; use crate::patch::redirect::upstream::client::NpmDist; @@ -3443,11 +3529,11 @@ mod tests { } fn include(text: &str, ws: Option<&str>, rc: Option<&str>, major: Option) -> bool { - pnpm_include_tarball(text, ws, rc, major, |url| url == HOSTED).on + pnpm_include_tarball(text, ws, rc, major, false, |url| url == HOSTED).on } fn guessed(text: &str, ws: Option<&str>, rc: Option<&str>, major: Option) -> bool { - pnpm_include_tarball(text, ws, rc, major, |url| url == HOSTED) + pnpm_include_tarball(text, ws, rc, major, false, |url| url == HOSTED) .guess .is_some() } @@ -3586,8 +3672,11 @@ mod tests { let rush_json = "{\n // \"pnpmVersion\": \"8.0.0\",\n /* c */ \"pnpmVersion\": \"9.15.9\",\n \ \"projects\": [],\n}\n"; - assert_eq!(rush_json_pnpm_major(rush_json), Some(9)); - assert_eq!(rush_json_pnpm_major("{ \"npmVersion\": \"10.0.0\" }"), None); + assert_eq!(rush_json_pnpm_version(rush_json), Some((9, Some(15)))); + assert_eq!( + rush_json_pnpm_version("{ \"npmVersion\": \"10.0.0\" }"), + None + ); } /// #902 tier 1: the lock's unpinned registry resolutions decide. @@ -3722,6 +3811,60 @@ mod tests { assert_eq!(package_json_pnpm_major("not json"), None); } + /// #1619: only pnpm 12.0–12.12 record a scoped-registry package's + /// conventional `tarball:`. + #[test] + fn pnpm_12_before_13_records_scoped_tarballs() { + let records = |spec: &str| pnpm_records_scoped_tarballs(pnpm_spec_version(spec)); + for spec in ["pnpm@12.0.0", "pnpm@12.11.2", "pnpm@12.12.1+sha512.abc"] { + assert!(records(spec), "{spec}"); + } + for spec in [ + "pnpm@11.28.6", + "pnpm@12.13.0", + "pnpm@12.14.0", + "pnpm@13.0.0", + "pnpm@12", + ] { + assert!(!records(spec), "{spec}"); + } + assert_eq!( + modules_yaml_pnpm_version("{\"packageManager\": \"pnpm@12.12.1\"}"), + Some((12, Some(12))) + ); + assert_eq!( + package_json_pnpm_version(r#"{"packageManager":"pnpm@12.13.0"}"#), + Some((12, Some(13))) + ); + } + + /// #1619: a lock whose only registry entry is a scoped one with its + /// conventional URL proves `lockfileIncludeTarballUrl` on for a pnpm + /// that compares it with the scope registry, but shows nothing for pnpm + /// 12.0–12.12, which records it anyway; their scoped URL is not one + /// pnpm derives either, unless the scope registry is the default one. + #[test] + fn pnpm_12_scoped_tarball_is_no_include_tarball_evidence() { + let scoped = "https://s.example/@s/a/-/a-1.0.0.tgz"; + let text = format!( + "lockfileVersion: '9.0'\n\npackages:\n '@s/a@1.0.0':\n \ + resolution: {{integrity: sha512-A==, tarball: {scoped}}}\n\n\ + snapshots:\n '@s/a@1.0.0': {{}}\n" + ); + let rc = "@s:registry=https://s.example/\n"; + let on = |quirk| pnpm_include_tarball(&text, None, Some(rc), Some(12), quirk, |_| false).on; + assert!(on(false)); + assert!(!on(true)); + let derives = |rc: &str, quirk| { + let registry = pnpm_tarball_registry(None, Some(rc), Some(12), quirk, "@s/a"); + registry_derives_tarball(registry.as_deref(), true, "@s/a", "1.0.0", scoped) + }; + assert!(derives(rc, false)); + assert!(!derives(rc, true)); + let both = "registry=https://s.example/\n@s:registry=https://s.example/\n"; + assert!(derives(both, true)); + } + #[test] fn pnpm_reads_the_npmrc_registry_and_scope_registries() { let rc_lookup = |rc, name| pnpm_lookup_registry(None, rc, None, name); From 87750af77bacea1fa52c4f5a802a3c2f72d7c6b9 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 02:51:23 +0000 Subject: [PATCH 3/3] Share pnpm registry check, test the mirror case Fold the two derivability calls in the pnpm restore policy into one, share the rule for whether a pnpm-named registry is the project's own with the lock-evidence check, and unit-test the policy with a non-npmjs default registry next to the scope registry. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/patch/redirect/upstream/npm.rs | 67 ++++++++++++++----- 1 file changed, 49 insertions(+), 18 deletions(-) diff --git a/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs b/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs index 169f735b9..c3e8cf31c 100644 --- a/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs @@ -1196,31 +1196,32 @@ impl PnpmTarballPolicy { tarball: &str, from_project: bool, ) -> bool { - let registry = pnpm_tarball_registry( + let own = self.registry(name); + let compared = pnpm_tarball_registry( self.workspace.as_deref(), self.npmrc.as_deref(), self.major, self.scoped_tarballs, name, ); - if registry == self.registry(name) { - return registry_derives_tarball( - registry.as_deref(), - from_project, - name, - version, - tarball, - ); - } // pnpm compares with the default registry, not the scope one the // document came from (#1619). - let own = registry - .as_deref() - .is_some_and(|r| non_default_registry(r).is_some()); - registry_derives_tarball(registry.as_deref(), own, name, version, tarball) + let from_project = if compared == own { + from_project + } else { + pnpm_registry_is_own(compared.as_deref()) + }; + registry_derives_tarball(compared.as_deref(), from_project, name, version, tarball) } } +/// Whether a registry pnpm names is the project's own rather than the +/// default one, as [`registry_derives_tarball`]'s `from_project` reads it +/// when no version document was read from it. +fn pnpm_registry_is_own(registry: Option<&str>) -> bool { + registry.is_some_and(|r| non_default_registry(r).is_some()) +} + /// The pnpm major whose settings reading applies to the lock `text`: the /// installed / pinned `pm_major`, else 8 for a pre-9 lock or a shrinkwrap /// (only pnpm <= 8 writes those), else 11 for a lock carrying pnpm 11+'s @@ -1487,9 +1488,7 @@ fn pnpm_lock_tarball_evidence( // No document was read: the lock's URL came from the // registry pnpm names, the default one when it names none. let registry = registry(name); - let own = registry - .as_deref() - .is_some_and(|r| non_default_registry(r).is_some()); + let own = pnpm_registry_is_own(registry.as_deref()); derived |= registry_derives_tarball(registry.as_deref(), own, name, version, tarball); } @@ -2800,7 +2799,7 @@ mod tests { pnpm_lookup_registry, pnpm_records_scoped_tarballs, pnpm_spec_version, pnpm_tarball_guess_warning, pnpm_tarball_registry, registry_derives_tarball, rush_json_pnpm_version, rush_lock_root, split_userinfo, yaml_top_level_value, - yarn_expand_env, PnpmTarballGuess, ProjectDist, + yarn_expand_env, PnpmTarballGuess, PnpmTarballPolicy, ProjectDist, }; use crate::patch::redirect::upstream::client::NpmDist; @@ -3865,6 +3864,38 @@ mod tests { assert!(derives(both, true)); } + /// #1619 through the restore's policy: pnpm 12.0–12.12 derive a scoped + /// URL only when it is conventional under the default registry, a + /// configured mirror included, while other pnpm compare with the scope + /// registry the document came from. + #[test] + fn pnpm_12_policy_compares_scoped_tarballs_with_the_default_registry() { + let policy = |npmrc: &str, scoped_tarballs| PnpmTarballPolicy { + always: false, + guess: None, + rush: false, + workspace: None, + npmrc: Some(npmrc.to_string()), + major: Some(12), + scoped_tarballs, + settings_prefix: String::new(), + }; + let rc = "registry=https://m.example/npm/\n@s:registry=https://s.example/\n"; + let scoped = "https://s.example/@s/a/-/a-1.0.0.tgz"; + let mirrored = "https://m.example/npm/@s/a/-/a-1.0.0.tgz"; + let derives = |quirk, name: &str, tarball| { + policy(rc, quirk).derives_tarball(name, "1.0.0", tarball, true) + }; + assert!(derives(false, "@s/a", scoped)); + assert!(!derives(true, "@s/a", scoped)); + assert!(!derives(false, "@s/a", mirrored)); + assert!(derives(true, "@s/a", mirrored)); + // An unscoped name resolves against the default registry anyway. + let plain = "https://m.example/npm/a/-/a-1.0.0.tgz"; + assert!(derives(false, "a", plain)); + assert!(derives(true, "a", plain)); + } + #[test] fn pnpm_reads_the_npmrc_registry_and_scope_registries() { let rc_lookup = |rc, name| pnpm_lookup_registry(None, rc, None, name);