From ebf6a74efde7c902ca1f846d48244e71ca86b9c4 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 00:22:08 +0000 Subject: [PATCH 1/5] Start fix for #1094 Assisted-by: Claude Code:claude-opus-5-5 From 4e520c37ecde0c30744487ab503c391e0e0822ed Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 00:32:46 +0000 Subject: [PATCH 2/5] Refuse npm workspace members with a stray lock A workspace member that still held its own package-lock.json or npm-shrinkwrap.json skipped the workspace-member refusal. npm never reads a lock inside a member: it installs every member from the workspace root's lock. So hosted scan/get pinned the ignored member lock and vendored mode vendored into it. Both exited 0 while npm kept installing the unpatched package. Hosted scan/get from such a member now refuse with redirect_workspace_lockfile_elsewhere, naming the root lock and the ignored member lock. Vendored refuses with vendor_lockfile_missing, as it does for a member with no lock. A member that also holds a lock its own manager reads (pnpm, yarn, Bun, vlt, Rush), or a workspace root with no npm lock, keeps the own-lock shortcut. Fixes #1094 (hosted and vendored legs) Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 2 +- .../tests/in_process_redirect_pnpm.rs | 112 ++++++++++- .../src/hosted/governing_root.rs | 181 ++++++++++++++++-- .../src/vendor/npm_flavor.rs | 57 ++++++ 4 files changed, 325 insertions(+), 27 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index b02fa44fa..9c0dbda81 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -1274,7 +1274,7 @@ Every `--json` invocation emits a single JSON object that follows the **unified | `vendor_would_revert_redirect` / `vendor_takeover_reverted_redirect` | `skipped` (advisory event) | vendor / scan / get `--mode vendored` over a hosted pin (every ecosystem, v5.0): dry run — the upstream restore was resolved (registry lookups included) and would succeed (for bun, only after the Bun vendored preflight accepted the lock; a refused lock is previewed as the wet run's `failed ` instead) / wet run — the pin's lock entries were restored to their upstream registry entry before vendoring (mode takeover; detail ` was hosted; restored its upstream registry entry () before vendoring (mode takeover)`), so `vendor --revert` later returns to upstream. Fires on the run that takes over, not on re-runs, and not for a purl whose takeover was rolled back because the backend refused it (see "Takeover reconciliation"). | | `redirect_revert_failed` | `failed` | vendor / scan / get `--mode vendored` (dry and wet): the upstream restore of a hosted pin was refused (`--offline`, a registry that does not answer, a lock shape the restore refuses — for `bun.lockb`, a record the codec cannot rebuild) — detail `cannot vendor over the live hosted pin: cannot restore to its upstream registry entry: ; restore it from version control instead (`git checkout -- `)`; nothing vendored for the purl, hosted wiring left in place, exit 1 `partial_failure`. | | `patch_fetch_failed` (eject) | `failed` | vendor eject (v5.0): a hosted pin's patch record could not be fetched from `…/patches/view/`; the whole eject is refused (`eject_refused`), nothing touched, exit 1. | -| `redirect_pnpm_lockfile_elsewhere` / `redirect_workspace_lockfile_elsewhere` / `cargo_manifest_not_workspace_root` (hosted) | top-level `errorCode` (`status: "error"`) | scan / get `--mode hosted` (v5.0): the project directory is a workspace member whose lock lives in another directory, so the rewriters, which read only the project directory, would pin nothing (pnpm: no npm-family lock here, and the nearest ancestor `pnpm-workspace.yaml` or the project's `lockfile-dir` (`.npmrc`) / `lockfileDir` (`pnpm-workspace.yaml`) puts `pnpm-lock.yaml` elsewhere; npm / yarn / Bun, `redirect_workspace_lockfile_elsewhere`: no npm-family lock here, and the nearest ancestor `package.json` whose `workspaces` (array, or the object form's `packages`) matches the directory holds `package-lock.json`, `npm-shrinkwrap.json`, `yarn.lock`, `bun.lock` or `bun.lockb`; a matching root with none of them that is itself listed by an outer root's `workspaces` hands the check to that root; when a pnpm workspace also governs the directory, the nearer root is named and a tie goes to `redirect_pnpm_lockfile_elsewhere`) or rewrite the member as a lockless project (cargo: the vendored workspace-root check). Refused before any takeover or write, `--dry-run` included; the message names the directory to run from; exit 1. Disk runs only (an in-memory project has no ancestors). | +| `redirect_pnpm_lockfile_elsewhere` / `redirect_workspace_lockfile_elsewhere` / `cargo_manifest_not_workspace_root` (hosted) | top-level `errorCode` (`status: "error"`) | scan / get `--mode hosted` (v5.0): the project directory is a workspace member whose lock lives in another directory, so the rewriters, which read only the project directory, would pin nothing (pnpm: no npm-family lock here, and the nearest ancestor `pnpm-workspace.yaml` or the project's `lockfile-dir` (`.npmrc`) / `lockfileDir` (`pnpm-workspace.yaml`) puts `pnpm-lock.yaml` elsewhere; npm / yarn / Bun, `redirect_workspace_lockfile_elsewhere`: no npm-family lock here, and the nearest ancestor `package.json` whose `workspaces` (array, or the object form's `packages`) matches the directory holds `package-lock.json`, `npm-shrinkwrap.json`, `yarn.lock`, `bun.lock` or `bun.lockb`; a matching root with none of them that is itself listed by an outer root's `workspaces` hands the check to that root; when a pnpm workspace also governs the directory, the nearer root is named and a tie goes to `redirect_pnpm_lockfile_elsewhere`; a directory whose only locks are `package-lock.json` / `npm-shrinkwrap.json` is refused the same way when that root holds `package-lock.json` or `npm-shrinkwrap.json`, because npm never reads a lock inside a workspace member (#1094; vendored refuses it with `vendor_lockfile_missing`)) or rewrite the member as a lockless project (cargo: the vendored workspace-root check). Refused before any takeover or write, `--dry-run` included; the message names the directory to run from; exit 1. Disk runs only (an in-memory project has no ancestors). | | `redirect_pnpm_settings_elsewhere` | top-level `errorCode` (`status: "error"`) | scan / get `--mode hosted`: the project directory is a pnpm workspace member with its own v9 `pnpm-lock.yaml` (`sharedWorkspaceLockfile: false`) and no `pnpm-workspace.yaml` of its own, so its pnpm settings come from the nearest ancestor `pnpm-workspace.yaml`, which pnpm reads alone (a member's own file is ignored). When that file neither carries `trustLockfile: true` nor explicitly sets another value, the trust auto-config has nowhere to go: refused before any takeover or write, `--dry-run` included; the message names the root file to add `trustLockfile: true` to (or `--no-trust-lockfile-config` pins without it); exit 1. Once the root file trusts the lock (or opts out), the member is pinned and no nested `pnpm-workspace.yaml` is created; the `redirect_pnpm_trust_lockfile` warning names the root file. Disk runs only. | | `eject_refused` | top-level `errorCode` (`status: "error"`) | vendor eject (v5.0): a record fetch failed or a pin's upstream restore was refused while planning; nothing was changed, exit 1. | | `eject_planned` | `applied` (reason) | vendor eject `--dry-run` (v5.0): the pin would be restored upstream and vendored; nothing written. | 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 47937792d..626b28033 100644 --- a/crates/socket-patch-cli/tests/in_process_redirect_pnpm.rs +++ b/crates/socket-patch-cli/tests/in_process_redirect_pnpm.rs @@ -56,7 +56,10 @@ async fn rollback_hosted(cwd: &Path, server: &MockServer) -> i32 { }))) .mount(server) .await; - std::env::set_var("SOCKET_NPM_REGISTRY", format!("{}/npm-registry", server.uri())); + std::env::set_var( + "SOCKET_NPM_REGISTRY", + format!("{}/npm-registry", server.uri()), + ); let code = rollback::run(RollbackArgs { targets: Vec::new(), common: socket_patch_cli::args::GlobalArgs { @@ -431,7 +434,10 @@ async fn hosted_bom_lock_and_workspace_read_like_their_plain_twins() { assert_eq!(code, 0, "scan --mode hosted should succeed on a BOM lock"); let lock = std::fs::read_to_string(&lock_path).unwrap(); assert!(lock.starts_with("\u{feff}lockfileVersion:"), "{lock}"); - assert!(lock.contains(HOSTED_URL), "the BOM lock is redirected: {lock}"); + assert!( + lock.contains(HOSTED_URL), + "the BOM lock is redirected: {lock}" + ); let ws_path = tmp.path().join("pnpm-workspace.yaml"); assert_eq!( std::fs::read_to_string(&ws_path).ok().as_deref(), @@ -446,7 +452,10 @@ async fn hosted_bom_lock_and_workspace_read_like_their_plain_twins() { pristine, "rollback restores the BOM lock byte for byte" ); - assert!(!ws_path.exists(), "the auto-created workspace file goes too"); + assert!( + !ws_path.exists(), + "the auto-created workspace file goes too" + ); // A BOM workspace file whose first key is the user's opt-out: left // byte-identical (no duplicate `trustLockfile`), lock still redirected. @@ -472,7 +481,11 @@ async fn hosted_bom_lock_and_workspace_read_like_their_plain_twins() { "the lock is still redirected for {user_ws:?}" ); let ws = std::fs::read_to_string(tmp.path().join("pnpm-workspace.yaml")).unwrap(); - assert_eq!(ws, want.unwrap_or(user_ws), "workspace file for {user_ws:?}"); + assert_eq!( + ws, + want.unwrap_or(user_ws), + "workspace file for {user_ws:?}" + ); assert_eq!(ws.matches("trustLockfile").count(), 1, "{ws:?}"); } } @@ -877,7 +890,11 @@ async fn hosted_pnpm_manifestless_vex_from_lockfile_legacy_ledger_and_api() { ..VexRun::offline() }, ); - assert_eq!(out.code, Some(0), "[{lock_name}] legacy ledger, offline: {out}"); + assert_eq!( + out.code, + Some(0), + "[{lock_name}] legacy ledger, offline: {out}" + ); assert_attested(out.doc(), PURL, UUID, Marker::Redirected, vulns); assert_eq!(api.request_count(), seen); @@ -1669,3 +1686,88 @@ fn assert_refused_workspace_lock_elsewhere( "{case}: nothing written in the member" ); } + +/// #1094: an npm workspace member holding a stray `package-lock.json` / +/// `npm-shrinkwrap.json` of its own (one npm never reads; members install +/// from the root lock) used to skip the #884 refusal. `scan` and `get` +/// pinned the ignored member lock and exited 0. They now refuse, name the +/// root lock and the ignored member lock, and leave both untouched. +#[tokio::test] +#[serial] +async fn hosted_scan_from_npm_member_with_stray_lock_refuses() { + let server = MockServer::start().await; + mock_discovery(&server).await; + mock_reference(&server).await; + mock_view(&server).await; + for (root_lock, member_lock) in [ + ("package-lock.json", "package-lock.json"), + ("package-lock.json", "npm-shrinkwrap.json"), + ("npm-shrinkwrap.json", "package-lock.json"), + ] { + let tmp = tempfile::tempdir().unwrap(); + let member = write_package_json_workspace(tmp.path(), root_lock, false); + let stray = member.join(member_lock); + let stray_text = serde_json::json!({ + "name": "a", "version": "1.0.0", "lockfileVersion": 3, + "packages": { + "": { "name": "a", "version": "1.0.0", "dependencies": { NAME: VERSION } }, + format!("node_modules/{NAME}"): { + "version": VERSION, + "resolved": format!("https://registry.npmjs.org/{NAME}/-/{NAME}-{VERSION}.tgz"), + "integrity": "sha512-orig==" + } + } + }) + .to_string(); + std::fs::write(&stray, &stray_text).unwrap(); + let lock = tmp.path().join(root_lock); + let before = std::fs::read_to_string(&lock).unwrap(); + let case = format!("root {root_lock}, member {member_lock}"); + + for args in [ + vec!["scan", "--mode", "hosted"], + vec!["get", UUID, "--mode", "hosted"], + ] { + let out = scrubbed_cli() + .args(&args) + .args([ + "--json", + "--yes", + "--cwd", + member.to_str().unwrap(), + "--api-url", + &server.uri(), + "--org", + ORG, + "--api-token", + "fake", + ]) + .output() + .expect("run socket-patch"); + let doc: serde_json::Value = serde_json::from_slice(&out.stdout).unwrap_or_else(|e| { + panic!( + "{case} {args:?}: output is not JSON ({e}):\n{}\n{}", + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr) + ) + }); + let case = format!("{case} {args:?}"); + assert_refused_workspace_lock_elsewhere( + &case, + out.status.code(), + &doc, + &lock, + &before, + &member, + ); + let message = doc["error"].as_str().unwrap_or_default(); + assert!(message.contains(member_lock), "{case}: {message}"); + assert_eq!( + std::fs::read_to_string(&stray).unwrap(), + stray_text, + "{case}: the stray member lock is untouched" + ); + assert!(!member.join(".npmrc").exists(), "{case}"); + } + } +} diff --git a/crates/socket-patch-core/src/hosted/governing_root.rs b/crates/socket-patch-core/src/hosted/governing_root.rs index c646adc4c..5179d72ed 100644 --- a/crates/socket-patch-core/src/hosted/governing_root.rs +++ b/crates/socket-patch-core/src/hosted/governing_root.rs @@ -88,7 +88,7 @@ pub async fn refusal( } if candidates.iter().any(|c| c.dep.ecosystem == "npm") { let workspace = if has_own_npm_family_lock(root) { - None + npm_member_stray_lock_refusal(root).await } else { package_json_workspace_refusal(root).await }; @@ -276,6 +276,95 @@ const WORKSPACE_ROOT_LOCKS: [&str; 5] = [ /// governing root with the refusal, so [`refusal`] can weigh it against /// the pnpm check (the nearer root wins; a tie goes to pnpm's message). async fn package_json_workspace_refusal(root: &Path) -> Option<(PathBuf, Refusal)> { + let (ancestor, locks) = package_json_workspace_root(root).await?; + let refusal = Refusal { + code: WORKSPACE_LOCKFILE_ELSEWHERE.to_string(), + message: format!( + "{} is a workspace member with no lockfile of its own: the workspace \ + root {} lists it under \"workspaces\" and installs it from {}, which a \ + hosted run here cannot see; run socket-patch from {} (the workspace \ + root); nothing was written", + root.display(), + ancestor.display(), + join_paths(&ancestor, &locks), + ancestor.display() + ), + }; + Some((ancestor, refusal)) +} + +/// #1094: the project directory holds only npm locks (`package-lock.json`, +/// `npm-shrinkwrap.json`) and is a member of a `package.json` workspace +/// whose root holds an npm lock. npm installs every workspace member from +/// the root's lock and never reads a lock inside the member (a stray one, +/// typically left behind when the package moved into the monorepo), so a +/// run here would pin or vendor a lock npm ignores and report success. +/// +/// A member that also holds a lock its own manager reads (pnpm, yarn, Bun, +/// vlt, or a Rush repo) keeps the own-lock shortcut: pnpm and vlt ignore +/// `package.json` workspaces, and yarn berry treats a nested `yarn.lock` +/// as a separate project. A root with no npm lock is left alone too. +/// +/// Returns the workspace root with a one-line detail naming both locks, +/// `None` otherwise. +pub(crate) async fn npm_member_stray_lock(root: &Path) -> Option<(PathBuf, String)> { + let own: Vec<&str> = NPM_LOCKS + .iter() + .copied() + .filter(|name| root.join(name).exists()) + .collect(); + let other_own = OWN_LOCKS + .iter() + .chain(std::iter::once(&VLT_LOCK)) + .any(|name| root.join(name).exists()) + || root.join("rush.json").exists(); + if own.is_empty() || other_own { + return None; + } + let (ancestor, locks) = package_json_workspace_root(root).await?; + let npm_locks: Vec<&str> = locks + .into_iter() + .filter(|name| NPM_LOCKS.contains(name)) + .collect(); + if npm_locks.is_empty() { + return None; + } + let detail = format!( + "{} is a member of the npm workspace rooted at {}: npm installs it from {} and \ + ignores its own {}, so a lock rewritten here would never be installed", + root.display(), + ancestor.display(), + join_paths(&ancestor, &npm_locks), + join_paths(root, &own) + ); + Some((ancestor, detail)) +} + +/// The hosted refusal for [`npm_member_stray_lock`]. +async fn npm_member_stray_lock_refusal(root: &Path) -> Option<(PathBuf, Refusal)> { + let (ancestor, detail) = npm_member_stray_lock(root).await?; + let refusal = Refusal { + code: WORKSPACE_LOCKFILE_ELSEWHERE.to_string(), + message: format!( + "{detail}; run socket-patch from {} (the workspace root), or delete the \ + stray member lock; nothing was written", + ancestor.display() + ), + }; + Some((ancestor, refusal)) +} + +fn join_paths(dir: &Path, names: &[&str]) -> String { + names + .iter() + .map(|name| dir.join(name).display().to_string()) + .collect::>() + .join(", ") +} + +/// The governing `package.json` workspace root of a member and the +/// workspace locks it holds (see [`package_json_workspace_refusal`]). +async fn package_json_workspace_root(root: &Path) -> Option<(PathBuf, Vec<&'static str>)> { let canonical = tokio::fs::canonicalize(root) .await .unwrap_or_else(|_| root.to_path_buf()); @@ -297,7 +386,7 @@ async fn package_json_workspace_refusal(root: &Path) -> Option<(PathBuf, Refusal if !workspaces_include(&patterns, &rel) { continue; } - let locks: Vec<&str> = WORKSPACE_ROOT_LOCKS + let locks: Vec<&'static str> = WORKSPACE_ROOT_LOCKS .iter() .copied() .filter(|name| ancestor.join(name).is_file()) @@ -311,24 +400,7 @@ async fn package_json_workspace_refusal(root: &Path) -> Option<(PathBuf, Refusal member = ancestor; continue; } - let refusal = Refusal { - code: WORKSPACE_LOCKFILE_ELSEWHERE.to_string(), - message: format!( - "{} is a workspace member with no lockfile of its own: the workspace \ - root {} lists it under \"workspaces\" and installs it from {}, which a \ - hosted run here cannot see; run socket-patch from {} (the workspace \ - root); nothing was written", - root.display(), - ancestor.display(), - locks - .iter() - .map(|name| ancestor.join(name).display().to_string()) - .collect::>() - .join(", "), - ancestor.display() - ), - }; - return Some((ancestor.to_path_buf(), refusal)); + return Some((ancestor.to_path_buf(), locks)); } None } @@ -792,7 +864,8 @@ mod tests { code(&tmp.path().join("packages/excluded"), "npm").await, None ); - // A member with its own lock. + // A member with its own npm lock, under a root whose yarn.lock npm + // does not read (the npm-root case is #1094's test). write(tmp.path(), "packages/a/package-lock.json", "{}"); assert_eq!(code(&member, "npm").await, None); @@ -804,6 +877,72 @@ mod tests { assert_eq!(code(&tmp.path().join("sub"), "npm").await, None); } + /// #1094: npm installs every workspace member from the root's lock and + /// never reads a `package-lock.json` / `npm-shrinkwrap.json` inside the + /// member, so a stray member npm lock does not make the member its own + /// lock root when the workspace root holds an npm lock. Other own locks + /// (pnpm, yarn, Bun, vlt), and a root with no npm lock, keep the + /// member's own-lock shortcut. + #[tokio::test] + async fn npm_member_with_stray_npm_lock_is_refused() { + for (root_lock, member_lock) in [ + ("package-lock.json", "package-lock.json"), + ("package-lock.json", "npm-shrinkwrap.json"), + ("npm-shrinkwrap.json", "package-lock.json"), + ] { + let tmp = tempfile::tempdir().unwrap(); + write( + tmp.path(), + "package.json", + r#"{"name":"root","private":true,"workspaces":["packages/*"]}"#, + ); + write(tmp.path(), root_lock, "{}"); + write(tmp.path(), "packages/a/package.json", "{}"); + write(tmp.path(), &format!("packages/a/{member_lock}"), "{}"); + let member = tmp.path().join("packages/a"); + let refused = refusal(&ProjectView::Disk(&member), &[candidate("npm")], true) + .await + .unwrap_or_else(|| panic!("{root_lock}/{member_lock}: member must be refused")); + assert_eq!(refused.code, WORKSPACE_LOCKFILE_ELSEWHERE); + assert!( + refused.message.contains(root_lock) + && refused.message.contains(member_lock) + && refused.message.contains("ignores") + && refused.message.contains("nothing was written"), + "{}", + refused.message + ); + assert_eq!(code(&member, "pypi").await, None); + assert_eq!(code(tmp.path(), "npm").await, None); + } + + let tmp = tempfile::tempdir().unwrap(); + write( + tmp.path(), + "package.json", + r#"{"private":true,"workspaces":["packages/*"]}"#, + ); + write(tmp.path(), "packages/a/package.json", "{}"); + write(tmp.path(), "packages/a/package-lock.json", "{}"); + let member = tmp.path().join("packages/a"); + // No npm lock at the root (lockless, or another manager's lock). + assert_eq!(code(&member, "npm").await, None); + write(tmp.path(), "yarn.lock", ""); + assert_eq!(code(&member, "npm").await, None); + // A member that also holds a lock its manager does read keeps the + // shortcut (yarn berry treats a nested yarn.lock as its own project). + write(tmp.path(), "package-lock.json", "{}"); + assert_eq!( + code(&member, "npm").await.as_deref(), + Some(WORKSPACE_LOCKFILE_ELSEWHERE) + ); + for own in ["yarn.lock", "bun.lock", "vlt-lock.json"] { + write(tmp.path(), &format!("packages/a/{own}"), ""); + assert_eq!(code(&member, "npm").await, None, "{own}"); + std::fs::remove_file(member.join(own)).unwrap(); + } + } + /// The nearest ancestor that lists the member is its root, past an /// intermediate `package.json` that does not. #[tokio::test] diff --git a/crates/socket-patch-core/src/vendor/npm_flavor.rs b/crates/socket-patch-core/src/vendor/npm_flavor.rs index 21fb4aa46..8e91742ec 100644 --- a/crates/socket-patch-core/src/vendor/npm_flavor.rs +++ b/crates/socket-patch-core/src/vendor/npm_flavor.rs @@ -356,6 +356,21 @@ pub async fn vendor_npm_any<'a>( Ok(found) => found, Err((code, detail)) => return VendorOutcome::Refused { code, detail }, }; + // #1094: a workspace member's own npm lock is one npm never reads; the + // member is refused as it is without that lock. + if flavor == NpmLockFlavor::PackageLock { + if let Some((_, detail)) = + crate::hosted::governing_root::npm_member_stray_lock(project_root).await + { + return VendorOutcome::Refused { + code: "vendor_lockfile_missing", + detail: format!( + "{detail}; vendor from the workspace root, or delete the stray member \ + lock" + ), + }; + } + } if let Some(detail) = flavor_change_refusal(project_root, purl, flavor).await { return VendorOutcome::Refused { code: "vendor_flavor_changed", @@ -1576,6 +1591,48 @@ mod tests { ))); } + /// #1094: a workspace member's own package-lock.json is a lock npm never + /// reads (members install from the workspace root's lock), so vendoring + /// into it would wire nothing. The member is refused as it is without + /// the stray lock, and nothing is written. + #[tokio::test] + async fn npm_member_with_stray_lock_is_refused() { + let (tmp, record) = npm_project().await; + let ws = tempfile::tempdir().unwrap(); + let member = ws.path().join("packages/a"); + tokio::fs::create_dir_all(member.parent().unwrap()) + .await + .unwrap(); + tokio::fs::rename(tmp.path(), &member).await.unwrap(); + touch( + ws.path(), + "package.json", + r#"{"name":"root","private":true,"workspaces":["packages/*"]}"#, + ) + .await; + touch(ws.path(), "package-lock.json", "{}").await; + let lock_before = tokio::fs::read(member.join("package-lock.json")) + .await + .unwrap(); + + let outcome = vendor_any(&member, &record).await; + let VendorOutcome::Refused { code, detail } = outcome else { + panic!("expected Refused, got {outcome:?}"); + }; + assert_eq!(code, "vendor_lockfile_missing"); + assert!( + detail.contains("workspace") && detail.contains("ignores"), + "{detail}" + ); + assert!(!member.join(".socket/vendor").exists()); + assert_eq!( + tokio::fs::read(member.join("package-lock.json")) + .await + .unwrap(), + lock_before + ); + } + /// A yarn.lock ROUTES to the yarn-classic backend. With a header-only /// lock that has no matching block, the backend's own `vendor_lock_entry_not_found` /// proves the dispatch reached it — and nothing is written. From cddf38df40d424ab6892d059073c2cf6838f23af Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 00:33:18 +0000 Subject: [PATCH 3/5] Keep the test file's existing formatting Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/in_process_redirect_pnpm.rs | 27 ++++--------------- 1 file changed, 5 insertions(+), 22 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 626b28033..dd64db845 100644 --- a/crates/socket-patch-cli/tests/in_process_redirect_pnpm.rs +++ b/crates/socket-patch-cli/tests/in_process_redirect_pnpm.rs @@ -56,10 +56,7 @@ async fn rollback_hosted(cwd: &Path, server: &MockServer) -> i32 { }))) .mount(server) .await; - std::env::set_var( - "SOCKET_NPM_REGISTRY", - format!("{}/npm-registry", server.uri()), - ); + std::env::set_var("SOCKET_NPM_REGISTRY", format!("{}/npm-registry", server.uri())); let code = rollback::run(RollbackArgs { targets: Vec::new(), common: socket_patch_cli::args::GlobalArgs { @@ -434,10 +431,7 @@ async fn hosted_bom_lock_and_workspace_read_like_their_plain_twins() { assert_eq!(code, 0, "scan --mode hosted should succeed on a BOM lock"); let lock = std::fs::read_to_string(&lock_path).unwrap(); assert!(lock.starts_with("\u{feff}lockfileVersion:"), "{lock}"); - assert!( - lock.contains(HOSTED_URL), - "the BOM lock is redirected: {lock}" - ); + assert!(lock.contains(HOSTED_URL), "the BOM lock is redirected: {lock}"); let ws_path = tmp.path().join("pnpm-workspace.yaml"); assert_eq!( std::fs::read_to_string(&ws_path).ok().as_deref(), @@ -452,10 +446,7 @@ async fn hosted_bom_lock_and_workspace_read_like_their_plain_twins() { pristine, "rollback restores the BOM lock byte for byte" ); - assert!( - !ws_path.exists(), - "the auto-created workspace file goes too" - ); + assert!(!ws_path.exists(), "the auto-created workspace file goes too"); // A BOM workspace file whose first key is the user's opt-out: left // byte-identical (no duplicate `trustLockfile`), lock still redirected. @@ -481,11 +472,7 @@ async fn hosted_bom_lock_and_workspace_read_like_their_plain_twins() { "the lock is still redirected for {user_ws:?}" ); let ws = std::fs::read_to_string(tmp.path().join("pnpm-workspace.yaml")).unwrap(); - assert_eq!( - ws, - want.unwrap_or(user_ws), - "workspace file for {user_ws:?}" - ); + assert_eq!(ws, want.unwrap_or(user_ws), "workspace file for {user_ws:?}"); assert_eq!(ws.matches("trustLockfile").count(), 1, "{ws:?}"); } } @@ -890,11 +877,7 @@ async fn hosted_pnpm_manifestless_vex_from_lockfile_legacy_ledger_and_api() { ..VexRun::offline() }, ); - assert_eq!( - out.code, - Some(0), - "[{lock_name}] legacy ledger, offline: {out}" - ); + assert_eq!(out.code, Some(0), "[{lock_name}] legacy ledger, offline: {out}"); assert_attested(out.doc(), PURL, UUID, Marker::Redirected, vulns); assert_eq!(api.request_count(), seen); From 57f5bbf16739671d55d426058172b8277bf02cc0 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 01:40:03 +0000 Subject: [PATCH 4/5] Refuse stray-lock members before takeover A vendored run from an npm workspace member with a stray lock refused only inside the vendor backend, after the hosted-to-vendored takeover had already restored a leftover hosted pin. The takeover preflight now raises the same vendor_lockfile_missing refusal first, so nothing is reverted. The hosted and vendored messages no longer suggest deleting the stray lock: the directory is still a listed workspace member, so it would be refused again. Both name the workspace root to run from. Refs #1094 Assisted-by: Claude Code:claude-opus-5-5 --- .../src/hosted/governing_root.rs | 9 ++-- .../src/vendor/npm_flavor.rs | 43 +++++++++++++------ .../socket-patch-core/src/vendor/npm_lock.rs | 9 +++- 3 files changed, 44 insertions(+), 17 deletions(-) diff --git a/crates/socket-patch-core/src/hosted/governing_root.rs b/crates/socket-patch-core/src/hosted/governing_root.rs index 319cd65d1..3f9cfe9a9 100644 --- a/crates/socket-patch-core/src/hosted/governing_root.rs +++ b/crates/socket-patch-core/src/hosted/governing_root.rs @@ -351,8 +351,8 @@ async fn npm_member_stray_lock_refusal(root: &Path) -> Option<(PathBuf, Refusal) let refusal = Refusal { code: WORKSPACE_LOCKFILE_ELSEWHERE.to_string(), message: format!( - "{detail}; run socket-patch from {} (the workspace root), or delete the \ - stray member lock; nothing was written", + "{detail}; run socket-patch from {} (the workspace root); nothing was \ + written", ancestor.display() ), }; @@ -1203,7 +1203,10 @@ mod tests { refused.message.contains(root_lock) && refused.message.contains(member_lock) && refused.message.contains("ignores") - && refused.message.contains("nothing was written"), + && refused.message.contains("nothing was written") + // The only convergent remedy: the directory stays a + // listed member whatever lock it holds (Bugbot on #1095). + && !refused.message.contains("delete"), "{}", refused.message ); diff --git a/crates/socket-patch-core/src/vendor/npm_flavor.rs b/crates/socket-patch-core/src/vendor/npm_flavor.rs index 3a20ac6f1..d5f82fd28 100644 --- a/crates/socket-patch-core/src/vendor/npm_flavor.rs +++ b/crates/socket-patch-core/src/vendor/npm_flavor.rs @@ -389,6 +389,26 @@ async fn detect_vendorable_npm_flavor_with( )) } +/// #1094: a package-lock project that is a member of an npm workspace +/// holds a lock npm never reads (members install from the workspace +/// root's lock), so vendoring into it would wire nothing. Refused as a +/// member without that lock is (`vendor_lockfile_missing`). Shared by +/// [`vendor_npm_any`] and the hosted→vendored takeover preflight +/// ([`super::npm_lock::npm_lock_vendor_preflight`]), which must refuse +/// before the takeover restores the hosted pin. +pub(crate) async fn npm_member_stray_lock_refusal( + project_root: &Path, +) -> Option<(&'static str, String)> { + let (root, detail) = crate::hosted::governing_root::npm_member_stray_lock(project_root).await?; + Some(( + "vendor_lockfile_missing", + format!( + "{detail}; vendor from {} (the workspace root)", + root.display() + ), + )) +} + /// Vendor one npm package through whichever lockfile-flavor backend serves /// this project (package-lock / yarn classic / yarn berry node-modules / /// pnpm / pnpm legacy / bun / vlt). Probe refusals (PnP, unsupported lock @@ -411,19 +431,9 @@ pub async fn vendor_npm_any<'a>( Ok(found) => found, Err((code, detail)) => return VendorOutcome::Refused { code, detail }, }; - // #1094: a workspace member's own npm lock is one npm never reads; the - // member is refused as it is without that lock. if flavor == NpmLockFlavor::PackageLock { - if let Some((_, detail)) = - crate::hosted::governing_root::npm_member_stray_lock(project_root).await - { - return VendorOutcome::Refused { - code: "vendor_lockfile_missing", - detail: format!( - "{detail}; vendor from the workspace root, or delete the stray member \ - lock" - ), - }; + if let Some((code, detail)) = npm_member_stray_lock_refusal(project_root).await { + return VendorOutcome::Refused { code, detail }; } } if let Some(detail) = flavor_change_refusal(project_root, purl, flavor).await { @@ -1771,6 +1781,13 @@ mod tests { .await .unwrap(); + // The hosted→vendored takeover preflight raises the same refusal + // first, so a leftover hosted pin is never restored only to be + // refused (Bugbot on #1095). + let preflight = crate::vendor::npm_lock_vendor_preflight(&member) + .await + .expect("the takeover preflight refuses the member"); + let outcome = vendor_any(&member, &record).await; let VendorOutcome::Refused { code, detail } = outcome else { panic!("expected Refused, got {outcome:?}"); @@ -1780,6 +1797,8 @@ mod tests { detail.contains("workspace") && detail.contains("ignores"), "{detail}" ); + assert!(!detail.contains("delete"), "{detail}"); + assert_eq!(preflight, (code, detail)); assert!(!member.join(".socket/vendor").exists()); assert_eq!( tokio::fs::read(member.join("package-lock.json")) diff --git a/crates/socket-patch-core/src/vendor/npm_lock.rs b/crates/socket-patch-core/src/vendor/npm_lock.rs index 7f14ddf8b..cf3ff48a8 100644 --- a/crates/socket-patch-core/src/vendor/npm_lock.rs +++ b/crates/socket-patch-core/src/vendor/npm_lock.rs @@ -417,8 +417,10 @@ pub async fn vendor_npm<'a>( } /// The project-level refusal [`vendor_npm`]'s step 2 raises whatever the -/// purl: the primary lock (`npm-shrinkwrap.json`, else `package-lock.json`) -/// is not parseable JSON or not a v2/v3 lock. `None` unless the project's +/// purl: the project is an npm workspace member whose own lock npm never +/// reads (#1094, [`super::npm_flavor::npm_member_stray_lock_refusal`]), or +/// the primary lock (`npm-shrinkwrap.json`, else `package-lock.json`) is +/// not parseable JSON or not a v2/v3 lock. `None` unless the project's /// npm flavor is package-lock (the probe `vendor_npm_any` routes on) and /// that lock fails the gate; a missing or unreadable lock is left to the /// backend's own refusal. @@ -437,6 +439,9 @@ pub async fn npm_lock_vendor_preflight(project_root: &Path) -> Option<(&'static ) { return None; } + if let Some(refusal) = super::npm_flavor::npm_member_stray_lock_refusal(project_root).await { + return Some(refusal); + } let (lock_name, lock_bytes, _) = select_lockfile(project_root).await.ok()??; let gate = match LOCK_MEMO.parse(&lock_bytes, || parse_json_manifest(&lock_bytes)) { Ok(lock) => lock_version_gate(&lock, &lock_name).err(), From 3faf081750fb33f205e4968c9da608a3bb2e8661 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 10:22:23 +0000 Subject: [PATCH 5/5] Label the setup-php pin with its real tag Ported from #1118. The upstream v2 tag moved off the pinned commit, so zizmor's ref-version-mismatch audit fails every PR on main. The pin itself is unchanged; only its comment now names 2.37.2. This no-ops once #1118 lands. Assisted-by: Claude Code:claude-opus-5-5 --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f4f8067ea..cce660612 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1397,7 +1397,7 @@ jobs: # The composer capstones shell out to a real composer; `composer:` # pins the release line (1, 2.2 LTS, 2) so the composer.lock grammar # the edits assert stays stable across runners. - uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # v2 + uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2 with: php-version: '8.2' tools: composer:${{ matrix.composer }}