From 7f1e184f21230fa65dbb726fcf685f250d710360 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 02:24:37 +0000 Subject: [PATCH 1/6] Start fix for #1462 Assisted-by: Claude Code:claude-opus-5-5 From 5e9277b07c64cd92c4dffef6595d5c956ed7d0fa Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 02:32:51 +0000 Subject: [PATCH 2/6] Check v4 yarn classic pins against served deps A hosted rescan of a yarn classic lock never fetched the served tarball of an artifact the lock already pinned with a sha1, so a pin v4.0.0 wrote for a patch whose package.json adds a dependency the lock doesn't lock was confirmed as applied with no warning. yarn then installed the patched package without that dependency, and vex attested it. The rescan now reads the served tarball for every pinned artifact too, so the stale pin gets the redirect_yarn_classic_dep_manifest_unlocked warning (now saying the earlier pin is not confirmed) and is listed as not hosted, with either grant shape. The lock's blocks are still read at most once, and never on a fully pinned rescan. vex no longer attests a hosted npm copy whose patch changes its package.json when a declared, non-optional dependency does not resolve from the copy; it is omitted as dependency_not_installed until the dependency is installed. Fixes #1462 Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 3 +- crates/socket-patch-cli/src/commands/vex.rs | 6 ++ .../tests/e2e_vex_lockfile/yarn.rs | 46 +++++++++ .../tests/scan/hosted_yarn_classic_sha1.rs | 97 +++++++++++++++++++ crates/socket-patch-core/src/hosted/engine.rs | 54 ++++++----- .../src/patch/redirect/mod.rs | 17 +++- crates/socket-patch-core/src/vex/verify.rs | 80 ++++++++++++++- 7 files changed, 273 insertions(+), 30 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 6506b62ad..5ba116741 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -1442,6 +1442,7 @@ command's integration tests check its output. | `vendor_ledger_missing` | `failed` (artifact-level: `uuid` + `details.{ecosystem,path}`, no purl) | repair (v5.0) and `vendor --check`: a lockfile references `.socket/vendor///` but the vendor ledger has no entry for it; repair no longer rebuilds ledger entries from lockfiles. Recovery: restore `.socket/vendor/state.json` from version control and re-run `repair`, or `git checkout -- ` and re-vendor. | | `vendor_wiring_unknown_revert_blocked` | `skipped` (beside the `failed`/`revert_failed` event) | vendor --revert: the ledger entry was reconstructed by a pre-v5 `repair` without wiring records and the live lockfile still resolves through the artifact — the revert refuses (fail-closed) instead of deleting a tarball the lock points at. Recovery: `socket-patch repair`, then restore the pre-vendor lock (or re-lock without the override) and re-run the revert. repair: an npm ledger entry whose `flavor` this release does not know (written by a newer socket-patch) is skipped, never health-checked or rebuilt, and the artifact, wiring and ledger stay as found (a lone `skipped` event; the run's exit is unaffected). Recovery: upgrade socket-patch. | | `stale_install` | `skipped` | vex (in-run `scan --mode hosted --vex`): a hosted stale-install probe found positively unpatched installed bytes, so the purl is omitted even under `--vex-no-verify` (see the gem / Python stale-install guards). | +| `dependency_not_installed` | `skipped` | vex: a hosted npm-family copy carries every patched file, but the patch changes its `package.json` and a dependency it declares (other than an optional one) does not resolve from the copy (no `node_modules/` in it or above it), because the lockfile never locked it (a yarn classic pin written before the served-manifest check, #1462). Omitted until the dependency is installed. | | `record_unavailable` | `skipped` | vex (manifest-less): a lockfile-wired patch has no local record (manifest, this run's hosted records or a pre-v5 redirect ledger, vendor ledger) and none could be fetched — `--offline`, transport error, 404, or a refused (paid) patch. Omitted, never attested from the `socket-patch.vendor.json` marker. | | `record_mismatch` | `skipped` | vex (manifest-less): the record found for a wired patch names another package or another patch uuid than the wiring. | | `vendor_unwired` | `skipped` | vex: a vendor-ledger entry whose committed artifact no lockfile/config wires any more (reverted lock, leftover ledger or artifact). Applies under `--no-verify` too. | @@ -1499,7 +1500,7 @@ command's integration tests check its output. | `vendor_bun_reinstall_required` | `skipped` (advisory; human: `Warning: …`); rollback/remove `warnings[]`; `scan --prune` top-level `warnings[]` (human: `GC: …`) | a wet Bun revert (`vendor --revert`, rollback / remove of a vendored entry, `--preserve-state` included) that restored the lock entry while `node_modules/` is a real directory, or the tree has no `node_modules/.bun/` (a hoisted install): Bun's hoisted linker does not re-extract a package whose lock entry moves from the vendored tarball back to the registry record of the same `name@version`, so a plain `bun install` (also `--frozen-lockfile`) reports no changes and keeps the vendored bytes (measured on 1.1.45 … 1.4.2). The detail names `name@version` and says to run `bun install --force` (or delete `node_modules` and run `bun install`); the human revert hint names `bun install --force` too. An isolated install (a link into `node_modules/.bun/`) relinks and a project without `node_modules/` has nothing installed: neither warns, and neither does a dry run, a drift-kept revert, or a revert that restored nothing (`vendor_lockfile_missing`, or `vendor_lock_entry_removed` after `bun remove`, whose copy a plain `bun install` prunes). | | `vendor_flavor_changed` | `failed` | vendor (npm): the purl's vendor ledger entry was written for another lockfile `flavor` than the one the router now detects (for example `npm` → `vlt` after switching package managers). Remedy: `socket-patch vendor --revert` it first, then re-vendor. Refused before any write. | | `vendor_dep_manifest_unlocked` | refused | vendor (yarn classic): the patch rewrites the package's own `package.json` to depend on a descriptor (`name@range`) no `yarn.lock` block is keyed by — an added dependency, or an existing one moved to a new range. yarn 1 builds its install graph from the lock, so the rewired block would name a dependency it never resolves: online frozen installs fetch it unpinned, `--offline` installs fail and every plain `yarn install` re-saves the lock (#591). Refused after staging and before any wiring is written (the staged uuid dir is removed); the detail names the descriptors. Remedy: lock them first (for example `yarn add `), then re-run. A dry run, which stages nothing, does not foresee it. Also `failed` on `repair` (and `vendor_check_failed` on `vendor --check`) for a lock an older release already vendored that way, with a block wired to the entry naming a descriptor no block locks and the recorded pre-vendor block did not (#1379); the lock is left as it is. Remedy: `yarn install` (without `--frozen-lockfile`) to lock them, then commit `yarn.lock`. | -| `redirect_yarn_classic_dep_manifest_unlocked` | `warnings[]` (warning) | scan/get `--mode hosted` (yarn classic): the served tarball's own `package.json` depends on a descriptor no `yarn.lock` block is keyed by. yarn 1 installs only what the lock names, so a pin would install the patched package without that dependency (#591). The dep is not pinned and never confirmed; the lock is left as it was. Same remedy as `vendor_dep_manifest_unlocked`. | +| `redirect_yarn_classic_dep_manifest_unlocked` | `warnings[]` (warning) | scan/get `--mode hosted` (yarn classic): the served tarball's own `package.json` depends on a descriptor no `yarn.lock` block is keyed by. yarn 1 installs only what the lock names, so a pin would install the patched package without that dependency (#591). The dep is not pinned and never confirmed; the lock is left as it was. The served tarball is read on every run, so a pin an earlier release wrote without this check (#1462) gets the same warning and is never confirmed either; it stays in the lock until the dependency is locked or the pin is rolled back. Same remedy as `vendor_dep_manifest_unlocked`. | | `redirect_yarn_classic_dep_manifest_rewritten` | `warnings[]` (warning) | scan/get `--mode hosted` (yarn classic): the served tarball's own `package.json` declares other dependencies than the pinned block's sub-maps, every descriptor already locked; the block's `dependencies:` / `optionalDependencies:` sub-maps are rewritten to match (#591). | | `vendor_artifact_gitignored` | `failed` | vendor (vlt, the npm-family tarball flavors — npm, pnpm, bun, yarn classic, yarn berry — NuGet, and the JVM trees of Maven, Gradle, sbt and scala-cli): inside a git work tree, `git check-ignore --no-index` reports the new artifact's uuid directory (JVM: its tree root, such as `.socket/vendor/maven2`) as ignored by a rule its own `.gitignore` cannot override (such as a root `.socket/` or `vendor/` rule; the detail names the rule). Remedy: drop that rule for `.socket/vendor/`. Refused before any write, dry run included. A file rule such as `*.tgz`, `*.nupkg` or `*.jar` is overridden by the `!*` `.gitignore` vendoring writes into the uuid dir or tree root; if a written tarball, directory payload or nupkg still reads as ignored, the run refuses and removes the uuid dir it created. | | `vendor_artifact_gitignore_unchecked` | warning | vendor (vlt, the npm-family tarball flavors and NuGet): git is installed but could not answer the ignore check for the written vendored directory (it failed to start, ran past 30 s, or `rev-parse` / `check-ignore` exited with an error); the package is vendored and the detail names what failed. Remedy: make sure no ignore rule covers `.socket/` before committing. Git absent, or a project outside any work tree, raises nothing. | diff --git a/crates/socket-patch-cli/src/commands/vex.rs b/crates/socket-patch-cli/src/commands/vex.rs index a74652229..3f7c12007 100644 --- a/crates/socket-patch-cli/src/commands/vex.rs +++ b/crates/socket-patch-cli/src/commands/vex.rs @@ -23,6 +23,7 @@ use clap::Args; use socket_patch_core::api::client::ApiClient; use socket_patch_core::manifest::schema::{PatchManifest, PatchRecord}; use socket_patch_core::telemetry::{track_vex_failed, track_vex_generated, TelemetryAuth}; +use socket_patch_core::vex::verify::DEPENDENCY_NOT_INSTALLED; use socket_patch_core::vex::{ build_document, detect_product, BuildOptions, Document, FailedPatch, VendorContext, VerifyOutcome, @@ -1781,6 +1782,10 @@ fn omission_phrase(reason: &str) -> &'static str { against" } "stale_install" => "the installed copy is not patched", + DEPENDENCY_NOT_INSTALLED => { + "the patched package.json declares a dependency that is not installed (the \ + lockfile does not lock it)" + } "mirror_overrides_source" => { "a Bundler mirror overrides the hosted source and installed bytes were not verified" } @@ -1986,6 +1991,7 @@ mod tests { "vendor_manifest_unverifiable", "stale_install", "mirror_overrides_source", + DEPENDENCY_NOT_INSTALLED, RECORD_UNAVAILABLE, RECORD_MISMATCH, VENDOR_UNWIRED, diff --git a/crates/socket-patch-cli/tests/e2e_vex_lockfile/yarn.rs b/crates/socket-patch-cli/tests/e2e_vex_lockfile/yarn.rs index 55b365309..a8dfc7436 100644 --- a/crates/socket-patch-cli/tests/e2e_vex_lockfile/yarn.rs +++ b/crates/socket-patch-cli/tests/e2e_vex_lockfile/yarn.rs @@ -672,6 +672,52 @@ fn hosted_lockfile_only_matrix() { } } +/// #1462: a hosted pin an earlier release wrote for a patch whose +/// package.json adds a dependency (`is-odd`) the lock never locked. yarn +/// installs the patched bytes without that dependency, so although every +/// patched file verifies, the patch is omitted until the dependency is +/// installed beside it. +#[test] +fn hosted_patch_adding_an_uninstalled_dependency_is_omitted() { + const MANIFEST: &str = "package/package.json"; + let pristine_manifest = format!(r#"{{"name":"{NAME}","version":"{VERSION}"}}"#); + let patched_manifest = format!( + r#"{{"name":"{NAME}","version":"{VERSION}","dependencies":{{"is-odd":"^3.0.0"}}}}"# + ); + let mut view = patch_view(UUID, PURL); + view["files"][MANIFEST] = serde_json::json!({ + "beforeHash": compute_git_sha256_from_bytes(pristine_manifest.as_bytes()), + "afterHash": compute_git_sha256_from_bytes(patched_manifest.as_bytes()), + }); + let (rt, server) = serve_patch_views(vec![(UUID, view)]); + for flavor in FLAVORS { + let tmp = tempfile::tempdir().unwrap(); + let cwd = tmp.path(); + hosted_checkout(cwd, flavor); + install(cwd, PATCHED); + put( + cwd, + "node_modules/left-pad/package.json", + patched_manifest.as_bytes(), + ); + + let (code, env) = vex_json(cwd, &["--proxy-url", &server.uri()]); + let cell = format!("{flavor:?} dependency not installed"); + assert_omitted(cwd, code, &env, "dependency_not_installed", &cell); + + // Hoisted beside it, the dependency resolves: attested. + put( + cwd, + "node_modules/is-odd/package.json", + br#"{"name":"is-odd","version":"3.0.1"}"#, + ); + let (code, env) = vex_json(cwd, &["--proxy-url", &server.uri()]); + let cell = format!("{flavor:?} dependency installed"); + assert_attested(cwd, code, &env, UUID, "redirected", &cell); + } + drop(rt); +} + // ────────────────────────────────────────────────────────────────────── // VENDORED — lockfile only (cells a, b, e) // ────────────────────────────────────────────────────────────────────── diff --git a/crates/socket-patch-cli/tests/scan/hosted_yarn_classic_sha1.rs b/crates/socket-patch-cli/tests/scan/hosted_yarn_classic_sha1.rs index 2e4c469e6..393ad9ea5 100644 --- a/crates/socket-patch-cli/tests/scan/hosted_yarn_classic_sha1.rs +++ b/crates/socket-patch-cli/tests/scan/hosted_yarn_classic_sha1.rs @@ -341,3 +341,100 @@ async fn mock_api_with_sha1(server: &MockServer, tarball: &[u8]) { .await; mock_api(server, tarball, Some(tarball.to_vec())).await; } + +/// The patched tarball of #1462: its package.json adds `is-odd`, which +/// [`LOCK`] doesn't lock. +fn tarball_adding_a_dependency() -> Vec { + tgz(&[ + ( + "package/package.json", + br#"{"name":"left-pad","version":"1.3.0","dependencies":{"is-odd":"^3.0.0"}}"#, + ), + ("package/index.js", b"module.exports = require('is-odd');\n"), + ]) +} + +/// [`write_classic_project`] with `left-pad` already pinned to the hosted +/// artifact the way v4.0.0 pinned it: `resolved` carries the tarball's +/// sha1, and the block's dependencies were never checked against the +/// served package.json. Returns the pinned lock. +fn write_v4_pinned_project(root: &Path, api: &str, tarball: &[u8]) -> String { + write_classic_project(root); + let url = format!("{api}/patch/npm/left-pad/1.3.0/{TOKEN}/{UUID}/left-pad-1.3.0.tgz"); + let lock = format!( + "# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.\n\ + # yarn lockfile v1\n\n\n\ + left-pad@1.3.0:\n version \"1.3.0\"\n \ + resolved \"{url}#{}\"\n integrity {}\n", + sha1_hex(tarball), + sha512_sri(tarball) + ); + std::fs::write(root.join("yarn.lock"), &lock).unwrap(); + lock +} + +/// #1462: a lock an earlier release pinned to a tarball whose package.json +/// adds a dependency the lock doesn't lock. Yarn installs the patched +/// package without it, so the rescan must not confirm the pin: it names +/// the unlocked dependency and reports the patch as not hosted, whether +/// the grant carries the sha1 (the tarball would otherwise never be +/// read) or not. +async fn assert_v4_pin_with_unlocked_dependency_is_not_confirmed(with_sha1: bool) { + let server = MockServer::start().await; + let tarball = tarball_adding_a_dependency(); + if with_sha1 { + mock_api_with_sha1(&server, &tarball).await; + } else { + mock_api(&server, &tarball, Some(tarball.clone())).await; + } + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join("proj"); + let lock = write_v4_pinned_project(&root, &server.uri(), &tarball); + + let (code, doc, stderr) = scan(&root, &server.uri()); + let warning = doc["warnings"] + .as_array() + .unwrap() + .iter() + .find(|w| w["code"] == "redirect_yarn_classic_dep_manifest_unlocked") + .unwrap_or_else(|| panic!("refusal warning (sha1: {with_sha1}): {doc:#}")); + let detail = warning["detail"].as_str().unwrap(); + assert!( + detail.contains("is-odd@^3.0.0") && detail.contains("an earlier run's pin of it"), + "{detail}" + ); + assert!( + common::envelope::hosted_pins(&doc).is_empty(), + "the stale pin is not confirmed (sha1: {with_sha1}): {doc:#}" + ); + assert!( + !common::envelope::mode_events(&doc, "hosted") + .iter() + .any(|e| e["purl"] == PURL && e["action"] == "applied"), + "no applied event (sha1: {with_sha1}): {doc:#}" + ); + // Like a fresh refusal, the patch is listed as not hosted; the exit + // code is unchanged pending the hosted exit-policy decision (#704). + assert!( + common::envelope::mode_events(&doc, "hosted") + .iter() + .any(|e| e["purl"] == PURL && e["action"] == "skipped"), + "listed as not hosted (sha1: {with_sha1}): {doc:#}" + ); + assert_eq!(code, 0, "{doc:#}\n{stderr}"); + assert_eq!( + std::fs::read_to_string(root.join("yarn.lock")).unwrap(), + lock, + "the scan never rewrites the earlier pin" + ); +} + +#[tokio::test] +async fn issue_1462_v4_pin_with_unlocked_dependency_is_not_confirmed_sha1_grant() { + assert_v4_pin_with_unlocked_dependency_is_not_confirmed(true).await; +} + +#[tokio::test] +async fn issue_1462_v4_pin_with_unlocked_dependency_is_not_confirmed_sha512_only_grant() { + assert_v4_pin_with_unlocked_dependency_is_not_confirmed(false).await; +} diff --git a/crates/socket-patch-core/src/hosted/engine.rs b/crates/socket-patch-core/src/hosted/engine.rs index eea16d612..2db7b72c6 100644 --- a/crates/socket-patch-core/src/hosted/engine.rs +++ b/crates/socket-patch-core/src/hosted/engine.rs @@ -1260,10 +1260,12 @@ pub fn yarn_berry_manifest_targets<'a>( /// The npm deps whose yarn classic pin reads the served tarball, one per /// distinct artifact URL: the project's `yarn.lock` is a classic lock that -/// names the package, the grant carries a sha512, and either the grant has -/// no sha1 for the `resolved` fragment yarn 1 keys its cache slot on -/// (#558), or the lock doesn't pin this artifact yet, so the tarball's own -/// dependencies must be checked against the lock (#591). +/// pins the artifact already or locks the package as a registry copy, and +/// the grant carries a sha512. The tarball gives the sha1 for the +/// `resolved` fragment yarn 1 keys its cache slot on when the grant has +/// none (#558), and its own dependencies are checked against the lock +/// (#591) on every run: a pin an earlier release wrote was never checked +/// (#1462). /// A lock the project's offline mirror refuses outright (`yarn_outer`: the /// mirror settings outside the project files) needs none. pub fn yarn_classic_artifact_targets<'a>( @@ -1285,11 +1287,10 @@ pub fn yarn_classic_artifact_targets<'a>( /// [`yarn_classic_artifact_targets`] over a classic `lock`, reading its /// registry copies through `read_copies`. The lock is read in one pass for -/// the artifacts it pins already, once a dep with a sha1 asks; the -/// registry-copy test reads every block, so it runs after that, at most -/// once per lock, and only when some dep still needs it (#1423): a rescan -/// whose grants all carry a sha1 and are pinned already never reads the -/// blocks. +/// the artifacts it pins already; the registry-copy test reads every +/// block, so it runs after that, at most once per lock, and only when some +/// dep is not pinned yet (#1423): a rescan whose deps are all pinned +/// already never reads the blocks. fn select_classic_artifacts<'a>( candidates: &'a [Candidate], lock: &str, @@ -1304,15 +1305,14 @@ fn select_classic_artifacts<'a>( .map(|c| &c.dep) .filter(|dep| dep.ecosystem == "npm" && dep.integrity.sha512.is_some()) .filter(|dep| { - dep.integrity.sha1.is_none() - || !pinned - .get_or_insert_with(|| quoted_fragment_urls(lock)) - .contains(dep.artifact_url.as_str()) - }) - .filter(|dep| { - registry_copies - .get_or_insert_with(|| read_copies.take().map_or_else(HashSet::new, |r| r(lock))) - .contains(&(crate::patch::redirect::full_name(dep), dep.version.clone())) + pinned + .get_or_insert_with(|| quoted_fragment_urls(lock)) + .contains(dep.artifact_url.as_str()) + || registry_copies + .get_or_insert_with(|| { + read_copies.take().map_or_else(HashSet::new, |r| r(lock)) + }) + .contains(&(crate::patch::redirect::full_name(dep), dep.version.clone())) }) .filter(|dep| seen.insert(dep.artifact_url.clone())) .collect() @@ -3250,10 +3250,10 @@ mod tests { ); } - /// #1423: the served tarball is read only for a dep the lock locks as - /// a registry copy and doesn't pin to the artifact yet. The lock's - /// blocks are read at most once however many deps there are, and not - /// at all on a rescan whose deps are all pinned already. + /// #1423: the served tarball is read only for a dep the lock pins to + /// the artifact or locks as a registry copy. The lock's blocks are + /// read at most once however many deps there are, and not at all on a + /// rescan whose deps are all pinned already. #[test] fn classic_artifact_targets_read_the_lock_blocks_at_most_once() { let mut lock = String::from("# yarn lockfile v1\n\n"); @@ -3277,13 +3277,17 @@ mod tests { classic_registry_copies(lock) }; - // Every dep pinned to its artifact already: nothing to fetch, and - // the blocks are never read. + // Every dep pinned to its artifact already: each tarball is still + // read, since an earlier release pinned it without checking its + // dependencies (#1462), but the blocks are never read. let mut pinned = lock.clone(); for c in &candidates { pinned.push_str(&format!("# \"{}#s1\"\n", c.dep.artifact_url)); } - assert!(select_classic_artifacts(&candidates, &pinned, counted).is_empty()); + assert_eq!( + select_classic_artifacts(&candidates, &pinned, counted).len(), + n + ); assert_eq!(reads.get(), 0); // Unpinned: every registry copy is selected once per artifact; a diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index 65c0d9067..fb87b2218 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -4098,13 +4098,24 @@ fn rewrite_yarn_classic_with( result .refused_yarn_classic_uuids .insert(dep.patch_uuid.clone()); + // A pin an earlier release wrote without this check (#1462) + // is left as found, but never confirmed: yarn installs the + // patched package from it without these dependencies. + let outcome = if blocks.iter().any(|b| { + classic_field(&b.lines, "resolved") + .is_some_and(|r| r.split('#').next() == Some(dep.artifact_url.as_str())) + }) { + "an earlier run's pin of it is not confirmed: yarn 1 installs only what the \ + lock names, so it installs the patched package without them" + } else { + "yarn 1 installs only what the lock names, so it is not pinned and stays \ + unpatched" + }; result.warnings.push(RewriteWarning { code: "redirect_yarn_classic_dep_manifest_unlocked".into(), detail: format!( "the patched {fname}@{} depends on {}, which yarn.lock does not lock; \ - yarn 1 installs only what the lock names, so it is not pinned and \ - stays unpatched. Lock them first (for example `yarn add {}`), then \ - re-run", + {outcome}. Lock them first (for example `yarn add {}`), then re-run", dep.version, missing.join(", "), missing.join(" ") diff --git a/crates/socket-patch-core/src/vex/verify.rs b/crates/socket-patch-core/src/vex/verify.rs index 1d1c9f2b2..0b4e58d78 100644 --- a/crates/socket-patch-core/src/vex/verify.rs +++ b/crates/socket-patch-core/src/vex/verify.rs @@ -479,7 +479,85 @@ async fn verify_hosted_copies( } None => record, }; - verify_every_copy(&copies.paths, record).await + verify_every_copy(&copies.paths, record).await?; + if Ecosystem::from_purl(purl) == Some(Ecosystem::Npm) && touches_npm_manifest(record) { + for path in &copies.paths { + if unresolved_npm_dependency(path).await.is_some() { + return Err(DEPENDENCY_NOT_INSTALLED.to_string()); + } + } + } + Ok(()) +} + +/// The omission tag of a hosted npm copy that carries the patched bytes +/// but whose patched `package.json` declares a dependency node cannot +/// resolve from it (#1462): the lock never locked it, so the package was +/// installed without it. +pub const DEPENDENCY_NOT_INSTALLED: &str = "dependency_not_installed"; + +/// Whether an npm `record` patches the package's own `package.json`, the +/// only way a patch changes the dependencies its package declares. +fn touches_npm_manifest(record: &PatchRecord) -> bool { + record + .files + .keys() + .any(|key| key.strip_prefix("package/").unwrap_or(key) == "package.json") +} + +/// The first `dependencies` entry of the installed npm copy at `pkg_path` +/// that node cannot resolve from it: no `node_modules/` in the copy +/// or in any directory above it (node's lookup, which also finds a +/// bundled, hoisted or pnpm-sibling copy). Optional dependencies may be +/// absent, and a copy outside a `node_modules` tree (a Plug'n'Play +/// install), an unreadable manifest or a name that is not a plain package +/// name is never evidence. +async fn unresolved_npm_dependency(pkg_path: &Path) -> Option { + if !pkg_path + .components() + .any(|c| c.as_os_str() == "node_modules") + { + return None; + } + let text = tokio::fs::read_to_string(pkg_path.join("package.json")) + .await + .ok()?; + let pkg: serde_json::Value = serde_json::from_str(&text).ok()?; + let optional = pkg + .get("optionalDependencies") + .and_then(serde_json::Value::as_object); + pkg.get("dependencies")? + .as_object()? + .keys() + .filter(|name| optional.is_none_or(|o| !o.contains_key(*name))) + .filter(|name| plain_npm_name(name)) + .find(|name| !node_resolves(pkg_path, name)) + .cloned() +} + +/// `name` is `pkg` or `@scope/pkg`, with no empty, `.` or `..` component +/// and no backslash, so joining it under `node_modules` stays there. +fn plain_npm_name(name: &str) -> bool { + let parts: Vec<&str> = name.split('/').collect(); + let shape_ok = match parts.as_slice() { + [_] => true, + [scope, _] => scope.starts_with('@') && scope.len() > 1, + _ => false, + }; + shape_ok + && !name.contains('\\') + && parts + .iter() + .all(|p| !p.is_empty() && *p != "." && *p != "..") +} + +/// Whether node resolves the package `name` from the package directory +/// `from`: `/node_modules/` exists for `from` or an ancestor +/// that is not itself a `node_modules` directory. +fn node_resolves(from: &Path, name: &str) -> bool { + from.ancestors() + .filter(|dir| dir.file_name().is_none_or(|n| n != "node_modules")) + .any(|dir| dir.join("node_modules").join(name).exists()) } /// Whether `rest` (what follows a matched `from` prefix) starts a new name From daca7bd4e51186d54d4b87f4beb0987253e59dde Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 02:34:24 +0000 Subject: [PATCH 3/6] Test node lookup for the dependency check Covers the hoisted, bundled and pnpm-isolated layouts, optional dependencies, copies outside node_modules and unsafe dependency names. Refs #1462 Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-core/src/vex/verify.rs | 60 ++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/crates/socket-patch-core/src/vex/verify.rs b/crates/socket-patch-core/src/vex/verify.rs index 0b4e58d78..864f27eee 100644 --- a/crates/socket-patch-core/src/vex/verify.rs +++ b/crates/socket-patch-core/src/vex/verify.rs @@ -1946,6 +1946,66 @@ mod tests { dir } + /// #1462: node's lookup from an installed copy finds a bundled, + /// hoisted or pnpm-sibling dependency; an optional one may be absent, + /// and a copy outside `node_modules` or an unsafe name is no evidence. + #[tokio::test] + async fn unresolved_npm_dependency_follows_node_lookup() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + let put = |rel: &str, text: &str| { + let path = root.join(rel); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(path, text).unwrap(); + }; + let manifest = r#"{"name":"lp","dependencies":{"a":"1","@s/b":"1","opt":"1", + "../../escape":"1"},"optionalDependencies":{"opt":"1"}}"#; + + // Hoisted layout: `a` and `@s/b` at the root; `@s/b` missing first. + put("node_modules/lp/package.json", manifest); + put("node_modules/a/package.json", "{}"); + let hoisted = root.join("node_modules/lp"); + assert_eq!( + unresolved_npm_dependency(&hoisted).await.as_deref(), + Some("@s/b") + ); + // Bundled inside the package itself. + put("node_modules/lp/node_modules/@s/b/package.json", "{}"); + assert_eq!(unresolved_npm_dependency(&hoisted).await, None); + + // pnpm's isolated store: dependencies are siblings of the package. + let store = "node_modules/.pnpm/lp@1.0.0/node_modules"; + put(&format!("{store}/lp/package.json"), manifest); + put(&format!("{store}/a/package.json"), "{}"); + let isolated = root.join(store).join("lp"); + assert_eq!( + unresolved_npm_dependency(&isolated).await.as_deref(), + Some("@s/b") + ); + put(&format!("{store}/@s/b/package.json"), "{}"); + assert_eq!(unresolved_npm_dependency(&isolated).await, None); + + // Not in a node_modules tree: never evidence. + put("unplugged/lp/package.json", manifest); + assert_eq!( + unresolved_npm_dependency(&root.join("unplugged/lp")).await, + None + ); + + for (name, plain) in [ + ("a", true), + ("@s/b", true), + ("@/b", false), + ("s/b", false), + ("../x", false), + ("@s/..", false), + ("a\\b", false), + ("", false), + ] { + assert_eq!(plain_npm_name(name), plain, "{name}"); + } + } + fn hosted_ctx(purl: &str, copies: HostedCopies) -> VendorContext { VendorContext { hosted: [(purl.to_string(), copies)].into_iter().collect(), From 04db3f5c4204c9c5143c195d82bc6c4e0b3939a4 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 03:02:41 +0000 Subject: [PATCH 4/6] Resolve npm deps from the copy's real path pnpm, bun isolated and npm linked installs reach a package through a root-level symlink into a store, where its dependencies sit beside it. The new vex dependency check walked up from the symlink, so it missed those siblings and omitted a correctly installed patch. It now starts from the real path, runs its file probes off the async runtime, reuses the shared npm name check, and words the omission so it fits every package manager. Refs #1462 Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/src/commands/vex.rs | 4 +- crates/socket-patch-core/src/vex/verify.rs | 95 +++++++++------------ 2 files changed, 43 insertions(+), 56 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/vex.rs b/crates/socket-patch-cli/src/commands/vex.rs index 3f7c12007..4cac3b911 100644 --- a/crates/socket-patch-cli/src/commands/vex.rs +++ b/crates/socket-patch-cli/src/commands/vex.rs @@ -1783,8 +1783,8 @@ fn omission_phrase(reason: &str) -> &'static str { } "stale_install" => "the installed copy is not patched", DEPENDENCY_NOT_INSTALLED => { - "the patched package.json declares a dependency that is not installed (the \ - lockfile does not lock it)" + "the patched package.json declares a dependency that is not installed beside \ + the package" } "mirror_overrides_source" => { "a Bundler mirror overrides the hosted source and installed bytes were not verified" diff --git a/crates/socket-patch-core/src/vex/verify.rs b/crates/socket-patch-core/src/vex/verify.rs index 864f27eee..8209f2ca9 100644 --- a/crates/socket-patch-core/src/vex/verify.rs +++ b/crates/socket-patch-core/src/vex/verify.rs @@ -507,48 +507,35 @@ fn touches_npm_manifest(record: &PatchRecord) -> bool { /// The first `dependencies` entry of the installed npm copy at `pkg_path` /// that node cannot resolve from it: no `node_modules/` in the copy -/// or in any directory above it (node's lookup, which also finds a -/// bundled, hoisted or pnpm-sibling copy). Optional dependencies may be -/// absent, and a copy outside a `node_modules` tree (a Plug'n'Play -/// install), an unreadable manifest or a name that is not a plain package -/// name is never evidence. +/// or in any directory above its real path (node's lookup, which also +/// finds a bundled, hoisted or pnpm/bun store-sibling copy behind a +/// root-linked symlink). Optional dependencies may be absent, and a copy +/// outside a `node_modules` tree (a Plug'n'Play install), an unreadable +/// manifest or a name that is not a safe package name is never evidence. async fn unresolved_npm_dependency(pkg_path: &Path) -> Option { - if !pkg_path - .components() - .any(|c| c.as_os_str() == "node_modules") - { - return None; - } - let text = tokio::fs::read_to_string(pkg_path.join("package.json")) - .await - .ok()?; - let pkg: serde_json::Value = serde_json::from_str(&text).ok()?; - let optional = pkg - .get("optionalDependencies") - .and_then(serde_json::Value::as_object); - pkg.get("dependencies")? - .as_object()? - .keys() - .filter(|name| optional.is_none_or(|o| !o.contains_key(*name))) - .filter(|name| plain_npm_name(name)) - .find(|name| !node_resolves(pkg_path, name)) - .cloned() -} - -/// `name` is `pkg` or `@scope/pkg`, with no empty, `.` or `..` component -/// and no backslash, so joining it under `node_modules` stays there. -fn plain_npm_name(name: &str) -> bool { - let parts: Vec<&str> = name.split('/').collect(); - let shape_ok = match parts.as_slice() { - [_] => true, - [scope, _] => scope.starts_with('@') && scope.len() > 1, - _ => false, - }; - shape_ok - && !name.contains('\\') - && parts - .iter() - .all(|p| !p.is_empty() && *p != "." && *p != "..") + let pkg_path = pkg_path.to_path_buf(); + crate::utils::fs::run_blocking(move || { + if !pkg_path + .components() + .any(|c| c.as_os_str() == "node_modules") + { + return None; + } + let text = std::fs::read_to_string(pkg_path.join("package.json")).ok()?; + let pkg: serde_json::Value = serde_json::from_str(&text).ok()?; + let real = std::fs::canonicalize(&pkg_path).ok()?; + let optional = pkg + .get("optionalDependencies") + .and_then(serde_json::Value::as_object); + pkg.get("dependencies")? + .as_object()? + .keys() + .filter(|name| optional.is_none_or(|o| !o.contains_key(*name))) + .filter(|name| crate::vendor::npm_common::is_safe_npm_name(name)) + .find(|name| !node_resolves(&real, name)) + .cloned() + }) + .await } /// Whether node resolves the package `name` from the package directory @@ -1947,8 +1934,9 @@ mod tests { } /// #1462: node's lookup from an installed copy finds a bundled, - /// hoisted or pnpm-sibling dependency; an optional one may be absent, - /// and a copy outside `node_modules` or an unsafe name is no evidence. + /// hoisted or pnpm-sibling dependency, also through a root-linked + /// symlink; an optional one may be absent, and a copy outside + /// `node_modules` is no evidence. #[tokio::test] async fn unresolved_npm_dependency_follows_node_lookup() { let tmp = tempfile::tempdir().unwrap(); @@ -1992,17 +1980,16 @@ mod tests { None ); - for (name, plain) in [ - ("a", true), - ("@s/b", true), - ("@/b", false), - ("s/b", false), - ("../x", false), - ("@s/..", false), - ("a\\b", false), - ("", false), - ] { - assert_eq!(plain_npm_name(name), plain, "{name}"); + // A root-linked symlink into the store (pnpm, bun isolated, npm + // linked): the lookup starts from the real path. + #[cfg(unix)] + { + std::os::unix::fs::symlink(root.join(store).join("lp"), root.join("node_modules/lp2")) + .unwrap(); + assert_eq!( + unresolved_npm_dependency(&root.join("node_modules/lp2")).await, + None + ); } } From fdde961b06784ba56fef4625555d5ab65cadd79a Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 03:14:14 +0000 Subject: [PATCH 5/6] Read the copy's package.json FIFO-safely The vex dependency check opened the installed package.json with a plain read, so a FIFO planted at that path would hang vex. It now reads through the shared regular-file opener, like every other workspace read. Refs #1462 Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-core/src/vex/verify.rs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/crates/socket-patch-core/src/vex/verify.rs b/crates/socket-patch-core/src/vex/verify.rs index 8209f2ca9..958eb8158 100644 --- a/crates/socket-patch-core/src/vex/verify.rs +++ b/crates/socket-patch-core/src/vex/verify.rs @@ -521,7 +521,8 @@ async fn unresolved_npm_dependency(pkg_path: &Path) -> Option { { return None; } - let text = std::fs::read_to_string(pkg_path.join("package.json")).ok()?; + let text = + crate::utils::fs::read_regular_to_string_sync(&pkg_path.join("package.json")).ok()?; let pkg: serde_json::Value = serde_json::from_str(&text).ok()?; let real = std::fs::canonicalize(&pkg_path).ok()?; let optional = pkg @@ -1980,6 +1981,18 @@ mod tests { None ); + // A FIFO where the manifest should be is never opened (it would + // block): no evidence. + #[cfg(unix)] + { + let fifo_dir = root.join("node_modules/fifo"); + std::fs::create_dir_all(&fifo_dir).unwrap(); + let c = + std::ffi::CString::new(fifo_dir.join("package.json").to_str().unwrap()).unwrap(); + assert_eq!(unsafe { libc::mkfifo(c.as_ptr(), 0o644) }, 0); + assert_eq!(unresolved_npm_dependency(&fifo_dir).await, None); + } + // A root-linked symlink into the store (pnpm, bun isolated, npm // linked): the lookup starts from the real path. #[cfg(unix)] From 5fffa6975b90b653f8b88bb920aaa8f4a88bb4dc Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 05:06:41 +0000 Subject: [PATCH 6/6] Widen the dependency_not_installed contract wording The vex check omits any hosted npm-family copy whose patched package.json declares a non-optional dependency that does not resolve, whatever the reason. A yarn classic pin from before the served-manifest check is one example, not the only cause, and the lookup starts from the copy's real path. Refs #1462 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VDPgtA8XT3RDR96zCibCmo --- crates/socket-patch-cli/CLI_CONTRACT.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 5ba116741..9a12353ff 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -1442,7 +1442,7 @@ command's integration tests check its output. | `vendor_ledger_missing` | `failed` (artifact-level: `uuid` + `details.{ecosystem,path}`, no purl) | repair (v5.0) and `vendor --check`: a lockfile references `.socket/vendor///` but the vendor ledger has no entry for it; repair no longer rebuilds ledger entries from lockfiles. Recovery: restore `.socket/vendor/state.json` from version control and re-run `repair`, or `git checkout -- ` and re-vendor. | | `vendor_wiring_unknown_revert_blocked` | `skipped` (beside the `failed`/`revert_failed` event) | vendor --revert: the ledger entry was reconstructed by a pre-v5 `repair` without wiring records and the live lockfile still resolves through the artifact — the revert refuses (fail-closed) instead of deleting a tarball the lock points at. Recovery: `socket-patch repair`, then restore the pre-vendor lock (or re-lock without the override) and re-run the revert. repair: an npm ledger entry whose `flavor` this release does not know (written by a newer socket-patch) is skipped, never health-checked or rebuilt, and the artifact, wiring and ledger stay as found (a lone `skipped` event; the run's exit is unaffected). Recovery: upgrade socket-patch. | | `stale_install` | `skipped` | vex (in-run `scan --mode hosted --vex`): a hosted stale-install probe found positively unpatched installed bytes, so the purl is omitted even under `--vex-no-verify` (see the gem / Python stale-install guards). | -| `dependency_not_installed` | `skipped` | vex: a hosted npm-family copy carries every patched file, but the patch changes its `package.json` and a dependency it declares (other than an optional one) does not resolve from the copy (no `node_modules/` in it or above it), because the lockfile never locked it (a yarn classic pin written before the served-manifest check, #1462). Omitted until the dependency is installed. | +| `dependency_not_installed` | `skipped` | vex: a hosted npm-family copy carries every patched file, but the patch changes its `package.json` and a dependency it declares (other than an optional one) does not resolve from the copy (no `node_modules/` in it or above its real path), for example because the lockfile never locked it (a yarn classic pin written before the served-manifest check, #1462). Omitted until the dependency is installed. | | `record_unavailable` | `skipped` | vex (manifest-less): a lockfile-wired patch has no local record (manifest, this run's hosted records or a pre-v5 redirect ledger, vendor ledger) and none could be fetched — `--offline`, transport error, 404, or a refused (paid) patch. Omitted, never attested from the `socket-patch.vendor.json` marker. | | `record_mismatch` | `skipped` | vex (manifest-less): the record found for a wired patch names another package or another patch uuid than the wiring. | | `vendor_unwired` | `skipped` | vex: a vendor-ledger entry whose committed artifact no lockfile/config wires any more (reverted lock, leftover ledger or artifact). Applies under `--no-verify` too. |