From 8efa6ccd309bff1fa2dd127249595bc5dd4a2226 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 02:24:21 +0000 Subject: [PATCH 1/4] Start fix for #1576 Assisted-by: Claude Code:claude-opus-5-5 From 4371bb245a19764a8743b15be4ea2b4ee26b7c99 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 02:32:03 +0000 Subject: [PATCH 2/4] Fix npm v4 lock refusal advice that loops Vendoring an npm project whose package-lock.json is lockfileVersion 4 (written by npm >= 12.1 for `npm patch`) is refused, and the refusal told users to drop `patchedDependencies` and run `npm install`. npm keeps an existing v4 lock at v4, so following that advice refused again with the same text, forever. The refusal now names `npm install --lockfile-version 3`, the step that actually rewrites the lock as v3. A v4 lock with no native patch left no longer claims vendoring would break a patch it does not have. Fixes #1576 Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-core/src/vendor/npm_lock.rs | 92 ++++++++++++++++++- docs/testing/npm-compatibility.md | 6 +- 2 files changed, 92 insertions(+), 6 deletions(-) diff --git a/crates/socket-patch-core/src/vendor/npm_lock.rs b/crates/socket-patch-core/src/vendor/npm_lock.rs index 6f1d95ec4..6b6822158 100644 --- a/crates/socket-patch-core/src/vendor/npm_lock.rs +++ b/crates/socket-patch-core/src/vendor/npm_lock.rs @@ -445,15 +445,23 @@ fn lock_version_gate(lock: &Value, lock_name: &str) -> Result, Box= 12.1 writes lockfileVersion 4 for its native `npm patch` // (root `patchedDependencies` + a `patched` record on the entry), // and re-running `npm install` keeps it at 4: the npm >= 7 upgrade - // advice would be wrong (#711). - let detail = if lock_version == Some(4) { + // advice would be wrong (#711), and so would a plain `npm install` + // remedy, which loops (#1576). + let detail = if lock_version == Some(4) && has_native_patch(lock) { format!( "{lock_name} has lockfileVersion 4, which npm >= 12.1 writes for its native \ `npm patch` (`patchedDependencies` in package.json); vendored mode supports \ only v2/v3 locks and would drop or break that patch. Fold the Socket fix into \ - your own patch, or remove the `patchedDependencies` entries and \ - regenerate the lock with `npm install`, or use hosted mode, which leaves the \ - patched package alone" + your own patch, or remove the `patchedDependencies` entries and rewrite the \ + lock as v3 with `npm install --lockfile-version 3`, or use hosted mode, which \ + leaves the patched package alone" + ) + } else if lock_version == Some(4) { + format!( + "{lock_name} has lockfileVersion 4, which npm >= 12.1 keeps once it has \ + written it, even after the last `npm patch` is removed; vendored mode \ + supports only v2/v3 locks. Rewrite the lock as v3 with \ + `npm install --lockfile-version 3` and re-run" ) } else { format!( @@ -470,6 +478,19 @@ fn lock_version_gate(lock: &Value, lock_name: &str) -> Result, Box= 12.1 native patch: a `patched` +/// record on a `packages` entry, or `patchedDependencies` on the root one. +fn has_native_patch(lock: &Value) -> bool { + lock.get("packages") + .and_then(Value::as_object) + .is_some_and(|packages| { + packages.iter().any(|(key, entry)| { + entry.get("patched").is_some() + || (key.is_empty() && entry.get("patchedDependencies").is_some()) + }) + }) +} + /// [`vendor_npm`]'s step 3: the rewritable `packages` instances of /// `name@version`, refusing a workspace member, an entry whose every /// instance is bundled or linked, and an absent entry. Nothing here reads @@ -2922,6 +2943,67 @@ mod tests { ); } + /// REGRESSION (#1576): `npm install` keeps a v4 lock at v4, so the + /// remedy must name the flag that writes v3, never a plain install. + #[tokio::test] + async fn lockfile_v4_refusal_remedy_downgrades_the_lock() { + let lock = json!({ + "name": "fixture", + "version": "1.0.0", + "lockfileVersion": 4, + "packages": { + "": { "name": "fixture", "version": "1.0.0" }, + "node_modules/left-pad": { + "version": "1.3.0", + "resolved": REG_RESOLVED, + "integrity": "sha512-orig==", + "patched": { "integrity": "sha512-user==", "path": "patches/left-pad@1.3.0.patch" } + } + } + }); + let fx = fixture_with("left-pad", "1.3.0", lock).await; + let detail = expect_refused( + fx.vendor(false).await, + "vendor_lockfile_version_unsupported", + ); + assert!( + detail.contains("npm install --lockfile-version 3") + && !detail.contains("regenerate the lock with `npm install`"), + "{detail}" + ); + } + + /// REGRESSION (#1576): a v4 lock whose native patches are all gone + /// is refused without claiming a patch it no longer has, and names + /// the downgrade that clears the refusal. + #[tokio::test] + async fn lockfile_v4_without_native_patch_names_only_the_downgrade() { + let lock = json!({ + "name": "fixture", + "version": "1.0.0", + "lockfileVersion": 4, + "packages": { + "": { "name": "fixture", "version": "1.0.0" }, + "node_modules/left-pad": { + "version": "1.3.0", + "resolved": REG_RESOLVED, + "integrity": "sha512-orig==" + } + } + }); + let fx = fixture_with("left-pad", "1.3.0", lock).await; + let detail = expect_refused( + fx.vendor(false).await, + "vendor_lockfile_version_unsupported", + ); + assert!( + detail.contains("lockfileVersion 4") + && detail.contains("npm install --lockfile-version 3") + && !detail.contains("would drop or break"), + "{detail}" + ); + } + /// The takeover preflight raises exactly the backend's own version /// refusal on a v1 lock (#659), before any write. #[tokio::test] diff --git a/docs/testing/npm-compatibility.md b/docs/testing/npm-compatibility.md index 297a45779..f10760efa 100644 --- a/docs/testing/npm-compatibility.md +++ b/docs/testing/npm-compatibility.md @@ -29,7 +29,11 @@ npm 12 notes: mode therefore leaves such a package on its registry entry, in every npm lock, and warns `redirect_npm_patched_dependency_skipped`; other packages in the lock are still pinned. Vendored mode refuses the v4 lock - (`vendor_lockfile_version_unsupported`, naming `npm patch`) (#711). + (`vendor_lockfile_version_unsupported`, naming `npm patch`) (#711). npm + keeps a v4 lock at v4 on a plain `npm install`, even after `npm patch rm`, + so the refusal names `npm install --lockfile-version 3` as the step that + clears it; a v4 lock with no native patch left is refused without naming + one (#1576). - `allow-remote` defaults to `none`: any tarball whose `resolved` origin is not the configured registry is refused. `allow-remote=root` admits only direct dependencies. `allow-remote=all` is the setting a hosted redirect needs; it From b3eaff48333314718c3b15f1e094ab56beece8ee Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 02:46:20 +0000 Subject: [PATCH 3/4] Read package.json for the v4 native-patch check The v4 refusal now treats a project as still carrying an `npm patch` when package.json has a `patchedDependencies` key, even if no lock entry records it, and ignores a `"patched": null` record. That is how hosted mode already reads it, so a user who kept the key but lost the lock record is still told to remove it before downgrading the lock. Refs #1576 Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 2 +- .../socket-patch-core/src/vendor/npm_lock.rs | 52 +++++++++++++------ 2 files changed, 36 insertions(+), 18 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 3756e7abf..d59773e2a 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -1556,7 +1556,7 @@ command's integration tests check its output. | `redirect_bun_patched_dependency_skipped` | `warnings[]` (warning) | scan/get `--mode hosted` (bun, `bun.lock` and `bun.lockb`): the project patches the granted `name@version` itself with `bun patch` (a `patchedDependencies` key for `name@version`, or the bare name, in the root `package.json` or mirrored in `bun.lock`). Bun applies that patch only to the registry resolution, so the entry is left on its registry tuple instead of silently losing the user's patch (#367). Per-dep; the detail names the key and the remedy (fold the Socket fix into the user's patch, or drop the `patchedDependencies` entry and re-run); the in-run VEX never assumes the uuid applied. Vendored mode refuses the same package `vendor_lock_entry_unsupported` before any write or download. Exit 0. | | `redirect_bun_default_trust_lost` / `vendor_bun_default_trust_lost` | `warnings[]` / vendor `warnings[]` (warning) | scan/get `--mode hosted` and vendored mode (bun, `bun.lock` and `bun.lockb`): the rewired package is on Bun's built-in default trusted list (better-sqlite3, esbuild, sharp, …) and the project declares no `trustedDependencies` (root `package.json`, or the copy mirrored in `bun.lock`). Bun 1.3.5 and later apply that list only to packages resolved from the npm registry, so on a hosted URL or a local tarball the package's install scripts are skipped with exit 0 (#371). The package is still rewired; the detail tells the user to add it to `trustedDependencies` (which replaces Bun's default list, so other default-trusted dependencies whose scripts matter must be listed too). Repeated on every run while the pin stays and no list is declared. Exit 0. | | `redirect_bun_non_registry_entry_skipped` / `vendor_non_registry_entry_skipped` | `warnings[]` / vendor warnings (warning) | scan/get `--mode hosted` and vendored mode (bun, `bun.lock` and `bun.lockb`): the lock also installs the granted `name@version` from a user URL or `file:` tarball (its `-.tgz` leaf names that version). Bun installs it from that spec, so the copy stays unpatched beside any rewired registry copy; it is left untouched, the in-run VEX does not assume the patch, and `vex` attests nothing for that `name@version` (#497). With no registry copy left, vendoring refuses with `vendor_lock_entry_not_rewritable`. A same-name URL / `file:` tarball whose leaf names no version, or a git, folder or `link:` copy, has no version in the lock: these codes do not fire for it, but `vex` attests no ref of that package from that lock (`patched_ref_unattributable`), since the copy may be the wired version. | -| `redirect_npm_patched_dependency_skipped` | `warnings[]` (warning) | scan/get `--mode hosted` (`package-lock.json` / `npm-shrinkwrap.json`): the project patches the granted `name@version` itself with npm ≥ 12.1's native `npm patch` (a root `package.json` `patchedDependencies` key for `name@version` or the bare name, or a `packages` entry carrying npm's `patched` record, which npm writes in a lockfileVersion 4 lock). npm applies the user's diff on top of whatever tarball the lock names and fails the install `EPATCHFAILED` when it no longer applies, so a hosted pin would break every later `npm ci` / `npm install` (or install bytes VEX can never attest); the dep is left on its registry entry in every present npm lock instead (#711). Per-dep; the detail names the key or lock entry and the remedy (fold the Socket fix into the user's patch, or drop the `patchedDependencies` entry and re-run); when an earlier hosted run already pinned the entry, the detail says so and names `socket-patch rollback ` to restore the registry entry instead of claiming it is unchanged; the in-run VEX never assumes the uuid applied, and no sibling lock confirms it. Vendored mode refuses the lockfileVersion 4 lock `vendor_lockfile_version_unsupported`, its detail naming `npm patch` / `patchedDependencies`. Exit 0. | +| `redirect_npm_patched_dependency_skipped` | `warnings[]` (warning) | scan/get `--mode hosted` (`package-lock.json` / `npm-shrinkwrap.json`): the project patches the granted `name@version` itself with npm ≥ 12.1's native `npm patch` (a root `package.json` `patchedDependencies` key for `name@version` or the bare name, or a `packages` entry carrying npm's `patched` record, which npm writes in a lockfileVersion 4 lock). npm applies the user's diff on top of whatever tarball the lock names and fails the install `EPATCHFAILED` when it no longer applies, so a hosted pin would break every later `npm ci` / `npm install` (or install bytes VEX can never attest); the dep is left on its registry entry in every present npm lock instead (#711). Per-dep; the detail names the key or lock entry and the remedy (fold the Socket fix into the user's patch, or drop the `patchedDependencies` entry and re-run); when an earlier hosted run already pinned the entry, the detail says so and names `socket-patch rollback ` to restore the registry entry instead of claiming it is unchanged; the in-run VEX never assumes the uuid applied, and no sibling lock confirms it. Vendored mode refuses the lockfileVersion 4 lock `vendor_lockfile_version_unsupported`, its detail naming `npm patch` / `patchedDependencies` and `npm install --lockfile-version 3` (npm keeps a v4 lock at v4 on a plain `npm install`); a v4 lock with no native patch left (no `patchedDependencies` key, no non-null `patched` record) gets a detail naming only that downgrade (#1576). Exit 0. | | `nested_project_skipped` | `warnings[]` (warning) | scan `--mode hosted` / `--mode vendored` (npm family): the crawl found a patched package only in the `node_modules` of a nested, independent npm project below the scan root (a directory with its own npm-family lock that is not a workspace member whose package manager ignores that lock), the root's lockfiles do not list it, and the root's own install holds no copy. A hosted or vendored run rewires only the root's lockfiles, so the package is left out of the run (no `packages[]` row, no event, no VEX statement) instead of failing `vendor_lock_entry_not_found` or skipping `redirect_unconfirmed`. One warning per nested project; the detail names the purls, the project directory and the PATH to pass (`socket-patch scan --mode `). Exit and status unchanged (#1534). | | `redirect_vlt_lock_unsupported` | `warnings[]` (warning) | scan/get `--mode hosted` (vlt): `vlt-lock.json` has a `lockfileVersion` other than absent, `0` or `1` (decided on the raw JSON token), is not a JSON object, starts with a UTF-8 BOM, or its `nodes` section is not vlt's one-node-per-line layout. Nothing rewritten; also refuses a vendored → hosted takeover of a `flavor: "vlt"` entry before its revert (`skipped` (hosted event)). Exit 0. | | `redirect_takeover_kept_vendored` | `warnings[]` (warning) | scan/get `--mode hosted`: a vendored → hosted takeover the hosted rewrite would not pin was retracted (see **Staged takeover**): the package keeps its vendored wiring, ledger entry and artifact byte-identical and stays patched. The detail names the cause code, which is also the purl's `skipped` (hosted event). Exit 0. Replaces v5.0-pre `redirect_takeover_unpatched`, which reported a package left unpatched in both modes and is no longer emitted. | diff --git a/crates/socket-patch-core/src/vendor/npm_lock.rs b/crates/socket-patch-core/src/vendor/npm_lock.rs index 6b6822158..1b257641c 100644 --- a/crates/socket-patch-core/src/vendor/npm_lock.rs +++ b/crates/socket-patch-core/src/vendor/npm_lock.rs @@ -182,7 +182,7 @@ impl NpmLockBackend for PackageLockBackend { ))); } }; - let lock_version = lock_version_gate(&lock, &lock_name)?; + let lock_version = lock_version_gate(&lock, &lock_name, project_root)?; // The root manifest's `overrides` (#490): which git / url / `file:` // dependent specs npm really installs from. @@ -423,7 +423,7 @@ pub async fn npm_lock_vendor_preflight(project_root: &Path) -> Option<(&'static } 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(), + Ok(lock) => lock_version_gate(&lock, &lock_name, project_root).err(), Err(e) => Some(Box::new(refused( "vendor_lockfile_version_unsupported", format!("{lock_name} is not parseable JSON: {e}"), @@ -437,7 +437,11 @@ pub async fn npm_lock_vendor_preflight(project_root: &Path) -> Option<(&'static /// The lock version gate of [`vendor_npm`]'s step 2: only v2/v3 locks with /// a `packages` object are rewritten. `Ok` is the parsed `lockfileVersion`. -fn lock_version_gate(lock: &Value, lock_name: &str) -> Result, Box> { +fn lock_version_gate( + lock: &Value, + lock_name: &str, + project_root: &Path, +) -> Result, Box> { let lock_version = lock.get("lockfileVersion").and_then(Value::as_u64); if !matches!(lock_version, Some(2) | Some(3)) || !lock.get("packages").is_some_and(Value::is_object) @@ -447,7 +451,7 @@ fn lock_version_gate(lock: &Value, lock_name: &str) -> Result, Box= 7 upgrade // advice would be wrong (#711), and so would a plain `npm install` // remedy, which loops (#1576). - let detail = if lock_version == Some(4) && has_native_patch(lock) { + let detail = if lock_version == Some(4) && has_native_patch(lock, project_root) { format!( "{lock_name} has lockfileVersion 4, which npm >= 12.1 writes for its native \ `npm patch` (`patchedDependencies` in package.json); vendored mode supports \ @@ -478,16 +482,21 @@ fn lock_version_gate(lock: &Value, lock_name: &str) -> Result, Box= 12.1 native patch: a `patched` -/// record on a `packages` entry, or `patchedDependencies` on the root one. -fn has_native_patch(lock: &Value) -> bool { +/// Whether the project still carries an npm >= 12.1 native patch, read as +/// hosted mode's #711 gate reads it: a root `package.json` +/// `patchedDependencies` key, or a non-null `patched` record on a lock +/// entry. Only the refusal path calls it, so the blocking read is rare. +fn has_native_patch(lock: &Value, project_root: &Path) -> bool { + let manifest = std::fs::read_to_string(project_root.join("package.json")).ok(); + if !super::bun_lock_text::patched_dependency_keys(manifest.as_deref(), None).is_empty() { + return true; + } lock.get("packages") .and_then(Value::as_object) .is_some_and(|packages| { - packages.iter().any(|(key, entry)| { - entry.get("patched").is_some() - || (key.is_empty() && entry.get("patchedDependencies").is_some()) - }) + packages + .values() + .any(|entry| entry.get("patched").is_some_and(|p| !p.is_null())) }) } @@ -585,7 +594,8 @@ pub(super) async fn read_project(project_root: &Path) -> Result= 7"), "{detail}" ); } - /// REGRESSION (#1576): `npm install` keeps a v4 lock at v4, so the - /// remedy must name the flag that writes v3, never a plain install. + /// REGRESSION (#1576): a `patchedDependencies` key in package.json + /// still counts as a native patch when no lock entry records it, and + /// the remedy names the flag that writes v3, never a plain install. #[tokio::test] - async fn lockfile_v4_refusal_remedy_downgrades_the_lock() { + async fn lockfile_v4_manifest_patch_remedy_downgrades_the_lock() { let lock = json!({ "name": "fixture", "version": "1.0.0", @@ -2957,17 +2969,23 @@ mod tests { "version": "1.3.0", "resolved": REG_RESOLVED, "integrity": "sha512-orig==", - "patched": { "integrity": "sha512-user==", "path": "patches/left-pad@1.3.0.patch" } + "patched": null } } }); let fx = fixture_with("left-pad", "1.3.0", lock).await; + std::fs::write( + fx.root().join("package.json"), + r#"{"name":"fixture","patchedDependencies":{"left-pad@1.3.0":"patches/left-pad@1.3.0.patch"}}"#, + ) + .unwrap(); let detail = expect_refused( fx.vendor(false).await, "vendor_lockfile_version_unsupported", ); assert!( - detail.contains("npm install --lockfile-version 3") + detail.contains("would drop or break") + && detail.contains("npm install --lockfile-version 3") && !detail.contains("regenerate the lock with `npm install`"), "{detail}" ); From 2b1c49e92c2e1e77923c99ea45e915469fb9d6a2 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 03:18:55 +0000 Subject: [PATCH 4/4] Don't block on a FIFO at package.json The v4 refusal's native-patch probe read package.json with a plain open, so a FIFO or device at that path would hang the vendored run forever. It now goes through the regular-file reader the rest of the vendor code uses, which refuses non-regular files without blocking. Refs #1576 Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-core/src/vendor/npm_lock.rs | 31 +++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/crates/socket-patch-core/src/vendor/npm_lock.rs b/crates/socket-patch-core/src/vendor/npm_lock.rs index 1b257641c..d58aba5a1 100644 --- a/crates/socket-patch-core/src/vendor/npm_lock.rs +++ b/crates/socket-patch-core/src/vendor/npm_lock.rs @@ -485,9 +485,11 @@ fn lock_version_gate( /// Whether the project still carries an npm >= 12.1 native patch, read as /// hosted mode's #711 gate reads it: a root `package.json` /// `patchedDependencies` key, or a non-null `patched` record on a lock -/// entry. Only the refusal path calls it, so the blocking read is rare. +/// entry. Only the refusal path calls it, so the blocking read is rare; it +/// goes through the regular-file reader, so a FIFO there can't wedge it. fn has_native_patch(lock: &Value, project_root: &Path) -> bool { - let manifest = std::fs::read_to_string(project_root.join("package.json")).ok(); + let manifest = + crate::utils::fs::read_regular_to_string_sync(&project_root.join("package.json")).ok(); if !super::bun_lock_text::patched_dependency_keys(manifest.as_deref(), None).is_empty() { return true; } @@ -3022,6 +3024,31 @@ mod tests { ); } + /// A FIFO at package.json is not a manifest: the native-patch probe + /// reads past it instead of blocking in open(2). + #[cfg(unix)] + #[test] + fn native_patch_probe_does_not_block_on_a_fifo_manifest() { + use std::os::unix::ffi::OsStrExt as _; + let tmp = tempfile::tempdir().unwrap(); + let fifo = tmp.path().join("package.json"); + let c_path = std::ffi::CString::new(fifo.as_os_str().as_bytes()).unwrap(); + assert_eq!(unsafe { libc::mkfifo(c_path.as_ptr(), 0o644) }, 0); + let (tx, rx) = std::sync::mpsc::channel(); + let root = tmp.path().to_path_buf(); + std::thread::spawn(move || { + let _ = tx.send(has_native_patch(&json!({ "packages": {} }), &root)); + }); + match rx.recv_timeout(std::time::Duration::from_secs(5)) { + Ok(found) => assert!(!found), + Err(_) => { + // Release the wedged opener so the suite can fail cleanly. + let _ = std::fs::OpenOptions::new().write(true).open(&fifo); + panic!("the native-patch probe wedged in open(2) on a FIFO"); + } + } + } + /// The takeover preflight raises exactly the backend's own version /// refusal on a v1 lock (#659), before any write. #[tokio::test]