From 77890cc52c5dc218eccb333467f5d5e57a5678df Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 13:21:36 +0000 Subject: [PATCH 1/3] Start fix for #1379 Assisted-by: Claude Code:claude-opus-5-5 From 9e70b1b8de48894c966dba975720070996b5fab0 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 13:32:35 +0000 Subject: [PATCH 2/3] Flag yarn.lock deps an old vendor never locked vendor --check reported a yarn classic lock as healthy when an older release had vendored a patch that adds a dependency without locking it, so yarn 1 installed that dependency unpinned and offline installs failed. vendor --check now fails such an entry and names the missing descriptors, and repair reports it instead of leaving it silently. Fixes #1379 Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 2 +- .../src/commands/vendored_backend/repair.rs | 18 +++ .../tests/covgap_commands_vendor.rs | 78 +++++++++++++ .../src/formats/yarn/classic_deps.rs | 106 +++++++++++++++++- .../src/vendor/npm_flavor.rs | 29 ++++- .../src/vendor/yarn_classic_lock.rs | 99 +++++++++++++++- 6 files changed, 321 insertions(+), 11 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index abfd8da37..c6c1b5606 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -1464,7 +1464,7 @@ Every `--json` invocation emits a single JSON object that follows the **unified | `vendor_vlt_reinstall_required` | `skipped` (advisory; human: `Warning: …`) | vendor / scan / get `--mode vendored` (vlt), wet and dry runs, and in-sync reruns: (a) the run rewires an optional dependency, or an importer's `node_modules/` of an optional dependency still resolves into `node_modules/.vlt/`: from vlt 0.0.0-30 a plain `vlt install` (1.2.0: also `--force`) keeps that installed upstream copy linked; the detail says to run `vlt ci` (or delete `node_modules` and run `vlt install`) to link the vendored copy, and that vlt 0.0.0-30 … 1.0.4 install no optional dependency from the lock of a project that declares only optional dependencies (upgrade to 1.0.5 or later first); (b) otherwise, an importer's link of the dependency still resolves into `node_modules/.vlt/`: the detail names the links (`node_modules/`, `/node_modules/`) and says `vlt install` (or `vlt ci`) links the vendored copy — on a warm tree after a plain `vlt install` that is true of every vendored direct dependency; (c) an importer's link resolves into the vendored dir of the patch this run replaces (a new patch uuid), which the run removes: the detail names the links and says `vlt install` (or `vlt ci`) links the new vendored copy; (d) a redownload of the payload (vendor, or `repair` after a corrupt or missing payload) could not keep vlt's links to the package's own dependencies (its old `node_modules/` held more than links): the detail says to run `vlt ci` (or delete `node_modules` and run `vlt install`), since a plain `vlt install` does not re-link them. `repair` moves those links back into the downloaded payload when they are only links. The package is vendored either way; a run whose patch fails to apply emits neither. A wet `vendor --revert` (and the revert a vendored → hosted takeover runs, whose advisory joins `warnings[]`): (a) the revert moves an `optionalDependencies` spec back from the `file:` dir, or an optional importer's `node_modules/` still resolves into the vendored uuid dir: from vlt 0.0.0-30 a plain `vlt install` keeps that link (dangling once the dir is removed), so the detail says to run `vlt ci` (or delete `node_modules` and run `vlt install`) to link the restored copy, with the same vlt 1.0.5 note; (b) otherwise, an importer's link still resolves into the vendored uuid dir: the detail names the links and says `vlt install` (or `vlt ci`) links the restored copy. A dry-run revert emits neither. | | `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. | +| `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 (#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_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. | diff --git a/crates/socket-patch-cli/src/commands/vendored_backend/repair.rs b/crates/socket-patch-cli/src/commands/vendored_backend/repair.rs index 0b56ed576..42c8b946a 100644 --- a/crates/socket-patch-cli/src/commands/vendored_backend/repair.rs +++ b/crates/socket-patch-cli/src/commands/vendored_backend/repair.rs @@ -474,6 +474,24 @@ impl VendoredBackend<'_> { } match health { ArtifactHealth::Healthy => { + // A yarn classic block a pre-v5 vendor left naming a + // dependency with no lock block (#1379): the artifact + // is fine, but repair can't resolve the descriptor. + if entry.ecosystem == "npm" { + if let Some(detail) = + vendor::npm_flavor::dangling_lock_dependencies(&entry, &common.cwd) + .await + { + fail( + env, + common.json, + purl, + "vendor_dep_manifest_unlocked", + detail, + ); + continue; + } + } // vlt's `/.gitignore` and `.gitattributes` are not // part of the artifact: a missing or edited one is // simply rewritten. diff --git a/crates/socket-patch-cli/tests/covgap_commands_vendor.rs b/crates/socket-patch-cli/tests/covgap_commands_vendor.rs index 969a98cb1..9f8b51f65 100644 --- a/crates/socket-patch-cli/tests/covgap_commands_vendor.rs +++ b/crates/socket-patch-cli/tests/covgap_commands_vendor.rs @@ -1240,6 +1240,84 @@ fn yarn_classic_workspaces_vendor_warns_about_member_dir_installs() { } } +/// #1379: a yarn classic lock a pre-v5 release vendored with the +/// patched manifest's new dependency in the block's sub-map but no lock +/// block for it. Releases since #591 refuse to write that, but on such a +/// lock `vendor --check` reported `vendor_check_ok` and `repair` left it +/// alone, while yarn 1 installs the dependency unpinned. Both now fail the +/// entry and name the descriptor; locking it turns the check green. +#[test] +fn yarn_classic_check_and_repair_flag_a_dangling_dependency_descriptor() { + let fx = npm_fixture(); + std::fs::remove_file(fx.lock_path()).unwrap(); + std::fs::write( + fx.root().join("package.json"), + r#"{"name":"fixture","version":"1.0.0","private":true,"dependencies":{"left-pad":"^1.3.0"}}"#, + ) + .unwrap(); + let yarn_lock = fx.root().join("yarn.lock"); + std::fs::write( + &yarn_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 \"{REG_RESOLVED}\"\n \ + integrity {REG_INTEGRITY}\n" + ), + ) + .unwrap(); + let (code, env) = vendor_cli(fx.root(), &[]); + assert_eq!(code, 0, "{env:#}"); + let (code, env) = vendor_cli(fx.root(), &["--check"]); + assert_eq!(code, 0, "the fresh vendor is healthy: {env:#}"); + + // What a pre-v5 vendor of a patch adding `is-number` left behind. + let wired = std::fs::read_to_string(&yarn_lock).unwrap(); + assert!( + wired.contains("resolved \"file:./.socket/vendor/"), + "{wired}" + ); + assert!( + wired.ends_with("==\n"), + "left-pad is the last block: {wired}" + ); + let dangling = format!("{wired} dependencies:\n is-number \"^7.0.0\"\n"); + std::fs::write(&yarn_lock, &dangling).unwrap(); + + let (code, env) = vendor_cli(fx.root(), &["--check"]); + assert_eq!(code, 1, "{env:#}"); + let failed = find_event(&env, "failed", None).to_string(); + assert!( + failed.contains("vendor_check_failed") && failed.contains("is-number@^7.0.0"), + "{env:#}" + ); + + let cwd = fx.root().to_str().unwrap(); + let (code, stdout, stderr) = run_cli( + fx.root(), + &["repair", "--json", "--offline", "--cwd", cwd], + &[], + ); + assert_ne!(code, 0, "{stdout}\n{stderr}"); + let env: Value = serde_json::from_str(&stdout).unwrap(); + let failed = find_event(&env, "failed", Some("vendor_dep_manifest_unlocked")).to_string(); + assert!(failed.contains("is-number@^7.0.0"), "{env:#}"); + assert_eq!( + std::fs::read_to_string(&yarn_lock).unwrap(), + dangling, + "repair leaves the lock as it was" + ); + + // Once the descriptor is locked, the check is green again. + std::fs::write( + &yarn_lock, + format!("{dangling}\nis-number@^7.0.0:\n version \"7.0.0\"\n"), + ) + .unwrap(); + let (code, env) = vendor_cli(fx.root(), &["--check"]); + assert_eq!(code, 0, "{env:#}"); +} + /// #627: a symlinked lockfile (a lock shared with another checkout) is /// refused like hosted mode refuses it — `redirect_symlinked_file_unsupported`, /// exit 1 — instead of being renamed over: the link survives, its target diff --git a/crates/socket-patch-core/src/formats/yarn/classic_deps.rs b/crates/socket-patch-core/src/formats/yarn/classic_deps.rs index 77932443e..86e405419 100644 --- a/crates/socket-patch-core/src/formats/yarn/classic_deps.rs +++ b/crates/socket-patch-core/src/formats/yarn/classic_deps.rs @@ -53,14 +53,34 @@ pub(crate) fn with_manifest_dep_maps(lines: &[String], pkg: &Value) -> Vec Vec { + unresolved( + blocks, + manifest_dep_maps(pkg) + .into_iter() + .flat_map(|(_, deps)| deps) + .map(|(name, range)| format!("{name}@{range}")), + ) +} + +/// Every descriptor the dependency sub-maps of `lines` (one block) name +/// that no block of `blocks` is keyed by, sorted and deduplicated (#1379). +/// The writers refuse before leaving such a block; this is how a reader +/// finds one a release before that refusal already wrote. +pub(crate) fn unlocked_block_descriptors(blocks: &[LockBlock], lines: &[String]) -> Vec { + unresolved( + blocks, + block_dep_entries(lines) + .into_iter() + .map(|(name, range)| format!("{name}@{range}")), + ) +} + +fn unresolved(blocks: &[LockBlock], descriptors: impl Iterator) -> Vec { let locked: std::collections::BTreeSet = blocks .iter() .flat_map(|b| split_key_patterns(&b.key)) .collect(); - let mut missing: Vec = manifest_dep_maps(pkg) - .into_iter() - .flat_map(|(_, deps)| deps) - .map(|(name, range)| format!("{name}@{range}")) + let mut missing: Vec = descriptors .filter(|descriptor| !locked.contains(descriptor)) .collect(); missing.sort(); @@ -68,6 +88,54 @@ pub(crate) fn unlocked_descriptors(blocks: &[LockBlock], pkg: &Value) -> Vec Vec<(String, String)> { + let mut out = Vec::new(); + let mut in_map = false; + for line in lines.iter().skip(1) { + if let Some(rest) = body_field_line(line) { + in_map = is_dep_map_header(rest); + continue; + } + if !in_map { + continue; + } + let (name, rest) = yarn_token(line.trim_start()); + let (range, _) = yarn_token(rest.trim_start()); + if !name.is_empty() && !range.is_empty() { + out.push((name, range)); + } + } + out +} + +/// One classic-grammar token from the front of `s` — a `"`-quoted string +/// (with `\"` / `\\` escapes) or a bare run up to whitespace — and the +/// rest of `s`. +fn yarn_token(s: &str) -> (String, &str) { + let Some(quoted) = s.strip_prefix('"') else { + let end = s.find(char::is_whitespace).unwrap_or(s.len()); + return (s[..end].to_string(), &s[end..]); + }; + let mut token = String::new(); + let mut chars = quoted.char_indices(); + while let Some((i, c)) = chars.next() { + match c { + '"' => return (token, "ed[i + 1..]), + '\\' => { + if let Some((_, next)) = chars.next() { + token.push(next); + } + } + c => token.push(c), + } + } + (token, "") +} + /// Whether the sub-maps `lines` carries already are exactly the ones `pkg` /// declares (so a writer leaves them alone). pub(crate) fn dep_maps_match(lines: &[String], pkg: &Value) -> bool { @@ -145,6 +213,36 @@ is-odd@3.0.1:\n version \"3.0.1\"\n dependencies:\n is-number \"^6.0.0\"\n" assert!(unlocked_descriptors(&blocks, &serde_json::json!({})).is_empty()); } + #[test] + fn unlocked_block_descriptors_reads_the_blocks_own_sub_maps() { + // #1379: a block a pre-v5 vendor rewrote to a patched manifest + // without locking the new range. + let lock = format!( + "{LOCK}\nleft-pad@^1.3.0:\n version \"1.3.0\"\n dependencies:\n \ + is-number \"^7.0.0\"\n tag latest\n optionalDependencies:\n \ + \"@scope/opt\" \"^2.0.0\"\n integrity sha512-X==\n" + ); + let blocks = scan_blocks(&lock); + let pad = blocks.iter().find(|b| b.key == "left-pad@^1.3.0").unwrap(); + assert_eq!( + unlocked_block_descriptors(&blocks, &pad.lines), + vec!["is-number@^7.0.0", "tag@latest"] + ); + let odd = blocks.iter().find(|b| b.key == "is-odd@3.0.1").unwrap(); + assert!(unlocked_block_descriptors(&blocks, &odd.lines).is_empty()); + } + + #[test] + fn yarn_token_reads_quoted_and_bare_tokens() { + assert_eq!( + yarn_token("left-pad \"^1\""), + ("left-pad".into(), " \"^1\"") + ); + assert_eq!(yarn_token("\"@s/x\" y"), ("@s/x".into(), " y")); + assert_eq!(yarn_token("\"a\\\"b\""), ("a\"b".into(), "")); + assert_eq!(yarn_token(""), (String::new(), "")); + } + #[test] fn sub_maps_are_rebuilt_in_yarns_order() { let block = lines("is-odd@3.0.1:\n version \"3.0.1\"\n dependencies:\n is-number \"^6.0.0\"\n integrity sha512-X=="); diff --git a/crates/socket-patch-core/src/vendor/npm_flavor.rs b/crates/socket-patch-core/src/vendor/npm_flavor.rs index fdb0c4d73..6c4717707 100644 --- a/crates/socket-patch-core/src/vendor/npm_flavor.rs +++ b/crates/socket-patch-core/src/vendor/npm_flavor.rs @@ -809,9 +809,10 @@ pub async fn revert_npm_any( /// `vendor --check`'s wiring audit for an npm-family entry: `Err` (the /// human reason) when the lock installs a copy of the entry's -/// `name@version` the vendored artifact does not reach (#588). Only the -/// package-lock flavor audits its lock today; the other flavors' wiring is -/// left to the artifact check and `vex`. +/// `name@version` the vendored artifact does not reach (#588), and for +/// yarn classic when a wired block names a dependency the lock never +/// resolves (#1379). The other flavors' wiring is left to the artifact +/// check and `vex`. pub async fn check_npm_wiring(entry: &VendorEntry, project_root: &Path) -> Result<(), String> { match NpmLockFlavor::from_recorded(entry.flavor.as_deref()) { Some(NpmLockFlavor::PackageLock) => { @@ -827,10 +828,32 @@ pub async fn check_npm_wiring(entry: &VendorEntry, project_root: &Path) -> Resul None => Ok(()), } } + Some(NpmLockFlavor::YarnClassic) => { + match dangling_lock_dependencies(entry, project_root).await { + Some(detail) => Err(detail), + None => Ok(()), + } + } _ => Ok(()), } } +/// The detail naming the dependencies `entry`'s wired lock entries list +/// but the lock never resolves, for the flavors whose lock an older +/// release could leave that way: yarn classic before #591's refusal +/// (#1379). `None` for a sound lock and for every other flavor. +pub async fn dangling_lock_dependencies( + entry: &VendorEntry, + project_root: &Path, +) -> Option { + match NpmLockFlavor::from_recorded(entry.flavor.as_deref()) { + Some(NpmLockFlavor::YarnClassic) => { + yarn_classic_lock::dangling_descriptors(entry, project_root).await + } + _ => None, + } +} + /// [`revert_npm_any`] with full [`RevertOpts`], threaded through to the /// flavor backend that wired the entry. pub async fn revert_npm_any_opts( diff --git a/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs b/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs index 206721eed..f547b34e8 100644 --- a/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs +++ b/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs @@ -30,7 +30,9 @@ use crate::constants::SOCKET_DIR; use crate::formats::yarn::blocks::{ block_eol, classic_field, repin_classic_block, replace_block, scan_blocks, LockBlock, }; -use crate::formats::yarn::classic_deps::{unlocked_descriptors, with_manifest_dep_maps}; +use crate::formats::yarn::classic_deps::{ + unlocked_block_descriptors, unlocked_descriptors, with_manifest_dep_maps, +}; use crate::formats::yarn::patterns::{classic_key_real_name, split_key_patterns}; use crate::formats::yarn::source::{classic_copy_source, CopySource}; use crate::manifest::schema::PatchRecord; @@ -41,8 +43,8 @@ use crate::utils::socket_dir::remove_tree_and_prune; use super::common::refused; use super::npm_common::{ - guard_revert_uuid_dir, vendor_npm_family, NpmCommit, NpmCoords, NpmLockBackend, NpmStagedPack, - NpmVendorRequest, WireCx, + guard_revert_uuid_dir, parse_npm_purl, vendor_npm_family, NpmCommit, NpmCoords, NpmLockBackend, + NpmStagedPack, NpmVendorRequest, WireCx, }; use super::parse_memo::ParseMemo; use super::path::parse_vendor_path; @@ -538,6 +540,48 @@ pub(crate) async fn preflight_packages( gated } +/// `vendor --check` and `repair` for a yarn-classic entry (#1379): the +/// detail naming every dependency descriptor a block wired to `entry` +/// lists in its sub-maps but no block of `yarn.lock` resolves, or `None` +/// when there is none. Since #591 the writers refuse before leaving such +/// a block, but a lock an older release vendored can still carry one: +/// yarn 1 then installs the dependency unpinned and `--offline` installs +/// fail. A missing, unreadable or berry lock is `None` — that is the +/// liveness rule's and the flavor router's to report. +pub async fn dangling_descriptors(entry: &VendorEntry, project_root: &Path) -> Option { + let text = read_regular_to_string(&project_root.join(YARN_LOCK)) + .await + .ok()?; + if crate::formats::yarn::is_berry_lock(&text) { + return None; + } + let blocks = scan_blocks_shared(&text); + let mut missing: Vec = blocks + .iter() + .filter(|b| { + classic_field(&b.lines, "resolved") + .and_then(parse_vendor_path) + .is_some_and(|p| p.eco == "npm" && p.uuid == entry.uuid) + }) + .flat_map(|b| unlocked_block_descriptors(&blocks, &b.lines)) + .collect(); + if missing.is_empty() { + return None; + } + missing.sort(); + missing.dedup(); + let pkg = parse_npm_purl(&entry.base_purl) + .map(|(name, version)| format!("{name}@{version}")) + .unwrap_or_else(|| entry.base_purl.clone()); + Some(format!( + "{YARN_LOCK} wires {pkg} to depend on {}, which no {YARN_LOCK} block locks (a \ + socket-patch release before v5 wrote it this way); yarn 1 installs them unpinned \ + and `--offline` installs fail. Run `yarn install` (without \ + `--frozen-lockfile`) to lock them, then commit {YARN_LOCK}", + missing.join(", ") + )) +} + /// Undo one yarn-classic vendored package: restore the recorded lock blocks /// and remove the artifact dir. /// Test-only shorthand — production routes through @@ -1533,6 +1577,55 @@ wow@^1.0.0: ); } + /// #1379: a lock a pre-v5 release vendored with a dependency it never + /// locked. The vendor run refuses that now (#591), so the test vendors + /// a locked range and then drops its block, as the old writer left it. + /// `vendor --check`'s audit and repair's both name the dangling + /// descriptor; the sound lock passes. + #[tokio::test] + async fn issue_1379_check_flags_a_dangling_descriptor_an_old_release_wrote() { + let seven = "is-number@^7.0.0:\n version \"7.0.0\"\n"; + let lock = format!("{Y2_BEFORE}\n{seven}"); + let mut fx = fixture_with_lock(&lock).await; + patch_manifest( + &mut fx, + br#"{"name":"left-pad","version":"1.3.0","dependencies":{"is-number":"^7.0.0"}}"#, + ) + .await; + let (result, entry, _) = expect_done(fx.vendor(false).await); + assert!(result.success, "{:?}", result.error); + let entry = entry.expect("wired"); + assert_eq!(dangling_descriptors(&entry, fx.root()).await, None); + assert_eq!( + crate::vendor::npm_flavor::check_npm_wiring(&entry, fx.root()).await, + Ok(()) + ); + + let text = fx.lock_text().await; + assert!(text.contains(seven), "{text}"); + tokio::fs::write(fx.lock_path(), text.replace(seven, "")) + .await + .unwrap(); + forget_block_scans(); + let detail = dangling_descriptors(&entry, fx.root()) + .await + .expect("the dangling descriptor is reported"); + assert!( + detail.contains("left-pad@1.3.0") + && detail.contains("is-number@^7.0.0") + && detail.contains("yarn install"), + "{detail}" + ); + assert_eq!( + crate::vendor::npm_flavor::check_npm_wiring(&entry, fx.root()).await, + Err(detail.clone()) + ); + assert_eq!( + crate::vendor::npm_flavor::dangling_lock_dependencies(&entry, fx.root()).await, + Some(detail) + ); + } + /// #920: the `package.json` advisory is emitted once, by the run that /// wires — an in-sync re-run of a manifest-rewriting patch is a quiet /// AlreadyPatched. From f22885a461ebcb66b3abb960aa777838db639991 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 13:53:39 +0000 Subject: [PATCH 3/3] Skip dependencies upstream left unlocked The new yarn classic check also flagged a dependency that the package already listed before vendoring but that yarn 1 never locks, such as a workspace member, so vendor --check and repair failed on healthy projects. It now reports only dependencies the original lock entry did not name, and the contract describes the new vendor --check failure. Refs #1379 Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 8 ++- .../src/formats/yarn/classic_deps.rs | 60 +++++++++-------- .../src/vendor/npm_flavor.rs | 11 ++-- .../src/vendor/yarn_classic_lock.rs | 64 ++++++++++++++----- 4 files changed, 90 insertions(+), 53 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index c6c1b5606..577386f11 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -1464,7 +1464,7 @@ Every `--json` invocation emits a single JSON object that follows the **unified | `vendor_vlt_reinstall_required` | `skipped` (advisory; human: `Warning: …`) | vendor / scan / get `--mode vendored` (vlt), wet and dry runs, and in-sync reruns: (a) the run rewires an optional dependency, or an importer's `node_modules/` of an optional dependency still resolves into `node_modules/.vlt/`: from vlt 0.0.0-30 a plain `vlt install` (1.2.0: also `--force`) keeps that installed upstream copy linked; the detail says to run `vlt ci` (or delete `node_modules` and run `vlt install`) to link the vendored copy, and that vlt 0.0.0-30 … 1.0.4 install no optional dependency from the lock of a project that declares only optional dependencies (upgrade to 1.0.5 or later first); (b) otherwise, an importer's link of the dependency still resolves into `node_modules/.vlt/`: the detail names the links (`node_modules/`, `/node_modules/`) and says `vlt install` (or `vlt ci`) links the vendored copy — on a warm tree after a plain `vlt install` that is true of every vendored direct dependency; (c) an importer's link resolves into the vendored dir of the patch this run replaces (a new patch uuid), which the run removes: the detail names the links and says `vlt install` (or `vlt ci`) links the new vendored copy; (d) a redownload of the payload (vendor, or `repair` after a corrupt or missing payload) could not keep vlt's links to the package's own dependencies (its old `node_modules/` held more than links): the detail says to run `vlt ci` (or delete `node_modules` and run `vlt install`), since a plain `vlt install` does not re-link them. `repair` moves those links back into the downloaded payload when they are only links. The package is vendored either way; a run whose patch fails to apply emits neither. A wet `vendor --revert` (and the revert a vendored → hosted takeover runs, whose advisory joins `warnings[]`): (a) the revert moves an `optionalDependencies` spec back from the `file:` dir, or an optional importer's `node_modules/` still resolves into the vendored uuid dir: from vlt 0.0.0-30 a plain `vlt install` keeps that link (dangling once the dir is removed), so the detail says to run `vlt ci` (or delete `node_modules` and run `vlt install`) to link the restored copy, with the same vlt 1.0.5 note; (b) otherwise, an importer's link still resolves into the vendored uuid dir: the detail names the links and says `vlt install` (or `vlt ci`) links the restored copy. A dry-run revert emits neither. | | `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 (#1379); the lock is left as it is. Remedy: `yarn install` (without `--frozen-lockfile`) to lock them, then commit `yarn.lock`. | +| `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_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. | @@ -1994,7 +1994,11 @@ from that package's own `npm-shrinkwrap.json` whatever the lock says (#753); the reason names the entry and its shrinkwrapping dependency, which must be updated since re-vendoring cannot reach that copy. A package-lock entry also fails when npm's effective `allow-file` setting refuses its vendored `file:` tarball (#969, -see the npm vendored `allow-file` contract). Missing ledger entries fail with +see the npm vendored `allow-file` contract). A yarn classic entry fails when a +`yarn.lock` block wired to it names a dependency descriptor that no block locks +and that the recorded pre-vendor block did not already name (#1379: a release +before #591 could vendor a patch that way); the reason names the descriptors +and the remedy, `yarn install` without `--frozen-lockfile`. Missing ledger entries fail with `vendor_ledger_missing`: a manifest patch with no ledger entry, and a lockfile reference to `.socket/vendor///` that no ledger entry owns (the artifact-level event repair emits: `uuid` plus `details.{ecosystem,path}`, no diff --git a/crates/socket-patch-core/src/formats/yarn/classic_deps.rs b/crates/socket-patch-core/src/formats/yarn/classic_deps.rs index 86e405419..d962d5919 100644 --- a/crates/socket-patch-core/src/formats/yarn/classic_deps.rs +++ b/crates/socket-patch-core/src/formats/yarn/classic_deps.rs @@ -53,7 +53,7 @@ pub(crate) fn with_manifest_dep_maps(lines: &[String], pkg: &Value) -> Vec Vec { - unresolved( + unlocked_among( blocks, manifest_dep_maps(pkg) .into_iter() @@ -62,20 +62,12 @@ pub(crate) fn unlocked_descriptors(blocks: &[LockBlock], pkg: &Value) -> Vec Vec { - unresolved( - blocks, - block_dep_entries(lines) - .into_iter() - .map(|(name, range)| format!("{name}@{range}")), - ) -} - -fn unresolved(blocks: &[LockBlock], descriptors: impl Iterator) -> Vec { +/// Every descriptor of `descriptors` no block of `blocks` is keyed by, +/// sorted and deduplicated. +pub(crate) fn unlocked_among( + blocks: &[LockBlock], + descriptors: impl Iterator, +) -> Vec { let locked: std::collections::BTreeSet = blocks .iter() .flat_map(|b| split_key_patterns(&b.key)) @@ -88,6 +80,15 @@ fn unresolved(blocks: &[LockBlock], descriptors: impl Iterator) - missing } +/// The descriptors (`name@range`) the dependency sub-maps of `lines` (one +/// block) name: what yarn 1 resolves through other blocks of the lock. +pub(crate) fn block_dep_descriptors(lines: &[String]) -> Vec { + block_dep_entries(lines) + .into_iter() + .map(|(name, range)| format!("{name}@{range}")) + .collect() +} + /// The `(name, range)` entries of a block's `dependencies:` / /// `optionalDependencies:` sub-maps, read back from yarn 1's writing: /// a 4-space-indented `name range` pair, either side quoted when yarn @@ -113,27 +114,26 @@ fn block_dep_entries(lines: &[String]) -> Vec<(String, String)> { } /// One classic-grammar token from the front of `s` — a `"`-quoted string -/// (with `\"` / `\\` escapes) or a bare run up to whitespace — and the -/// rest of `s`. +/// or a bare run up to whitespace — and the rest of `s`. Quotes are +/// dropped and a `\\`-escape is kept verbatim (only skipped over when +/// finding the closing quote), as [`split_key_patterns`] reads a block +/// key, so the two compare alike. fn yarn_token(s: &str) -> (String, &str) { let Some(quoted) = s.strip_prefix('"') else { let end = s.find(char::is_whitespace).unwrap_or(s.len()); return (s[..end].to_string(), &s[end..]); }; - let mut token = String::new(); let mut chars = quoted.char_indices(); while let Some((i, c)) = chars.next() { match c { - '"' => return (token, "ed[i + 1..]), + '"' => return (quoted[..i].to_string(), "ed[i + 1..]), '\\' => { - if let Some((_, next)) = chars.next() { - token.push(next); - } + chars.next(); } - c => token.push(c), + _ => {} } } - (token, "") + (quoted.to_string(), "") } /// Whether the sub-maps `lines` carries already are exactly the ones `pkg` @@ -214,7 +214,7 @@ is-odd@3.0.1:\n version \"3.0.1\"\n dependencies:\n is-number \"^6.0.0\"\n" } #[test] - fn unlocked_block_descriptors_reads_the_blocks_own_sub_maps() { + fn block_dep_descriptors_reads_the_blocks_own_sub_maps() { // #1379: a block a pre-v5 vendor rewrote to a patched manifest // without locking the new range. let lock = format!( @@ -225,11 +225,15 @@ is-odd@3.0.1:\n version \"3.0.1\"\n dependencies:\n is-number \"^6.0.0\"\n" let blocks = scan_blocks(&lock); let pad = blocks.iter().find(|b| b.key == "left-pad@^1.3.0").unwrap(); assert_eq!( - unlocked_block_descriptors(&blocks, &pad.lines), + block_dep_descriptors(&pad.lines), + vec!["is-number@^7.0.0", "tag@latest", "@scope/opt@^2.0.0"] + ); + assert_eq!( + unlocked_among(&blocks, block_dep_descriptors(&pad.lines).into_iter()), vec!["is-number@^7.0.0", "tag@latest"] ); let odd = blocks.iter().find(|b| b.key == "is-odd@3.0.1").unwrap(); - assert!(unlocked_block_descriptors(&blocks, &odd.lines).is_empty()); + assert!(unlocked_among(&blocks, block_dep_descriptors(&odd.lines).into_iter()).is_empty()); } #[test] @@ -239,7 +243,7 @@ is-odd@3.0.1:\n version \"3.0.1\"\n dependencies:\n is-number \"^6.0.0\"\n" ("left-pad".into(), " \"^1\"") ); assert_eq!(yarn_token("\"@s/x\" y"), ("@s/x".into(), " y")); - assert_eq!(yarn_token("\"a\\\"b\""), ("a\"b".into(), "")); + assert_eq!(yarn_token("\"a\\\"b\" c"), ("a\\\"b".into(), " c")); assert_eq!(yarn_token(""), (String::new(), "")); } diff --git a/crates/socket-patch-core/src/vendor/npm_flavor.rs b/crates/socket-patch-core/src/vendor/npm_flavor.rs index 6c4717707..f7160b769 100644 --- a/crates/socket-patch-core/src/vendor/npm_flavor.rs +++ b/crates/socket-patch-core/src/vendor/npm_flavor.rs @@ -828,13 +828,10 @@ pub async fn check_npm_wiring(entry: &VendorEntry, project_root: &Path) -> Resul None => Ok(()), } } - Some(NpmLockFlavor::YarnClassic) => { - match dangling_lock_dependencies(entry, project_root).await { - Some(detail) => Err(detail), - None => Ok(()), - } - } - _ => Ok(()), + _ => match dangling_lock_dependencies(entry, project_root).await { + Some(detail) => Err(detail), + None => Ok(()), + }, } } diff --git a/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs b/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs index f547b34e8..8c44bd924 100644 --- a/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs +++ b/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs @@ -31,7 +31,7 @@ use crate::formats::yarn::blocks::{ block_eol, classic_field, repin_classic_block, replace_block, scan_blocks, LockBlock, }; use crate::formats::yarn::classic_deps::{ - unlocked_block_descriptors, unlocked_descriptors, with_manifest_dep_maps, + block_dep_descriptors, unlocked_among, unlocked_descriptors, with_manifest_dep_maps, }; use crate::formats::yarn::patterns::{classic_key_real_name, split_key_patterns}; use crate::formats::yarn::source::{classic_copy_source, CopySource}; @@ -546,8 +546,11 @@ pub(crate) async fn preflight_packages( /// when there is none. Since #591 the writers refuse before leaving such /// a block, but a lock an older release vendored can still carry one: /// yarn 1 then installs the dependency unpinned and `--offline` installs -/// fail. A missing, unreadable or berry lock is `None` — that is the -/// liveness rule's and the flavor router's to report. +/// fail. A descriptor the recorded pre-vendor block already named is +/// upstream's (a yarn 1 workspace member or `link:` dependency has no +/// block of its own), not the patch's, so it is not reported. A missing, +/// unreadable or berry lock is `None` — that is the liveness rule's and +/// the flavor router's to report. pub async fn dangling_descriptors(entry: &VendorEntry, project_root: &Path) -> Option { let text = read_regular_to_string(&project_root.join(YARN_LOCK)) .await @@ -555,28 +558,30 @@ pub async fn dangling_descriptors(entry: &VendorEntry, project_root: &Path) -> O if crate::formats::yarn::is_berry_lock(&text) { return None; } - let blocks = scan_blocks_shared(&text); - let mut missing: Vec = blocks + let upstream: std::collections::HashSet = entry + .wiring .iter() - .filter(|b| { - classic_field(&b.lines, "resolved") - .and_then(parse_vendor_path) - .is_some_and(|p| p.eco == "npm" && p.uuid == entry.uuid) - }) - .flat_map(|b| unlocked_block_descriptors(&blocks, &b.lines)) + .filter(|r| r.file == YARN_LOCK && r.kind == KIND_LOCK_BLOCK) + .filter_map(|r| json_to_lines(r.original.as_ref()?)) + .flat_map(|lines| block_dep_descriptors(&lines)) .collect(); + let blocks = scan_blocks_shared(&text); + let wired = blocks + .iter() + .filter(|b| block_vendor_uuid(&b.lines).as_deref() == Some(entry.uuid.as_str())) + .flat_map(|b| block_dep_descriptors(&b.lines)) + .filter(|d| !upstream.contains(d)); + let missing = unlocked_among(&blocks, wired); if missing.is_empty() { return None; } - missing.sort(); - missing.dedup(); let pkg = parse_npm_purl(&entry.base_purl) .map(|(name, version)| format!("{name}@{version}")) .unwrap_or_else(|| entry.base_purl.clone()); Some(format!( "{YARN_LOCK} wires {pkg} to depend on {}, which no {YARN_LOCK} block locks (a \ - socket-patch release before v5 wrote it this way); yarn 1 installs them unpinned \ - and `--offline` installs fail. Run `yarn install` (without \ + socket-patch release before v5 could vendor a patch that way); yarn 1 installs \ + them unpinned and `--offline` installs fail. Run `yarn install` (without \ `--frozen-lockfile`) to lock them, then commit {YARN_LOCK}", missing.join(", ") )) @@ -969,9 +974,16 @@ fn rewrite_classic_block( /// Does this block's `resolved` already point into `.socket/vendor/npm/` /// (ours — current or stale uuid)? pub(super) fn block_points_into_vendor(lines: &[String]) -> bool { + block_vendor_uuid(lines).is_some() +} + +/// The patch uuid this block's `resolved` points into under +/// `.socket/vendor/npm/`, if it does. +fn block_vendor_uuid(lines: &[String]) -> Option { classic_field(lines, "resolved") .and_then(parse_vendor_path) - .is_some_and(|p| p.eco == "npm") + .filter(|p| p.eco == "npm") + .map(|p| p.uuid) } // ─────────────────── shared yarn-lock text helpers ─────────────────── @@ -1626,6 +1638,26 @@ wow@^1.0.0: ); } + /// #1379's boundary: yarn 1 writes no block for a workspace member (or + /// a `link:` dependency), so an upstream block can name a descriptor + /// nothing locks. The vendored block carries it over from the recorded + /// original, and the check leaves it alone: only what the patch added + /// is socket-patch's to report. + #[tokio::test] + async fn issue_1379_check_ignores_an_unlocked_descriptor_upstream_had() { + let lock = format!("{Y2_BEFORE} dependencies:\n ws-member \"^1.0.0\"\n"); + let fx = fixture_with_lock(&lock).await; + let (result, entry, _) = expect_done(fx.vendor(false).await); + assert!(result.success, "{:?}", result.error); + let entry = entry.expect("wired"); + let text = fx.lock_text().await; + assert!( + text.contains("file:./.socket/vendor/") && text.contains("ws-member \"^1.0.0\""), + "{text}" + ); + assert_eq!(dangling_descriptors(&entry, fx.root()).await, None); + } + /// #920: the `package.json` advisory is emitted once, by the run that /// wires — an in-sync re-run of a manifest-rewriting patch is a quiet /// AlreadyPatched.