From 15005266983406dd07046bf388a1918bfa0a5613 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Fri, 9 Oct 2026 12:43:19 -0400 Subject: [PATCH 1/2] WIP: Fix berry restore keeping tarball bin paths (#1131) Co-Authored-By: Claude Opus 5.5 (1M context) From 90bdcc288180cd9baa4cb6c2bb6094fc66f4b53b Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Fri, 9 Oct 2026 13:29:39 -0400 Subject: [PATCH 2/2] Restore registry bin paths on berry rollback A hosted yarn berry pin writes the served tarball's own bin: map (./dist/bin/uuid), as yarn does for a tarball locator. The hosted restore (rollback, remove, the hosted half of a vendored takeover) only swapped resolution, checksum and key back, so the restored npm: entry kept the tarball spelling. The lock was not byte-exact, and hardened or --refresh-lockfile installs failed YN0028 for packages like uuid and prettier. The upstream client now reads the version document's bin, and the restore re-renders a tarball-URL pin's bin: from it, the way yarn writes the npm: entry. Fixes #1131 Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/socket-patch-cli/CLI_CONTRACT.md | 2 +- .../tests/in_process_redirect.rs | 102 ++++++++++++++++++ .../src/patch/redirect/upstream/client.rs | 8 ++ .../src/patch/redirect/upstream/npm.rs | 27 ++++- 4 files changed, 134 insertions(+), 5 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index bb4592f4b..154bbe572 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -959,7 +959,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 `.yarnrc.yml` names another `npmRegistryServer` reads that registry's document instead, so a mirror's off-path `dist.tarball` keeps its `::__archiveUrl=` binding, 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`, the `.npmrc` `registry`, then `bunfig.toml` `[install] registry`; 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, 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 `.yarnrc.yml` names another `npmRegistryServer` reads that registry's document instead, 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`, the `.npmrc` `registry`, then `bunfig.toml` `[install] registry`; 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, 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`); 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 799267eeb..2811f1983 100644 --- a/crates/socket-patch-cli/tests/in_process_redirect.rs +++ b/crates/socket-patch-cli/tests/in_process_redirect.rs @@ -6055,3 +6055,105 @@ async fn hosted_json_reports_an_unpinned_row_for_a_granted_patch_nothing_pins() assert_eq!(rows[0]["action"], "unpinned", "{env:#}"); assert_eq!(rows[0]["errorCode"], "redirect_unconfirmed", "{env:#}"); } + +// ── #1131: the restored `npm:` entry takes the registry's `bin:` back ─────── + +/// #1131: a hosted berry pin writes the served tarball's own `bin:` +/// (`./cli.js`, #718), but yarn writes the version document's (`cli.js`, +/// as the registry normalized it on publish) for the `npm:` entry. The +/// restore must put the registry spelling back, or the lock is not +/// byte-exact and hardened `yarn install --immutable` fails YN0028. +#[tokio::test] +#[serial] +async fn yarn_berry_rollback_restores_the_registry_bin_spelling() { + let server = MockServer::start().await; + let tarball = upstream_tarball(); + let integrity = vlt_hosted_common::sha512_sri(&tarball); + let checksum = + socket_patch_core::vendor::test_support::service_fixture::berry_checksum(&tarball, NAME) + .unwrap(); + Mock::given(method("GET")) + .and(path(format!("/npm-registry/{NAME}/{VERSION}"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "name": NAME, + "version": VERSION, + "bin": { NAME: "cli.js", "other-cli": "bin/other.js" }, + "dist": { + "tarball": format!("{}/npm-registry/{NAME}/-/{NAME}-{VERSION}.tgz", server.uri()), + "integrity": integrity, + "shasum": "0".repeat(40), + } + }))) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path(format!("/upstream/npm/{UUID}.json"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "name": NAME, "version": VERSION, "integrity": integrity, "yarnBerry10c0": checksum + }))) + .mount(&server) + .await; + let hosted_url = HOSTED_URL.replace("http://patch.test", &server.uri()); + + let pristine_pkg = format!( + "{{\n \"name\": \"consumer\",\n \"version\": \"0.0.0\",\n \"dependencies\": {{\n \ + \"{NAME}\": \"^{VERSION}\"\n }}\n}}\n" + ); + let pinned_pkg = format!( + "{{\n \"name\": \"consumer\",\n \"version\": \"0.0.0\",\n \"dependencies\": {{\n \ + \"{NAME}\": \"^{VERSION}\"\n }},\n \"resolutions\": {{\n \ + \"{NAME}@npm:^{VERSION}\": \"{hosted_url}\"\n }}\n}}\n" + ); + let lock = |key: &str, resolution: &str, bin: &str, checksum: &str| { + format!( + "# This file is generated by running \"yarn install\" inside your project.\n\ + # Manual changes might be lost - proceed with caution!\n\n\ + __metadata:\n version: 8\n cacheKey: 10c0\n\n\ + \"consumer@workspace:.\":\n version: 0.0.0-use.local\n \ + resolution: \"consumer@workspace:.\"\n dependencies:\n \ + {NAME}: \"npm:^{VERSION}\"\n languageName: unknown\n linkType: soft\n\n\ + \"{key}\":\n version: {VERSION}\n resolution: \"{resolution}\"\n bin:\n \ + {NAME}: {bin}\n other-cli: {bin_other}\n checksum: {checksum}\n \ + languageName: node\n linkType: hard\n", + bin_other = if bin.starts_with("./") { + "./bin/other.js" + } else { + "bin/other.js" + }, + ) + }; + let pristine_lock = lock( + &format!("{NAME}@npm:^{VERSION}"), + &format!("{NAME}@npm:{VERSION}"), + "cli.js", + &checksum, + ); + let pinned_lock = lock( + &format!("{NAME}@{hosted_url}"), + &format!("{NAME}@{hosted_url}"), + "./cli.js", + BERRY_CHECKSUM, + ); + + let tmp = tempfile::tempdir().unwrap(); + std::fs::write(tmp.path().join("package.json"), &pinned_pkg).unwrap(); + std::fs::write(tmp.path().join("yarn.lock"), &pinned_lock).unwrap(); + std::fs::write(tmp.path().join(".yarnrc.yml"), "nodeLinker: node-modules\n").unwrap(); + + let (code, env) = rollback_json_with_origin(tmp.path(), &server, &server.uri()); + assert_eq!(code, Some(0), "rollback: {env:#}"); + assert_eq!( + env["hosted"]["reverted"], + serde_json::json!([PURL]), + "{env:#}" + ); + assert_eq!( + std::fs::read_to_string(tmp.path().join("yarn.lock")).unwrap(), + pristine_lock, + "the restored entry carries the registry's bin spelling" + ); + assert_eq!( + std::fs::read_to_string(tmp.path().join("package.json")).unwrap(), + pristine_pkg + ); +} diff --git a/crates/socket-patch-core/src/patch/redirect/upstream/client.rs b/crates/socket-patch-core/src/patch/redirect/upstream/client.rs index 1c055b800..923cdf0ea 100644 --- a/crates/socket-patch-core/src/patch/redirect/upstream/client.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/client.rs @@ -21,6 +21,11 @@ pub(crate) struct NpmDist { pub integrity: Option, /// The hex sha1 `dist.shasum`. pub shasum: Option, + /// The version document's `bin`, as yarn reads a manifest's + /// ([`crate::formats::yarn::berry_entry::manifest_bin`]): what yarn + /// berry writes in the `bin:` section of the version's `npm:` entry + /// (#1131). Empty when the document declares none. + pub bin: std::collections::BTreeMap, } /// A Go module version's two go.sum hashes. @@ -295,6 +300,7 @@ impl UpstreamClient { tarball, integrity: str_field("integrity"), shasum: str_field("shasum"), + bin: crate::formats::yarn::berry_entry::manifest_bin(&doc), }) } @@ -840,6 +846,7 @@ mod tests { tarball: format!("{}/archive.tgz", server.uri()), integrity: registry_sri, shasum: registry_sha1, + bin: Default::default(), }), ); for _ in 0..2 { @@ -900,6 +907,7 @@ mod tests { tarball: format!("{}/archive.tgz", server.uri()), integrity: Some("sha512-other".into()), shasum: None, + bin: Default::default(), }), ); assert!( 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 c3d0d26e7..700c6d107 100644 --- a/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs @@ -586,6 +586,9 @@ async fn restore_berry( version: String, key: Option, selectors: Vec, + /// A tarball-URL pin, whose `bin:` the pin took from the served + /// tarball's own manifest (#718), not the registry's. + url_pin: bool, } let mut hits: Vec = Vec::new(); for (i, block) in blocks.iter().enumerate() { @@ -681,6 +684,7 @@ async fn restore_berry( version, key: restored_key, selectors, + url_pin, }); } // The registry's `dist.tarball` decides the restored locator: yarn binds @@ -703,6 +707,7 @@ async fn restore_berry( version, key, selectors, + url_pin, } in hits { if result.refused.contains_key(&uuid) { @@ -730,10 +735,9 @@ async fn restore_berry( let Some(dist) = dists.get(&(name.clone(), version.clone())).map(|d| &d.dist) else { continue; }; - let resolution = format!( - " resolution: \"{}\"", - berry_registry_locator(project_registry.as_deref(), &name, &version, &dist.tarball) - ); + let locator = + berry_registry_locator(project_registry.as_deref(), &name, &version, &dist.tarball); + let resolution = format!(" resolution: \"{locator}\""); let mut lines = stanza_lines(&blocks[idx]); if let Some(pinned) = with_body_field(&lines, "resolution", &resolution) { lines = pinned; @@ -747,6 +751,20 @@ async fn restore_berry( lines[0] = format!("{key}:"); moved.push(key); } + // The tarball-URL pin wrote the served tarball's `bin:` (#718); + // yarn writes the version document's for the `npm:` entry (#1131). + if url_pin { + use crate::formats::yarn::berry_entry::{render_pinned_entry, Pin}; + lines = render_pinned_entry( + &lines[1..], + &Pin { + key_line: &lines[0], + resolution: &locator, + checksum: None, + bin: Some(&dist.bin), + }, + ); + } blocks[idx] = lines.join("\n"); if !selectors.is_empty() { if let Some(table) = pkg @@ -2407,6 +2425,7 @@ mod tests { tarball: tarball.to_string(), integrity: None, shasum: None, + bin: Default::default(), }, from_project, };