From 5aeb7d6273a5337b2a2caefc3f9b3f802fd5c2f5 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 12:42:05 +0000 Subject: [PATCH 1/3] Start fix for #1268 Assisted-by: Claude Code:claude-opus-5-5 From cc43bd6dd1d92e85e4291dae4b6a5084b1da913f Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 12:51:32 +0000 Subject: [PATCH 2/3] Warn about trustLockfile after member-lock rollback In a pnpm workspace with sharedWorkspaceLockfile: false, hosted mode pins each member's own pnpm-lock.yaml and adds trustLockfile: true to the root pnpm-workspace.yaml. Rollback and remove only looked at the root lock when deciding whether that setting is still needed, so: - restoring a member lock never warned pnpm_trust_lockfile_left, silently leaving pnpm's lockfile tarball check turned off; - restoring the root lock while a member lock stayed hosted reported the setting as unneeded. The cleanup now treats the root lock and every member lock of the workspace as the locks the setting trusts. Fixes #1268 Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 2 +- .../tests/in_process_redirect_pnpm.rs | 233 ++++++++++++++++++ .../src/patch/redirect/upstream/npm.rs | 18 +- 3 files changed, 250 insertions(+), 3 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 5acc22d01..589c99de7 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -1185,7 +1185,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). 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. + * **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 neither `pnpm-lock.yaml` nor, under `sharedWorkspaceLockfile: false`, any member's own `pnpm-lock.yaml` is hosted any more (#1268; 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_pnpm.rs b/crates/socket-patch-cli/tests/in_process_redirect_pnpm.rs index afc9a0a20..1b44d95a3 100644 --- a/crates/socket-patch-cli/tests/in_process_redirect_pnpm.rs +++ b/crates/socket-patch-cli/tests/in_process_redirect_pnpm.rs @@ -3132,3 +3132,236 @@ async fn hosted_pnpm_file_copy_is_not_attested_by_the_in_run_vex() { "nothing is attested while the file: copy stays unpatched: {statements:?}" ); } + +// ── #1268: trust cleanup over `sharedWorkspaceLockfile: false` member locks ── + +/// Run `rollback --json --yes [extra]` as a hermetic subprocess against the +/// mocked npm registry and parse the envelope back (an in-process `run` +/// prints to the real stdout, which the test can't read). Returns +/// (exit code, run-level warning codes, envelope). +fn rollback_subprocess( + cwd: &Path, + server: &MockServer, + extra: &[&str], +) -> (i32, Vec, serde_json::Value) { + let out = hermetic::binary_command() + .env_remove("npm_config_registry") + .env_remove("NPM_CONFIG_REGISTRY") + .env( + "SOCKET_NPM_REGISTRY", + format!("{}/npm-registry", server.uri()), + ) + .args([ + "rollback", + "--json", + "--yes", + "--patch-server-url", + "http://patch.test", + "--cwd", + cwd.to_str().unwrap(), + ]) + .args(extra) + .output() + .expect("run socket-patch"); + let envelope: serde_json::Value = serde_json::from_slice(&out.stdout).unwrap_or_else(|e| { + panic!( + "rollback --json stdout must be a JSON envelope: {e}\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr) + ) + }); + let codes = envelope["warnings"] + .as_array() + .map(|arr| { + arr.iter() + .filter_map(|w| w["code"].as_str().map(str::to_string)) + .collect() + }) + .unwrap_or_default(); + (out.status.code().unwrap_or(-1), codes, envelope) +} + +/// Serve the npm registry's version document for `name@version`. +async fn mock_registry_version(server: &MockServer, name: &str, version: &str) { + 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, + "dist": { + "tarball": format!("{}/npm-registry/{name}/-/{name}-{version}.tgz", server.uri()), + "integrity": UPSTREAM_SHA512, + } + }))) + .mount(server) + .await; +} + +/// A lockfileVersion 9.0 lock whose `.` importer depends on `name@version`, +/// resolved by `resolution`. +fn single_dep_lock(name: &str, version: &str, resolution: &str) -> String { + format!( + "lockfileVersion: '9.0' + +importers: + .: + dependencies: + {name}: + specifier: {version} + version: {version} + +packages: + {name}@{version}: + resolution: {resolution} + +snapshots: + {name}@{version}: {{}} +" + ) +} + +/// #1268: in a `sharedWorkspaceLockfile: false` workspace hosted mode pins +/// the member's own `packages/a/pnpm-lock.yaml` and adds `trustLockfile: +/// true` to the root `pnpm-workspace.yaml`. Once rollback restores that +/// member lock no lock is hosted any more, so the line the user's file +/// keeps must be reported with `pnpm_trust_lockfile_left`, exactly as for a +/// shared root lock. +#[tokio::test] +#[serial] +async fn issue_1268_rollback_of_a_member_lock_warns_trust_lockfile_left() { + let server = MockServer::start().await; + mock_discovery(&server).await; + mock_reference(&server).await; + mock_registry_version(&server, NAME, VERSION).await; + // pnpm 10+ read the setting from the workspace file; pnpm 9 from `.npmrc`. + for (user_ws, npmrc) in [ + ( + "packages:\n - packages/*\nsharedWorkspaceLockfile: false\n", + None, + ), + ( + "packages:\n - packages/*\n", + Some("shared-workspace-lockfile=false\n"), + ), + ] { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + std::fs::write( + root.join("package.json"), + r#"{ "name": "root", "version": "1.0.0", "private": true }"#, + ) + .unwrap(); + std::fs::write(root.join("pnpm-workspace.yaml"), user_ws).unwrap(); + if let Some(npmrc) = npmrc { + std::fs::write(root.join(".npmrc"), npmrc).unwrap(); + } + let member = root.join("packages/a"); + let pkg = member.join("node_modules").join(NAME); + std::fs::create_dir_all(&pkg).unwrap(); + std::fs::write( + member.join("package.json"), + format!(r#"{{ "name": "a", "version": "1.0.0", "dependencies": {{ "{NAME}": "{VERSION}" }} }}"#), + ) + .unwrap(); + std::fs::write( + pkg.join("package.json"), + format!(r#"{{ "name": "{NAME}", "version": "{VERSION}" }}"#), + ) + .unwrap(); + let pristine = single_dep_lock(NAME, VERSION, &format!("{{integrity: {UPSTREAM_SHA512}}}")); + std::fs::write(member.join("pnpm-lock.yaml"), &pristine).unwrap(); + + let code = run(hosted_args(root, server.uri())).await; + assert_eq!(code, 0, "scan --mode hosted should succeed"); + assert!( + std::fs::read_to_string(member.join("pnpm-lock.yaml")) + .unwrap() + .contains(HOSTED_URL), + "the member lock is pinned" + ); + let ws = std::fs::read_to_string(root.join("pnpm-workspace.yaml")).unwrap(); + assert_eq!(ws, format!("{user_ws}trustLockfile: true\n")); + + let (code, warnings, envelope) = rollback_subprocess(root, &server, &[]); + assert_eq!(code, 0, "rollback restores the member pin: {envelope}"); + assert_eq!( + std::fs::read_to_string(member.join("pnpm-lock.yaml")).unwrap(), + pristine, + "the member lock is restored byte for byte" + ); + assert!( + warnings.contains(&"pnpm_trust_lockfile_left".to_string()), + "the trust line left in the user's file must be reported for {user_ws:?}: {envelope}" + ); + assert_eq!( + std::fs::read_to_string(root.join("pnpm-workspace.yaml")).unwrap(), + ws, + "a user-authored workspace file is never edited by the restore" + ); + } +} + +/// #1268: the trust setting is still needed while any lock of the +/// workspace stays hosted. A scoped rollback that restores the root lock +/// while a member's own lock keeps its hosted pin must not report the +/// setting as unneeded. +#[tokio::test] +#[serial] +async fn issue_1268_member_lock_still_hosted_keeps_trust_without_warning() { + const OTHER: &str = "other-pkg"; + const OTHER_VERSION: &str = "2.0.0"; + const OTHER_HOSTED_URL: &str = "http://patch.test/patch/npm/other-pkg/2.0.0/22222222-2222-4222-8222-222222222222/33333333-3333-4333-8333-333333333333/other-pkg-2.0.0.tgz"; + let server = MockServer::start().await; + mock_registry_version(&server, NAME, VERSION).await; + + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + std::fs::write( + root.join("package.json"), + format!(r#"{{ "name": "root", "version": "1.0.0", "dependencies": {{ "{NAME}": "{VERSION}" }} }}"#), + ) + .unwrap(); + let ws = "packages:\n - packages/*\nsharedWorkspaceLockfile: false\ntrustLockfile: true\n"; + std::fs::write(root.join("pnpm-workspace.yaml"), ws).unwrap(); + let hosted = |url: &str| format!("{{integrity: {PATCHED_SHA512}, tarball: {url}}}"); + std::fs::write( + root.join("pnpm-lock.yaml"), + single_dep_lock(NAME, VERSION, &hosted(HOSTED_URL)), + ) + .unwrap(); + let member = root.join("packages/a"); + std::fs::create_dir_all(&member).unwrap(); + std::fs::write( + member.join("package.json"), + format!(r#"{{ "name": "a", "version": "1.0.0", "dependencies": {{ "{OTHER}": "{OTHER_VERSION}" }} }}"#), + ) + .unwrap(); + let member_lock = single_dep_lock(OTHER, OTHER_VERSION, &hosted(OTHER_HOSTED_URL)); + std::fs::write(member.join("pnpm-lock.yaml"), &member_lock).unwrap(); + + let (code, warnings, envelope) = rollback_subprocess(root, &server, &[PURL]); + assert_eq!( + code, 0, + "the scoped rollback restores the root pin: {envelope}" + ); + assert!( + !std::fs::read_to_string(root.join("pnpm-lock.yaml")) + .unwrap() + .contains(HOSTED_URL), + "the root pin is restored" + ); + assert_eq!( + std::fs::read_to_string(member.join("pnpm-lock.yaml")).unwrap(), + member_lock, + "the out-of-scope member pin is left hosted" + ); + assert!( + !warnings.contains(&"pnpm_trust_lockfile_left".to_string()), + "a hosted member lock still needs the trust setting: {envelope}" + ); + assert_eq!( + std::fs::read_to_string(root.join("pnpm-workspace.yaml")).unwrap(), + ws, + "the workspace file is left as it was" + ); +} 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 3f4b47d3f..3fc3a79e4 100644 --- a/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs @@ -2808,8 +2808,22 @@ pub(crate) async fn cleanup_side_config( } } - let restored_pnpm = view.staged.keys().any(|k| k == "pnpm-lock.yaml"); - if restored_pnpm && !still_hosted(view, &["pnpm-lock.yaml"], ctx).await { + // The root `pnpm-workspace.yaml` trusts the root lock and, under + // `sharedWorkspaceLockfile: false`, every member's own lock (#1268): + // a restore of any of them may leave the setting unneeded, and it is + // still needed while any of them stays hosted. + let mut pnpm_locks = vec!["pnpm-lock.yaml".to_string()]; + if let crate::utils::pnpm_workspace::MemberLocks::PerMember(keys) = + crate::utils::pnpm_workspace::member_locks( + &crate::vendor::lock_inventory::ProjectView::Disk(view.root()), + ) + .await + { + pnpm_locks.extend(keys); + } + let restored_pnpm = view.staged.keys().any(|k| pnpm_locks.contains(k)); + let pnpm_locks: Vec<&str> = pnpm_locks.iter().map(String::as_str).collect(); + if restored_pnpm && !still_hosted(view, &pnpm_locks, ctx).await { const WORKSPACE: &str = "pnpm-workspace.yaml"; if let Ok(Some(ws)) = view.read(WORKSPACE).await { let lines = crate::hosted::guidance::TRUST_SCAFFOLD_LINES; From 0d6551d7b1735942efea1b1bee8405521fd35c91 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 13:12:31 +0000 Subject: [PATCH 3/3] Keep trust cleanup safe for unlisted members Review follow-ups for the member-lock trust cleanup: - when the workspace's member locks cannot be listed, leave the trust setting alone instead of judging it by the root lock only, since a member lock may still be hosted; - only list member locks when the restore actually put a pnpm lock back, so rollbacks of other ecosystems don't walk the workspace; - reuse the new registry mock in the existing pnpm rollback helper. Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/in_process_redirect_pnpm.rs | 13 +----- .../src/patch/redirect/upstream/npm.rs | 46 ++++++++++++------- 2 files changed, 31 insertions(+), 28 deletions(-) diff --git a/crates/socket-patch-cli/tests/in_process_redirect_pnpm.rs b/crates/socket-patch-cli/tests/in_process_redirect_pnpm.rs index 1b44d95a3..fb7f1cbc7 100644 --- a/crates/socket-patch-cli/tests/in_process_redirect_pnpm.rs +++ b/crates/socket-patch-cli/tests/in_process_redirect_pnpm.rs @@ -46,18 +46,7 @@ fn assert_no_ledger(root: &Path) { /// re-resolves `NAME@VERSION` from a mocked npm registry serving /// `UPSTREAM_SHA512`. async fn rollback_hosted(cwd: &Path, server: &MockServer) -> i32 { - 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, - "dist": { - "tarball": format!("{}/npm-registry/{NAME}/-/{NAME}-{VERSION}.tgz", server.uri()), - "integrity": UPSTREAM_SHA512, - } - }))) - .mount(server) - .await; + mock_registry_version(server, NAME, VERSION).await; std::env::set_var( "SOCKET_NPM_REGISTRY", format!("{}/npm-registry", server.uri()), 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 3fc3a79e4..7dad81435 100644 --- a/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs @@ -2772,6 +2772,35 @@ async fn still_hosted(view: &mut View<'_>, rels: &[&str], ctx: &Ctx<'_>) -> bool false } +/// Whether this restore put a pnpm lock the root `pnpm-workspace.yaml` +/// trusts back on the registry and none of those locks is hosted any more. +/// The file trusts the root lock and, under `sharedWorkspaceLockfile: +/// false`, every member's own lock (#1268). A workspace whose member locks +/// cannot be listed fails closed: a member lock may still need the setting. +async fn trusted_pnpm_locks_restored(view: &mut View<'_>, ctx: &Ctx<'_>) -> bool { + use crate::utils::pnpm_workspace::{member_locks, MemberLocks}; + use crate::vendor::lock_inventory::ProjectView; + + if !view + .staged + .keys() + .any(|k| k.rsplit('/').next() == Some("pnpm-lock.yaml")) + { + return false; + } + let mut locks = vec!["pnpm-lock.yaml".to_string()]; + match member_locks(&ProjectView::Disk(view.root())).await { + MemberLocks::Shared => {} + MemberLocks::PerMember(keys) => locks.extend(keys), + MemberLocks::Unresolved(_) => return false, + } + if !view.staged.keys().any(|k| locks.contains(k)) { + return false; + } + let rels: Vec<&str> = locks.iter().map(String::as_str).collect(); + !still_hosted(view, &rels, ctx).await +} + /// Remove the `.npmrc` / pnpm-workspace.yaml a hosted run CREATED (still /// byte-identical to the scaffold) once no lock entry needs it; warn about /// the setting when the file carries other content too (the line may be @@ -2808,22 +2837,7 @@ pub(crate) async fn cleanup_side_config( } } - // The root `pnpm-workspace.yaml` trusts the root lock and, under - // `sharedWorkspaceLockfile: false`, every member's own lock (#1268): - // a restore of any of them may leave the setting unneeded, and it is - // still needed while any of them stays hosted. - let mut pnpm_locks = vec!["pnpm-lock.yaml".to_string()]; - if let crate::utils::pnpm_workspace::MemberLocks::PerMember(keys) = - crate::utils::pnpm_workspace::member_locks( - &crate::vendor::lock_inventory::ProjectView::Disk(view.root()), - ) - .await - { - pnpm_locks.extend(keys); - } - let restored_pnpm = view.staged.keys().any(|k| pnpm_locks.contains(k)); - let pnpm_locks: Vec<&str> = pnpm_locks.iter().map(String::as_str).collect(); - if restored_pnpm && !still_hosted(view, &pnpm_locks, ctx).await { + if trusted_pnpm_locks_restored(view, ctx).await { const WORKSPACE: &str = "pnpm-workspace.yaml"; if let Ok(Some(ws)) = view.read(WORKSPACE).await { let lines = crate::hosted::guidance::TRUST_SCAFFOLD_LINES;