From b6e97dfa38b6ec13420494a989446ef13adc2df3 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 10:24:37 +0000 Subject: [PATCH 1/3] Start fix for #758 Assisted-by: Claude Code:claude-opus-5-5 From 4b6ae5fdb78a4c44d3ebba8ccbdeaa1ba58718f9 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 10:34:56 +0000 Subject: [PATCH 2/3] Stop attesting yarn 1 bundled copies as patched yarn 1 unpacks a package's bundleDependencies from its tarball into node_modules//node_modules and yarn.lock records nothing about them. A hosted or vendored scan wired the lock block, said nothing, and the VEX document claimed not_affected while the bundled copy shipped unpatched. When yarn.lock holds a Socket wiring, read the installed node_modules for bundled copies (each named bundle entry, its dependencies beside it, and everything nested under them). Lockfile discovery records each as an unpatched copy beside the parent's lock entry, so `vex` and the in-run VEX withhold the attestation and hosted scans name the copy with redirect_copy_unpatched. Vendored scans warn with vendor_bundled_instance_skipped, as they do for vlt. Fixes #758 Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 4 +- .../tests/e2e_redirect_yarn_classic_build.rs | 185 ++++++++++++++++ crates/socket-patch-core/src/vendor/mod.rs | 1 + .../src/vendor/vlt_bundled.rs | 6 +- .../src/vendor/yarn_classic_bundled.rs | 199 ++++++++++++++++++ .../src/vendor/yarn_classic_lock.rs | 61 ++++++ .../src/vex/discover/yarn.rs | 187 ++++++++++++++++ docs/ecosystems.md | 8 + 8 files changed, 646 insertions(+), 5 deletions(-) create mode 100644 crates/socket-patch-core/src/vendor/yarn_classic_bundled.rs diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index e2bc9b3d9..1dcbf1403 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -535,7 +535,7 @@ Recognition rules that hold for every ecosystem: * **Patch hosts.** A hosted reference counts only on `https://patch.socket.dev` or the `--patch-server-url` / `SOCKET_PATCH_SERVER_URL` origin, with no userinfo. The uuid is the URL's LAST canonical-uuid path segment, because grant tokens may themselves be uuid-shaped. The Go module prefix is fixed. `socket-patch-` registry / repository / source names count only through a pin. For a URL on any other host, see **Patch hosts** above. * **Pins, not definitions.** A registry, index or source *definition* alone (cargo `[registries]`, nuget ``, pom ``, uv index tables, `.npmrc`) never makes a reference, because it survives a reverted pin. Sections the package manager ignores are not read: npm's v2 `dependencies` mirror, a `.cargo/config.toml` shadowed by `.cargo/config`. A Socket pin inside a maven `` is diagnosed, never a reference. -* **Contested locks.** When one lock wires a package to a patch and another lock resolves the same `name@version` from a non-Socket source, the build's bytes depend on which package manager runs. The reference is then dropped with a `patched_ref_unattributable` diagnostic naming both files. This applies across npm / pnpm / yarn / bun and across uv / pylock / poetry / pdm / Pipfile.lock / requirements. PEP 723 script locks neither contest nor are contested. The root `requirements.txt` and its in-root `-r` includes count as one lock here: pip reads them as one requirement set, where a direct reference wins over a compatible `==` pin of the same version in another file of the set (#1086). A PyPI version matches under PEP 440 equality, so a requirements line pinned in any spelling that can only install the wired release (`==1.2.3.0`, `==v1.2.3`, `===1.2.3`) contests it like `==1.2.3` does; a range or wildcard (`~=1.2.3`, `==1.2.*`) does not (#1602). A **bundled** npm copy (`inBundle: true`, or v1 `bundled: true`, beneath a dependency that bundles it; the root project's own bundles install from the lock, #1618) of the same `name@version` contests the reference too, in the same lock, in the other npm lock of a shrinkwrap/package-lock pair, or in any other lock. npm unpacks it from the parent package's tarball, so no rewire reaches it and it stays unpatched. Bun and vlt unpack bundled copies the same way (#469, #471). pnpm unpacks bundled copies too, but its lock cannot tie one to a reference (see **Unattested references** below). For Bun, that is a `bun.lock` entry whose meta is `{ "bundled": true }`, or a `bun.lockb` record that a dependency edge with the `bundled` behavior bit reaches, including one Bun shares with a regular install. Such an entry is never a reference, and it contests the reference the same way. For vlt, the lock records no node for a bundled copy, so the copy is found in the installed store: a real package directory inside a store package's own `node_modules`. Hosted and vendored scans skip these copies with `redirect_bun_bundled_instance_skipped` / `redirect_vlt_bundled_instance_skipped` / `vendor_bundled_instance_skipped`. When a bundled copy is the only instance, vendoring refuses with `vendor_lock_entry_not_rewritable`. A reference withheld only because such an unreachable copy installs beside it (a bundled npm / Bun / vlt copy, an npm copy beneath a `hasShrinkwrap` package (#753) or one npm installs from a git / url / `file:` spec, or a yarn classic git or `file:` directory block of the same `name@version`) is still the rewriter's own wiring of exactly one package version: it is never attested, but `rollback`, `remove`, `list` and the hosted → vendored takeover treat it as an ordinary hosted pin and restore its upstream registry entry (#828), rather than refusing it with `hosted_wiring_contested`. Another entry of the **same** npm, Bun or yarn lock that resolves the wired `name@version` from a non-Socket source (for example a workspace member added after the rewire, then `npm install` / `bun install` / `yarn install`; for yarn berry, a registry locator such as `left-pad@npm:1.3.0` beside the hosted `…::__archiveUrl=` one) contests the reference too (#588). The package manager installs both entries, and that copy stays unpatched. Re-running `scan` / `vendor` rewires every copy. +* **Contested locks.** When one lock wires a package to a patch and another lock resolves the same `name@version` from a non-Socket source, the build's bytes depend on which package manager runs. The reference is then dropped with a `patched_ref_unattributable` diagnostic naming both files. This applies across npm / pnpm / yarn / bun and across uv / pylock / poetry / pdm / Pipfile.lock / requirements. PEP 723 script locks neither contest nor are contested. The root `requirements.txt` and its in-root `-r` includes count as one lock here: pip reads them as one requirement set, where a direct reference wins over a compatible `==` pin of the same version in another file of the set (#1086). A PyPI version matches under PEP 440 equality, so a requirements line pinned in any spelling that can only install the wired release (`==1.2.3.0`, `==v1.2.3`, `===1.2.3`) contests it like `==1.2.3` does; a range or wildcard (`~=1.2.3`, `==1.2.*`) does not (#1602). A **bundled** npm copy (`inBundle: true`, or v1 `bundled: true`, beneath a dependency that bundles it; the root project's own bundles install from the lock, #1618) of the same `name@version` contests the reference too, in the same lock, in the other npm lock of a shrinkwrap/package-lock pair, or in any other lock. npm unpacks it from the parent package's tarball, so no rewire reaches it and it stays unpatched. Bun and vlt unpack bundled copies the same way (#469, #471). pnpm unpacks bundled copies too, but its lock cannot tie one to a reference (see **Unattested references** below). For Bun, that is a `bun.lock` entry whose meta is `{ "bundled": true }`, or a `bun.lockb` record that a dependency edge with the `bundled` behavior bit reaches, including one Bun shares with a regular install. Such an entry is never a reference, and it contests the reference the same way. For vlt, the lock records no node for a bundled copy, so the copy is found in the installed store: a real package directory inside a store package's own `node_modules`. yarn classic's `yarn.lock` records nothing about bundled copies either (#758), so for a lock with a Socket wiring they are found in the installed `node_modules`: each package a parent's `bundleDependencies` / `bundledDependencies` names inside the parent's own `node_modules`, the dependencies of those beside them, and everything nested under them. Such a copy contests the reference in `yarn.lock` (the diagnostic names the parent's lock entry and the copy's directory); a lock-only `vex` with no `node_modules` can't see it. Hosted and vendored scans skip these copies with `redirect_bun_bundled_instance_skipped` / `redirect_vlt_bundled_instance_skipped` / `vendor_bundled_instance_skipped`, and a yarn classic hosted scan names one with `redirect_copy_unpatched`. When a bundled copy is the only instance, vendoring refuses with `vendor_lock_entry_not_rewritable`. A reference withheld only because such an unreachable copy installs beside it (a bundled npm / Bun / vlt copy, an npm copy beneath a `hasShrinkwrap` package (#753) or one npm installs from a git / url / `file:` spec, or a yarn classic git or `file:` directory block of the same `name@version`) is still the rewriter's own wiring of exactly one package version: it is never attested, but `rollback`, `remove`, `list` and the hosted → vendored takeover treat it as an ordinary hosted pin and restore its upstream registry entry (#828), rather than refusing it with `hosted_wiring_contested`. Another entry of the **same** npm, Bun or yarn lock that resolves the wired `name@version` from a non-Socket source (for example a workspace member added after the rewire, then `npm install` / `bun install` / `yarn install`; for yarn berry, a registry locator such as `left-pad@npm:1.3.0` beside the hosted `…::__archiveUrl=` one) contests the reference too (#588). The package manager installs both entries, and that copy stays unpatched. Re-running `scan` / `vendor` rewires every copy. * **Unattested references.** Some evidence shows a build may run a copy no wiring reaches, but cannot be tied to the reference's exact `name@version` or cannot say the build runs it. The reference then stays a reference: `list`, `rollback` and `remove` find it, and the ledgers' liveness gates (`vendor --check`, `scan`) keep treating the wiring as live, because re-running `scan` / `vendor` could never clear the evidence. Only `vex` omits it, as a run warning and as the `failed[].reason`. Four cases besides Gradle's (`vex_gradle_lock_above_base`): **pnpm bundled copies** (`vex_pnpm_bundled_copy`): a `packages:` entry's `bundledDependencies:` names the copies pnpm unpacks from that package's own tarball, but pnpm never locks them, so the bundled version is not in the lock. A pnpm reference whose package name a `bundledDependencies` list in the same lock names is omitted whatever its version (a missed attestation when the bundled copy is another version, never a false one), and `bundledDependencies: true` (or a value that cannot be read) omits every reference of that lock. It reaches no other lock. **deno.lock** (`vex_deno_lock_copy`): `deno install` installs a `package.json` project's npm dependencies from `deno.lock` and never reads `package-lock.json` / `pnpm-lock.yaml` / `yarn.lock`, so an npm-family reference whose `name@version` the `deno.lock` npm section also locks is omitted (#406). Whether Deno or another package manager populates `node_modules` (`nodeModulesDir: "manual"` allows either) is not in the files, so this holds whatever `nodeModulesDir` says. **Maven reactor roots** (`vex_maven_reactor_root`): a hosted pin in a root `pom.xml` that declares `` / `` is omitted, because a module's own literal `` overrides a root pin and module poms are not read (#261); hosted scan refuses such roots (`redirect_maven_multimodule_unsupported`), so this only covers pins an older release wrote. **Maven classifier copies** (`vex_maven_classifier_unpatched`): a `` dependency of a hosted-pinned GA is its own coordinate; the classifier-less pin does not retarget it and the grant serves only the main jar, so Maven fetches it from the next repository. When a live, non-profile classifier copy other than `sources` / `javadoc` (`tests`, a native or JDK-specific build) resolves any version other than the suffixed pin — its own literal ``, a `` entry with the same classifier, or no version in the root pom — the pin is omitted. `sources` / `javadoc` copies never run on a classpath and do not withhold the statement; hosted scan only warns about them (`redirect_maven_classifier_unsupported`) and skips the dep when an executable copy names the patched release. * **Shrinkwrap without a twin (#899).** npm >= 12 never reads `npm-shrinkwrap.json`: on a project whose only npm lock is the shrinkwrap it resolves the tree from the registry and writes a fresh `package-lock.json`, so a Socket reference there reaches npm <= 11 only. The reference is still discovered, so `list`, `rollback` and `remove` manage it, but `vex` omits the patch (`vex_npm_shrinkwrap_only`, as a run warning and as the `failed[].reason`). A `package-lock.json` beside it (wired the same way) makes it attestable again. Hosted and vendored runs still rewire the shrinkwrap and warn `redirect_npm_shrinkwrap_only` / `vendor_npm_shrinkwrap_only`. * **Lockless pins.** With no lock to name a version, a `Cargo.toml` pin (every declaration on `socket-patch-`, that registry defined on the patch host for the same uuid) or an exclusive nuget exact-id mapping is never a reference on its own, so v5.0 does not attest it (nor does `list` show it, or `rollback` / `remove` restore it — restore those files from version control). Only a pre-v5 redirect-ledger record naming a version the pin admits keeps it live. The same holds for a gem wired only in the `Gemfile` (the pre-bundler-2.6 mixed state, lock not converged). @@ -1621,7 +1621,7 @@ command's integration tests check its output. | `content_mismatch_overwritten` | `skipped` (warning) | apply (explicit `--allow-patch-conflicts` / `--force`): a file matched NEITHER beforeHash nor afterHash and was overwritten with the full verified patched content. This includes a file the patch adds (empty beforeHash) that already exists with other content. `--strict` turns this case into a `failed` event instead. Agent-mode `get` / `scan --json` carry it as a `(content_mismatch_overwritten) …` `warnings[]` entry (#1004). | | `vendor_lock_checksums_unsupported` / `vendor_stale_lock_checksum` | `failed` | vendor (gem): an ambiguous/platform CHECKSUMS entry, or a v1-wired lock whose stale token blocks the hot path (run `vendor --revert` + re-vendor). | | `redirect_unattributable` | `skipped` (hosted event) | scan/get `--mode hosted`: the rewriters would pin the patch, but lockfile discovery over the result reads that pin as contested (another lock or requirements file resolves the same version elsewhere, or the pin is not one the package manager consumes), so `vex`, `rollback`, `remove` and `vendor` would refuse it. The candidate is left out of the rewrite, so nothing is written for it; the detail carries discovery's findings. Exit code unchanged. | -| `redirect_copy_unpatched` | `warnings[]` (warning) | scan/get `--mode hosted` (pnpm, yarn): the pin was written, but its own lock also installs an unpatched copy of the same `name@version`, which no rewrite reaches: a pnpm `file:` directory or tarball dependency (#935), or a yarn `file:` tarball / directory, URL tarball or berry `portal:` / `link:` dependency declared under another name (`lp2@file:…`, #939, #1236; not repeated for a copy a yarn classic skip warning already names). The detail names the lock entry. Neither the run's own `--vex` nor a later `vex` attests the package (`vex` warns `patched_ref_unattributable`), but discovery still manages the pin, so `rollback` / `remove` undo it. | +| `redirect_copy_unpatched` | `warnings[]` (warning) | scan/get `--mode hosted` (pnpm, yarn): the pin was written, but its own lock also installs an unpatched copy of the same `name@version`, which no rewrite reaches: a pnpm `file:` directory or tarball dependency (#935), or a yarn `file:` tarball / directory, URL tarball or berry `portal:` / `link:` dependency declared under another name (`lp2@file:…`, #939, #1236; not repeated for a copy a yarn classic skip warning already names), or a yarn classic copy installed from a parent's `bundleDependencies` (#758). The detail names the lock entry (for a bundled copy, the parent's entry and the copy's directory). Neither the run's own `--vex` nor a later `vex` attests the package (`vex` warns `patched_ref_unattributable`), but discovery still manages the pin, so `rollback` / `remove` undo it. | | `redirect_pin_lockless` | `warnings[]` (warning) | scan/get `--mode hosted` (nuget, cargo): the pin was written without a lockfile that records its version, so `vex` cannot attest it and `rollback` / `remove` / `vendor` refuse it as unattributable. The detail names the lockfile to create (`dotnet restore --use-lock-file`, `cargo generate-lockfile`) before re-running the hosted scan. | | `redirect_pypi_stale_install` | `warnings[]` (warning) | Hosted Python redirect: readable installed files differ from patched hashes. Read-only, repeated on re-scan, and excludes the package from same-run VEX. See the "Python stale-install guard" section. | | `redirect_nuget_stale_global_package` / `vendor_nuget_stale_global_package` | `warnings[]` (warning, hosted) / `skipped` (warning, vendor advisory event) | scan `--mode hosted` / `vendor` / `scan --mode vendored` (nuget, v5.0 #352): NuGet's global packages folder (`NUGET_PACKAGES`, else `~/.nuget/packages`) already holds the patched package extracted from other bytes (its `.nupkg.metadata` `contentHash` is not the patched one). A patch keeps the upstream id and version and NuGet restores a package already in that folder without asking any source, so `dotnet restore` would keep the upstream bytes (silently without a lock, NU1403 against the re-pinned lock with one). The detail names the directory and the remedy: delete it, then `dotnet restore` (`dotnet nuget locals global-packages --clear` also works but empties the whole machine-wide folder, and the detail says so); CI caches of that folder must drop it too. Read-only like the gem guard (the folder is shared machine-wide), re-fired on every re-run until the copy is gone, skipped on `--dry-run`; a hosted purl it flags is excluded from the same run's `--vex` `assume_applied` and reported stale. A dir without `.nupkg.metadata` (a legacy `packages/` folder) is never judged. | diff --git a/crates/socket-patch-cli/tests/e2e_redirect_yarn_classic_build.rs b/crates/socket-patch-cli/tests/e2e_redirect_yarn_classic_build.rs index 3ed13e1ef..ce38670ef 100644 --- a/crates/socket-patch-cli/tests/e2e_redirect_yarn_classic_build.rs +++ b/crates/socket-patch-cli/tests/e2e_redirect_yarn_classic_build.rs @@ -1669,6 +1669,191 @@ async fn classic_outside_root_copy_under_another_name_is_named_and_not_attested( ); } +/// #758: `bund` (a local tarball) bundles its own left-pad@1.3.0, which +/// yarn 1 unpacks into `node_modules/bund/node_modules/left-pad` with no +/// yarn.lock block. The hosted pin of the registry block can't reach it, +/// so `scan --mode hosted` must name the bundled copy +/// (`redirect_copy_unpatched`), and neither its in-run VEX nor a later +/// `vex` over the installed tree may attest left-pad not_affected. +#[tokio::test(flavor = "multi_thread")] +#[serial_test::serial] +async fn classic_bundled_copy_is_named_and_not_attested() { + if !require_yarn_classic("e2e_redirect_yarn_classic_build (bundled copy)", |c| { + cache_env::isolate(c); + }) { + return; + } + // yarn 1.0–1.6 install nothing from a `file:` tarball, so the bundling + // parent can't be installed there (not a skip: there is nothing to + // test on those releases). + if !installs_file_tarballs(&yarn_classic_version()) { + println!( + "bundled copy: {} installs nothing from a `file:` tarball", + yarn_classic() + ); + return; + } + let tmp = tempfile::tempdir().unwrap(); + let proj = tmp.path().join("proj"); + std::fs::create_dir_all(&proj).unwrap(); + let cache = tmp.path().join("yarn-cache"); + let yarn_env = [("YARN_CACHE_FOLDER", cache.to_str().unwrap())]; + let manifest = |deps: &str| { + format!( + r#"{{"name":"bundled-classic","version":"0.0.0","private":true,"dependencies":{{"{DEP}":"{DEP_VERSION}"{deps}}}}}"# + ) + }; + std::fs::write(proj.join("package.json"), manifest("")).unwrap(); + let install = corepack( + &proj, + &yarn_classic(), + &["install", "--no-progress"], + &yarn_env, + ); + if !install.status.success() { + skip!( + "(bundled copy): fixture `yarn install` failed:\n{}", + String::from_utf8_lossy(&install.stderr) + ); + return; + } + let installed_dir = proj.join("node_modules").join(DEP); + let installed_orig = std::fs::read(installed_dir.join("index.js")).unwrap(); + + // The bundling parent: its tarball carries left-pad in its own + // node_modules, as `npm pack` writes a bundleDependencies package. + let bundsrc = tmp.path().join("bundsrc"); + copy_dir_recursive(&installed_dir, &bundsrc.join("node_modules").join(DEP)); + std::fs::write( + bundsrc.join("package.json"), + format!( + r#"{{"name":"bund","version":"1.0.0","main":"index.js","dependencies":{{"{DEP}":"{DEP_VERSION}"}},"bundledDependencies":["{DEP}"]}}"# + ), + ) + .unwrap(); + build_patched_tgz( + &bundsrc, + format!("module.exports = require('{DEP}');\n").as_bytes(), + &proj.join("bund-1.0.0.tgz"), + ); + std::fs::write( + proj.join("package.json"), + manifest(r#","bund":"file:./bund-1.0.0.tgz""#), + ) + .unwrap(); + let install = corepack( + &proj, + &yarn_classic(), + &["install", "--no-progress"], + &yarn_env, + ); + assert!( + install.status.success(), + "yarn install with the bundling parent failed:\n{}", + String::from_utf8_lossy(&install.stderr) + ); + let bundled = proj.join("node_modules/bund/node_modules").join(DEP); + assert_eq!( + std::fs::read(bundled.join("index.js")).ok(), + Some(installed_orig.clone()), + "fixture: yarn must unpack the bundled copy from bund's tarball" + ); + + let patched: Vec = [MARKER.as_bytes(), &installed_orig].concat(); + let tgz_path = tmp.path().join("patched.tgz"); + build_patched_tgz(&installed_dir, &patched, &tgz_path); + let tgz = std::fs::read(&tgz_path).unwrap(); + let server = mock_hosted_grant(&tgz, &installed_orig, &patched, "bundled fixture").await; + let api_url = server.uri(); + let api = [ + "--api-url", + api_url.as_str(), + "--org", + ORG, + "--api-token", + "fake", + ]; + let mut args = vec![ + "scan", + "--mode", + "hosted", + "--json", + "--yes", + "--cwd", + proj.to_str().unwrap(), + "--vex", + "out.vex.json", + "--vex-product", + PRODUCT, + ]; + args.extend(api); + let (code, stdout, stderr) = run_socket(&proj, &args); + println!("scan exit {code}\nstdout:\n{stdout}\nstderr:\n{stderr}"); + let env: serde_json::Value = serde_json::from_str(&stdout) + .unwrap_or_else(|e| panic!("scan --json output is not JSON: {e}\n{stdout}\n{stderr}")); + let lock = std::fs::read_to_string(proj.join("yarn.lock")).unwrap(); + assert!( + lock.contains(&format!("{UUID}/{DEP}-{DEP_VERSION}.tgz")), + "the registry left-pad block must still be pinned:\n{lock}" + ); + let named = env["warnings"].as_array().into_iter().flatten().any(|w| { + w["code"] == "redirect_copy_unpatched" + && w["detail"] + .as_str() + .is_some_and(|d| d.contains("node_modules/bund/node_modules/left-pad")) + }); + assert!(named, "the bundled copy must be named: {env}"); + let vex = std::fs::read_to_string(proj.join("out.vex.json")).unwrap_or_default(); + assert!( + !vex.contains("not_affected"), + "the in-run VEX must not attest left-pad:\n{vex}\n{env}" + ); + + // After a fresh install from the pinned lock, the bundled copy is still + // the unpatched bytes from bund's tarball, and `vex` must say nothing. + std::fs::remove_dir_all(proj.join("node_modules")).unwrap(); + let install = corepack( + &proj, + &yarn_classic(), + &["install", "--frozen-lockfile", "--no-progress"], + &yarn_env, + ); + assert!( + install.status.success(), + "frozen reinstall failed:\n{}", + String::from_utf8_lossy(&install.stderr) + ); + assert_eq!( + std::fs::read(installed_dir.join("index.js")).ok(), + Some(patched), + "the registry copy installs patched" + ); + assert_eq!( + std::fs::read(bundled.join("index.js")).ok(), + Some(installed_orig), + "the bundled copy stays unpatched" + ); + let mut args = vec![ + "vex", + "--cwd", + proj.to_str().unwrap(), + "--output", + "installed.vex.json", + "--product", + PRODUCT, + "--patch-server-url", + api_url.as_str(), + ]; + args.extend(api); + let (code, stdout, stderr) = run_socket(&proj, &args); + println!("vex exit {code}\nstdout:\n{stdout}\nstderr:\n{stderr}"); + let vex = std::fs::read_to_string(proj.join("installed.vex.json")).unwrap_or_default(); + assert!( + !vex.contains("not_affected"), + "vex over the installed tree must not attest left-pad:\n{vex}\n{stdout}\n{stderr}" + ); +} + /// A mock patch API granting one hosted patch of `DEP@DEP_VERSION` whose /// tarball is `tgz` (`index.js` from `orig` to `patched`). async fn mock_hosted_grant(tgz: &[u8], orig: &[u8], patched: &[u8], title: &str) -> MockServer { diff --git a/crates/socket-patch-core/src/vendor/mod.rs b/crates/socket-patch-core/src/vendor/mod.rs index b2ab62ebd..4c8b5cad6 100644 --- a/crates/socket-patch-core/src/vendor/mod.rs +++ b/crates/socket-patch-core/src/vendor/mod.rs @@ -111,6 +111,7 @@ pub mod vlt_bundled; pub mod vlt_lock; pub(crate) mod vlt_lock_text; pub(crate) mod yarn_berry_lock; +pub(crate) mod yarn_classic_bundled; pub(crate) mod yarn_classic_lock; pub(crate) mod yarn_copies; #[cfg(test)] diff --git a/crates/socket-patch-core/src/vendor/vlt_bundled.rs b/crates/socket-patch-core/src/vendor/vlt_bundled.rs index 5f18ac867..f91103307 100644 --- a/crates/socket-patch-core/src/vendor/vlt_bundled.rs +++ b/crates/socket-patch-core/src/vendor/vlt_bundled.rs @@ -153,7 +153,7 @@ impl RealDirs { /// Root-relative paths of the real (not symlinked) package directories in /// the `node_modules` dir `dir`, one `@scope` level deep. -fn real_package_dirs(root: &Path, dir: &str, budget: &mut usize) -> Vec { +pub(super) fn real_package_dirs(root: &Path, dir: &str, budget: &mut usize) -> Vec { let mut found = Vec::new(); let mut pending = vec![(dir.to_string(), true)]; while let Some((dir, scopes)) = pending.pop() { @@ -188,7 +188,7 @@ fn real_package_dirs(root: &Path, dir: &str, budget: &mut usize) -> Vec } /// The npm purl a package directory's `package.json` declares. -fn installed_purl(root: &Path, dir: &str) -> Option { +pub(super) fn installed_purl(root: &Path, dir: &str) -> Option { let text = crate::utils::fs::read_regular_to_string_sync(&root.join(dir).join("package.json")).ok()?; let manifest: serde_json::Value = serde_json::from_str(&text).ok()?; @@ -204,7 +204,7 @@ fn is_plain_segment(s: &str) -> bool { } /// An npm package name (`name` or `@scope/name`) made of plain segments. -fn is_package_name(name: &str) -> bool { +pub(super) fn is_package_name(name: &str) -> bool { match name.split_once('/') { Some((scope, bare)) => { scope.starts_with('@') && is_plain_segment(scope) && is_plain_segment(bare) diff --git a/crates/socket-patch-core/src/vendor/yarn_classic_bundled.rs b/crates/socket-patch-core/src/vendor/yarn_classic_bundled.rs new file mode 100644 index 000000000..c87dcfd98 --- /dev/null +++ b/crates/socket-patch-core/src/vendor/yarn_classic_bundled.rs @@ -0,0 +1,199 @@ +//! yarn classic's bundled copies (#758). yarn 1 unpacks a package's +//! `bundleDependencies` from its tarball into the package's own +//! `node_modules` (`node_modules//node_modules/`) and +//! records nothing about them in `yarn.lock`: no block, no `inBundle` +//! flag, not even the parent's `bundleDependencies`. So no hosted or +//! vendored rewire of the same `name@version` reaches such a copy, and +//! only the installed tree shows it. +//! +//! A bundle is what `npm pack` puts in the tarball: each named package +//! and, beside it in the parent's `node_modules`, the dependencies it +//! needs, plus everything nested under any of them. Packages yarn itself +//! placed in a parent's `node_modules` (a version the hoist could not +//! share) come from their lock blocks and are not bundled copies. +//! +//! Shared by lockfile discovery (which contests a ref of the same +//! `name@version`, like npm's `inBundle` copies) and the vendored scan +//! (which warns that the copy stays unpatched). + +use std::collections::{BTreeMap, BTreeSet, HashSet}; +use std::path::Path; + +use serde_json::Value; + +use super::vlt_bundled::{installed_purl, is_package_name, real_package_dirs}; + +/// A bundled copy found in the installed tree. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct BundledCopy { + /// Root-relative directory of the copy. + pub(crate) location: String, + /// The bundling package's name and version, as its `package.json` + /// says. + pub(crate) parent_name: String, + pub(crate) parent_version: String, +} + +/// Upper bound on the `node_modules` entries one walk inspects, so a huge +/// or hostile tree cannot stall `vex`. +const BUNDLED_WALK_LIMIT: usize = 50_000; + +/// purl → the first bundled copy of it installed under the project's +/// `node_modules` (empty without one). +pub(crate) async fn bundled_copies(root: &Path) -> BTreeMap { + let root = root.to_path_buf(); + tokio::task::spawn_blocking(move || bundled_copies_sync(&root)) + .await + .unwrap_or_else(|e| match e.try_into_panic() { + Ok(payload) => std::panic::resume_unwind(payload), + Err(e) => panic!("yarn classic bundled-copy walk failed: {e}"), + }) +} + +/// The scan warning detail for a bundled copy that a vendored rewire of +/// `name@version` cannot reach. +pub(crate) fn bundled_copy_detail(name: &str, version: &str, copy: &BundledCopy) -> String { + format!( + "yarn also installs {name}@{version} as a bundled copy at {:?}, unpacked from the \ + tarball of {}@{} with no yarn.lock block, and it CANNOT be rewired — that copy stays \ + UNPATCHED; vendor or update the bundling package to cover it", + copy.location, copy.parent_name, copy.parent_version + ) +} + +fn bundled_copies_sync(root: &Path) -> BTreeMap { + let mut copies = BTreeMap::new(); + let mut budget = BUNDLED_WALK_LIMIT; + // Directories inside a bundle, recorded with their whole subtree. + let mut bundled: HashSet = HashSet::new(); + let mut pending = vec!["node_modules".to_string()]; + while let Some(dir) = pending.pop() { + if !is_real_dir(root, &dir) { + continue; + } + for package in real_package_dirs(root, &dir, &mut budget) { + if bundled.contains(&package) { + continue; + } + let nested = format!("{package}/node_modules"); + pending.push(nested.clone()); + let Some(manifest) = read_manifest(root, &package) else { + continue; + }; + let names = bundled_names(&manifest); + if names.is_empty() { + continue; + } + let (Some(parent_name), Some(parent_version)) = ( + manifest.get("name").and_then(Value::as_str), + manifest.get("version").and_then(Value::as_str), + ) else { + continue; + }; + if !is_real_dir(root, &nested) { + continue; + } + for dir in bundle_closure(root, &nested, names) { + for copy in subtree(root, &dir, &mut budget) { + if let Some(purl) = installed_purl(root, ©) { + copies.entry(purl).or_insert_with(|| BundledCopy { + location: copy.clone(), + parent_name: parent_name.to_string(), + parent_version: parent_version.to_string(), + }); + } + bundled.insert(copy); + } + } + } + } + copies +} + +/// The root-relative directories of the bundle in `node_modules` dir +/// `nested`: each of `names` installed there, and each of their +/// dependencies installed there in turn. +fn bundle_closure(root: &Path, nested: &str, names: Vec) -> BTreeSet { + let mut found = BTreeSet::new(); + let mut queue = names; + while let Some(name) = queue.pop() { + if !is_package_name(&name) { + continue; + } + let dir = format!("{nested}/{name}"); + let scope_ok = match name.split_once('/') { + Some((scope, _)) => is_real_dir(root, &format!("{nested}/{scope}")), + None => true, + }; + if found.contains(&dir) || !scope_ok || !is_real_dir(root, &dir) { + continue; + } + if let Some(manifest) = read_manifest(root, &dir) { + queue.extend(dependency_names(&manifest)); + } + found.insert(dir); + } + found +} + +/// `dir` and every real package directory nested in its `node_modules`. +fn subtree(root: &Path, dir: &str, budget: &mut usize) -> Vec { + let mut out = vec![dir.to_string()]; + let mut pending = vec![format!("{dir}/node_modules")]; + while let Some(nm) = pending.pop() { + if !is_real_dir(root, &nm) { + continue; + } + for child in real_package_dirs(root, &nm, budget) { + pending.push(format!("{child}/node_modules")); + out.push(child); + } + } + out +} + +/// The names a manifest bundles: `bundleDependencies` (npm) or its legacy +/// `bundledDependencies` alias. `true` bundles every `dependencies` key, +/// an array names them, and an object bundles its keys (as npm-bundled +/// reads it). +fn bundled_names(manifest: &Value) -> Vec { + let mut names = Vec::new(); + for key in ["bundleDependencies", "bundledDependencies"] { + match manifest.get(key) { + Some(Value::Bool(true)) => names.extend( + manifest + .get("dependencies") + .and_then(Value::as_object) + .into_iter() + .flat_map(|deps| deps.keys().cloned()), + ), + Some(Value::Array(list)) => { + names.extend(list.iter().filter_map(Value::as_str).map(str::to_string)) + } + Some(Value::Object(map)) => names.extend(map.keys().cloned()), + _ => {} + } + } + names +} + +/// The `dependencies` and `optionalDependencies` keys of a manifest. +fn dependency_names(manifest: &Value) -> Vec { + ["dependencies", "optionalDependencies"] + .iter() + .filter_map(|k| manifest.get(*k).and_then(Value::as_object)) + .flat_map(|deps| deps.keys().cloned()) + .collect() +} + +fn read_manifest(root: &Path, dir: &str) -> Option { + let text = + crate::utils::fs::read_regular_to_string_sync(&root.join(dir).join("package.json")).ok()?; + serde_json::from_str(&text).ok() +} + +/// Whether root-relative `rel` is a directory and not a symlink (a link +/// may point outside the project, and yarn never links a bundled copy). +fn is_real_dir(root: &Path, rel: &str) -> bool { + std::fs::symlink_metadata(root.join(rel)).is_ok_and(|m| m.is_dir()) +} 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 5190f9b7c..d43beb131 100644 --- a/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs +++ b/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs @@ -128,6 +128,24 @@ impl NpmLockBackend for YarnClassicBackend { })?; warnings.extend(skipped); warnings.extend(other_name.into_iter().map(|(w, _)| w)); + // A copy yarn unpacked from a parent's `bundleDependencies` has no + // block (#758): it stays unpatched beside whatever this run wires. + let bundled = match crate::utils::purl::npm_purl(&coords.name, &coords.version) { + Some(purl) => super::yarn_classic_bundled::bundled_copies(project_root) + .await + .remove(&purl), + None => None, + }; + if let Some(copy) = bundled { + warnings.push(VendorWarning::new( + "vendor_bundled_instance_skipped", + super::yarn_classic_bundled::bundled_copy_detail( + &coords.name, + &coords.version, + ©, + ), + )); + } Ok(YarnClassicPlan { text, candidate_keys, @@ -1972,6 +1990,49 @@ wow@^1.0.0: } } + /// #758: yarn 1 unpacks a parent's `bundleDependencies` copy of + /// left-pad@1.3.0 into `node_modules//node_modules/left-pad` + /// with no yarn.lock block, so vendoring can't reach it: the registry + /// block is still wired and the run names the bundled copy. Control: a + /// parent that bundles nothing (yarn placed the nested copy from the + /// lock) is not named. + #[tokio::test] + async fn issue_758_bundled_copy_is_named() { + for (bundles, named) in [(true, true), (false, false)] { + let fx = fixture_with_lock(Y2_BEFORE).await; + let parent = fx.root().join("node_modules/bund"); + tokio::fs::create_dir_all(parent.join("node_modules/left-pad")) + .await + .unwrap(); + let manifest = if bundles { + r#"{"name":"bund","version":"1.0.0","dependencies":{"left-pad":"1.3.0"},"bundledDependencies":["left-pad"]}"# + } else { + r#"{"name":"bund","version":"1.0.0","dependencies":{"left-pad":"1.3.0"}}"# + }; + tokio::fs::write(parent.join("package.json"), manifest) + .await + .unwrap(); + tokio::fs::write( + parent.join("node_modules/left-pad/package.json"), + r#"{"name":"left-pad","version":"1.3.0"}"#, + ) + .await + .unwrap(); + let (result, entry, warnings) = expect_done(fx.vendor(false).await); + assert!(result.success, "{:?}", result.error); + assert_eq!(entry.unwrap().wiring.len(), 1, "the registry block"); + let bundled: Vec<&VendorWarning> = warnings + .iter() + .filter(|w| { + w.code == "vendor_bundled_instance_skipped" + && w.detail.contains("node_modules/bund/node_modules/left-pad") + && w.detail.contains("bund@1.0.0") + }) + .collect(); + assert_eq!(bundled.len(), usize::from(named), "{bundles}: {warnings:?}"); + } + } + /// #1236 follow-up: a `file:` TARBALL declared under another dependency /// name (`"lp2": "file:./forks/left-pad-1.3.0.tgz"`) is read from the /// tarball's own `package.json` and named too; the registry block is diff --git a/crates/socket-patch-core/src/vex/discover/yarn.rs b/crates/socket-patch-core/src/vex/discover/yarn.rs index c1292f35d..7ddfb1844 100644 --- a/crates/socket-patch-core/src/vex/discover/yarn.rs +++ b/crates/socket-patch-core/src/vex/discover/yarn.rs @@ -157,6 +157,51 @@ async fn extract_classic(ctx: &DiscoverCtx<'_>, entries: Vec, out: &m } } record_copies(ctx, copies, out).await; + record_bundled_copies(ctx, &entries, out).await; +} + +/// Record each copy yarn 1 unpacked from a parent's `bundleDependencies` +/// as an unpatched copy beside the parent's lock entry (#758). yarn.lock +/// has no block for it and doesn't say the parent bundles anything, so no +/// Socket wiring of the same `name@version` reaches it, and only the +/// installed tree shows it +/// ([`crate::vendor::yarn_classic_bundled::bundled_copies`]). +async fn record_bundled_copies(ctx: &DiscoverCtx<'_>, entries: &[YarnEntry], out: &mut Discovery) { + // Only a Socket wiring in this lock can be contested by a bundled copy + // (the walk reads around the view, so it is not paid for nothing). + if !out + .refs + .iter() + .any(|r| r.source_file == std::path::Path::new(YARN_LOCK)) + { + return; + } + // The bundle is an installed tree: only a disk has one. + let Some(root) = ctx.disk_root() else { + return; + }; + for (purl, copy) in crate::vendor::yarn_classic_bundled::bundled_copies(root).await { + let parent_key = entries + .iter() + .find(|e| { + e.live + && classic_key_real_name(&e.patterns) == Some(copy.parent_name.as_str()) + && classic_field(&e.block.lines, "version") + == Some(copy.parent_version.as_str()) + }) + .map(|e| e.block.key.clone()) + .unwrap_or_else(|| format!("{}@{}", copy.parent_name, copy.parent_version)); + out.unpatched_copy( + YARN_LOCK, + Some(purl), + &parent_key, + &format!( + "bundles its own copy at {:?}, unpacked from the package's tarball with no \ + {YARN_LOCK} block, which no Socket wiring reaches", + copy.location + ), + ); + } } /// The copy a classic `file:` / url block installs, to be read for the @@ -2916,4 +2961,146 @@ mod tests { assert!(out.refs.is_empty()); assert_eq!(diag_codes(&out), vec![DIAG_LOCKFILE_UNREADABLE]); } + + /// REGRESSION (#758): yarn classic unpacks a `bundledDependencies` + /// copy from its parent's tarball into `/node_modules/` + /// and yarn.lock records nothing about it, so no hosted or vendored + /// wiring of the same `name@version` reaches it. An installed bundled + /// copy (named directly, through `bundleDependencies: true`, or as a + /// dependency of a bundled package) contests a hosted and a vendored + /// ref alike. A bundled copy of another version, and a nested copy + /// yarn placed there from the lock (its parent bundles nothing), contest + /// nothing. + #[tokio::test] + async fn classic_installed_bundled_copy_contests_the_ref() { + const PURL: &str = "pkg:npm/left-pad@1.3.0"; + let lp = hosted_url("npm", "left-pad", "1.3.0", UUID_A, "left-pad-1.3.0.tgz"); + let wired = [ + ( + classic_block( + "left-pad@^1.3.0", + "1.3.0", + &format!("{lp}#{SHA1}"), + Some(SRI), + ), + UUID_A, + WiringMode::Hosted, + ), + ( + classic_block( + "left-pad@^1.3.0", + "1.3.0", + &format!("file:./.socket/vendor/npm/{UUID_B}/left-pad-1.3.0.tgz#{SHA1}"), + Some(SRI), + ), + UUID_B, + WiringMode::Vendored, + ), + ]; + let parent_block = classic_block( + "\"@bh/bund@^1.0.0\"", + "1.0.0", + &format!("https://registry.yarnpkg.com/@bh/bund/-/bund-1.0.0.tgz#{SHA1}"), + Some(SRI), + ); + // (what, parent package.json, copy dir under the parent, version, contested) + let cases = [ + ( + "named", + r#"{"name":"@bh/bund","version":"1.0.0","dependencies":{"left-pad":"1.3.0"},"bundledDependencies":["left-pad"]}"#, + "node_modules/left-pad", + "1.3.0", + true, + ), + ( + "bundleDependencies: true", + r#"{"name":"@bh/bund","version":"1.0.0","dependencies":{"left-pad":"1.3.0"},"bundleDependencies":true}"#, + "node_modules/left-pad", + "1.3.0", + true, + ), + ( + "dependency of a bundled package", + r#"{"name":"@bh/bund","version":"1.0.0","dependencies":{"wrap":"1.0.0"},"bundleDependencies":["wrap"]}"#, + "node_modules/left-pad", + "1.3.0", + true, + ), + ( + "nested under a bundled package", + r#"{"name":"@bh/bund","version":"1.0.0","dependencies":{"wrap":"1.0.0"},"bundleDependencies":["wrap"]}"#, + "node_modules/wrap/node_modules/left-pad", + "1.3.0", + true, + ), + ( + "another version", + r#"{"name":"@bh/bund","version":"1.0.0","dependencies":{"left-pad":"1.2.0"},"bundledDependencies":["left-pad"]}"#, + "node_modules/left-pad", + "1.2.0", + false, + ), + ( + "not bundled", + r#"{"name":"@bh/bund","version":"1.0.0","dependencies":{"left-pad":"1.3.0"}}"#, + "node_modules/left-pad", + "1.3.0", + false, + ), + ]; + for (wired_block, uuid, mode) in &wired { + for (what, parent_manifest, copy, version, contested) in cases { + let p = Project::new(); + p.write( + "yarn.lock", + classic(&[parent_block.clone(), wired_block.clone()]), + ); + p.write( + "node_modules/left-pad/package.json", + r#"{"name":"left-pad","version":"1.3.0"}"#, + ); + let parent = "node_modules/@bh/bund"; + p.write(&format!("{parent}/package.json"), parent_manifest); + p.write( + &format!("{parent}/node_modules/wrap/package.json"), + r#"{"name":"wrap","version":"1.0.0","dependencies":{"left-pad":"1.3.0"}}"#, + ); + p.write( + &format!("{parent}/{copy}/package.json"), + format!(r#"{{"name":"left-pad","version":"{version}"}}"#), + ); + let out = run(&p).await; + if contested { + assert_refs(&out, &[]); + let detail = out + .diagnostics + .iter() + .find(|d| d.code == DIAG_REF_UNATTRIBUTABLE) + .map(|d| d.detail.as_str()) + .unwrap_or_else(|| panic!("{what} {mode:?}: {:?}", out.diagnostics)); + assert!( + detail.contains(&format!("{parent}/{copy}")) + && detail.contains("@bh/bund@^1.0.0") + && detail.contains("bundles its own copy"), + "{what}: the diagnostic names the copy and its parent: {detail}" + ); + // Still a pin the lock records (#1195). + let shadowed: Vec<_> = out.shadowed.iter().map(|r| r.uuid.as_str()).collect(); + assert_eq!(shadowed, [*uuid], "{what} {mode:?}"); + assert!( + out.unpatched_copies.iter().any(|c| c.purl == PURL), + "{what}: {:?}", + out.unpatched_copies + ); + } else { + assert_refs(&out, &[(PURL, uuid, *mode)]); + assert!( + out.diagnostics.is_empty(), + "{what} {mode:?}: {:?}", + out.diagnostics + ); + } + } + } + } } diff --git a/docs/ecosystems.md b/docs/ecosystems.md index 60a2a8fce..0cd7ee3f6 100644 --- a/docs/ecosystems.md +++ b/docs/ecosystems.md @@ -408,6 +408,14 @@ Each Legacy format has an upgrade path and an undo path. Both work in v5: records no package name in the lock and is not detected. When every entry of the package is such a copy (git, `file:` directory, non-registry tarball or `link:`), vendoring refuses with `vendor_lock_entry_not_rewritable` naming them, since `yarn install` can't help. +- **yarn classic bundled copies** — yarn 1 unpacks a package's `bundleDependencies` + from its tarball into the package's own `node_modules` and `yarn.lock` records + nothing about them (#758), so no pin or vendored wiring reaches such a copy. When + `yarn.lock` holds a Socket wiring, the installed `node_modules` is read for them: + hosted scans name a bundled copy of the patched `name@version` with + `redirect_copy_unpatched`, vendored scans with `vendor_bundled_instance_skipped`, and + `vex` attests nothing for the package while the copy is installed. A lock-only `vex` + (no `node_modules`) can't see the copy. - **bun** — text `bun.lock` lockfileVersion 0, 1 or 2: 0 is the `--save-text-lockfile` opt-in lock of Bun 1.1.39–1.1.45, 1 the 1.2–1.3 default, 2 the 1.4+ default; all three emit one `packages` grammar, so registry entries rewrite identically. Any other or From 44f1a19cc8491931bd94394a45e0e6573379a306 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 10:47:48 +0000 Subject: [PATCH 3/3] Cover yarn 1 bundles in workspaces and peers Follow-ups from review of the bundled-copy fix: - Walk each workspace member's own node_modules too, reached through its link in the root node_modules, so a bundle yarn couldn't hoist is still found. Links leaving the project are never followed. - Count a bundled package's peers as part of the bundle, as npm 7+ packs them. - Name every bundled copy a parent ships, not just the first one: the hosted warning was de-duplicated per lock entry. - Tell the user to vendor or update the bundling package, not to "depend on the registry release", which they already do. - When the only installed copy is bundled, vendoring now refuses with vendor_lock_entry_not_rewritable naming it instead of suggesting `yarn install`. Refs #758 Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-core/src/hosted/engine.rs | 12 +- .../src/vendor/yarn_classic_bundled.rs | 160 +++++++++++++++++- .../src/vendor/yarn_classic_lock.rs | 78 +++++++-- .../socket-patch-core/src/vex/discover/mod.rs | 17 ++ .../src/vex/discover/testing/golden.rs | 18 +- .../src/vex/discover/yarn.rs | 2 +- 6 files changed, 262 insertions(+), 25 deletions(-) diff --git a/crates/socket-patch-core/src/hosted/engine.rs b/crates/socket-patch-core/src/hosted/engine.rs index 85f235c54..c2b37f684 100644 --- a/crates/socket-patch-core/src/hosted/engine.rs +++ b/crates/socket-patch-core/src/hosted/engine.rs @@ -1930,7 +1930,7 @@ async fn unattributed_pins( // [`Discovery::shadowed`]: crate::vex::discover::Discovery::shadowed let mut copy_left: Vec = Vec::new(); let mut copy_shadowed: BTreeSet = BTreeSet::new(); - let mut named: BTreeSet<(&std::path::Path, &str)> = BTreeSet::new(); + let mut named: BTreeSet<(&std::path::Path, &str, &str)> = BTreeSet::new(); for r in &discovery.shadowed { if r.mode != crate::vex::discover::WiringMode::Hosted || !done.confirmed.iter().any(|(_, uuid)| *uuid == r.uuid) @@ -1947,7 +1947,7 @@ async fn unattributed_pins( }) { copy_shadowed.insert(r.uuid.clone()); if yarn_rewriter_named(&done.rewrite.warnings, &c.file, &c.key) - || !named.insert((c.file.as_path(), c.key.as_str())) + || !named.insert((c.file.as_path(), c.key.as_str(), c.how.as_str())) { continue; } @@ -1955,14 +1955,16 @@ async fn unattributed_pins( REDIRECT_COPY_UNPATCHED, format!( "{}: {} is pinned to patch {}, but lock entry `{}` {}; that copy stays \ - UNPATCHED, so `vex` attests nothing for {}. Depend on the registry \ - release instead of that copy to patch it", + UNPATCHED, so `vex` attests nothing for {}. {}", c.file.display(), r.purl, r.uuid, c.key, c.how, - r.purl + r.purl, + c.remedy.as_deref().unwrap_or( + "Depend on the registry release instead of that copy to patch it" + ), ), )); } diff --git a/crates/socket-patch-core/src/vendor/yarn_classic_bundled.rs b/crates/socket-patch-core/src/vendor/yarn_classic_bundled.rs index c87dcfd98..c527f46d3 100644 --- a/crates/socket-patch-core/src/vendor/yarn_classic_bundled.rs +++ b/crates/socket-patch-core/src/vendor/yarn_classic_bundled.rs @@ -67,6 +67,13 @@ fn bundled_copies_sync(root: &Path) -> BTreeMap { // Directories inside a bundle, recorded with their whole subtree. let mut bundled: HashSet = HashSet::new(); let mut pending = vec!["node_modules".to_string()]; + // A workspace member's own `node_modules` (what the hoist couldn't + // share) is reached through its link in the root `node_modules`. + pending.extend( + linked_members(root) + .into_iter() + .map(|member| format!("{member}/node_modules")), + ); while let Some(dir) = pending.pop() { if !is_real_dir(root, &dir) { continue; @@ -136,6 +143,49 @@ fn bundle_closure(root: &Path, nested: &str, names: Vec) -> BTreeSet Vec { + let Ok(canonical_root) = std::fs::canonicalize(root) else { + return Vec::new(); + }; + let mut links = Vec::new(); + let mut pending = vec![(root.join("node_modules"), true)]; + while let Some((dir, scopes)) = pending.pop() { + let Ok(entries) = std::fs::read_dir(&dir) else { + continue; + }; + for entry in entries.flatten() { + let name = entry.file_name(); + let Ok(file_type) = entry.file_type() else { + continue; + }; + if scopes && file_type.is_dir() && name.to_string_lossy().starts_with('@') { + pending.push((entry.path(), false)); + } else if file_type.is_symlink() { + links.push(entry.path()); + } + } + } + let mut members: Vec = links + .into_iter() + .filter_map(|link| { + let real = std::fs::canonicalize(link).ok()?; + let rel = real.strip_prefix(&canonical_root).ok()?; + let rel: Vec<&str> = rel + .components() + .map(|c| c.as_os_str().to_str()) + .collect::>()?; + (!rel.is_empty() && !rel.contains(&"node_modules") && real.is_dir()) + .then(|| rel.join("/")) + }) + .collect(); + members.sort(); + members.dedup(); + members +} + /// `dir` and every real package directory nested in its `node_modules`. fn subtree(root: &Path, dir: &str, budget: &mut usize) -> Vec { let mut out = vec![dir.to_string()]; @@ -177,9 +227,11 @@ fn bundled_names(manifest: &Value) -> Vec { names } -/// The `dependencies` and `optionalDependencies` keys of a manifest. +/// The `dependencies`, `optionalDependencies` and `peerDependencies` keys +/// of a manifest (npm 7+ packs a bundled package's peers into the bundle +/// too). fn dependency_names(manifest: &Value) -> Vec { - ["dependencies", "optionalDependencies"] + ["dependencies", "optionalDependencies", "peerDependencies"] .iter() .filter_map(|k| manifest.get(*k).and_then(Value::as_object)) .flat_map(|deps| deps.keys().cloned()) @@ -197,3 +249,107 @@ fn read_manifest(root: &Path, dir: &str) -> Option { fn is_real_dir(root: &Path, rel: &str) -> bool { std::fs::symlink_metadata(root.join(rel)).is_ok_and(|m| m.is_dir()) } + +#[cfg(test)] +mod tests { + use super::*; + + fn write(root: &Path, rel: &str, text: &str) { + let path = root.join(rel); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(path, text).unwrap(); + } + + fn package(root: &Path, dir: &str, manifest: &str) { + write(root, &format!("{dir}/package.json"), manifest); + } + + async fn locations(root: &Path) -> Vec<(String, String)> { + bundled_copies(root) + .await + .into_iter() + .map(|(purl, copy)| (purl, copy.location)) + .collect() + } + + const LEFT_PAD: &str = r#"{"name":"left-pad","version":"1.3.0"}"#; + + /// A bundled package's peer is packed beside it (npm 7+), so it is part + /// of the bundle; a sibling the parent merely depends on is not. + #[tokio::test] + async fn a_bundled_packages_peer_is_bundled() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + package( + root, + "node_modules/bund", + r#"{"name":"bund","version":"1.0.0","dependencies":{"plugin":"1.0.0","other":"1.0.0"},"bundleDependencies":["plugin"]}"#, + ); + package( + root, + "node_modules/bund/node_modules/plugin", + r#"{"name":"plugin","version":"1.0.0","peerDependencies":{"left-pad":"^1.3.0"}}"#, + ); + package(root, "node_modules/bund/node_modules/left-pad", LEFT_PAD); + package( + root, + "node_modules/bund/node_modules/other", + r#"{"name":"other","version":"1.0.0"}"#, + ); + assert_eq!( + locations(root).await, + [ + ( + "pkg:npm/left-pad@1.3.0".to_string(), + "node_modules/bund/node_modules/left-pad".to_string() + ), + ( + "pkg:npm/plugin@1.0.0".to_string(), + "node_modules/bund/node_modules/plugin".to_string() + ), + ] + ); + } + + /// A workspace member's own `node_modules` is walked through the + /// member's link in the root `node_modules`; a link leaving the project + /// is not followed. + #[cfg(unix)] + #[tokio::test] + async fn workspace_member_node_modules_are_walked_and_links_stay_inside() { + use std::os::unix::fs::symlink; + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join("project"); + let bund = r#"{"name":"bund","version":"2.0.0","bundleDependencies":["left-pad"]}"#; + package( + &root, + "packages/app", + r#"{"name":"@ws/app","version":"1.0.0"}"#, + ); + package(&root, "packages/app/node_modules/bund", bund); + package( + &root, + "packages/app/node_modules/bund/node_modules/left-pad", + LEFT_PAD, + ); + std::fs::create_dir_all(root.join("node_modules/@ws")).unwrap(); + symlink(root.join("packages/app"), root.join("node_modules/@ws/app")).unwrap(); + // A link outside the project holding a bundle is never read. + let outside = tmp.path().join("outside"); + package(&outside, "ext", r#"{"name":"ext","version":"1.0.0"}"#); + package(&outside, "ext/node_modules/bund", bund); + package( + &outside, + "ext/node_modules/bund/node_modules/left-pad", + LEFT_PAD, + ); + symlink(outside.join("ext"), root.join("node_modules/ext")).unwrap(); + assert_eq!( + locations(&root).await, + [( + "pkg:npm/left-pad@1.3.0".to_string(), + "packages/app/node_modules/bund/node_modules/left-pad".to_string() + )] + ); + } +} 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 d43beb131..bb7442358 100644 --- a/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs +++ b/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs @@ -122,28 +122,42 @@ impl NpmLockBackend for YarnClassicBackend { let blocks = scan_blocks_shared(&text); let other_name = other_name_copy_warnings(project_root, &text, &coords.name, &coords.version).await; - let (candidate_keys, skipped) = - rewritable_candidates(&blocks, &coords.name, &coords.version).map_err(|o| { - only_other_name_copies(o, &blocks, &coords.name, &coords.version, &other_name) - })?; - warnings.extend(skipped); - warnings.extend(other_name.into_iter().map(|(w, _)| w)); // A copy yarn unpacked from a parent's `bundleDependencies` has no // block (#758): it stays unpatched beside whatever this run wires. let bundled = match crate::utils::purl::npm_purl(&coords.name, &coords.version) { Some(purl) => super::yarn_classic_bundled::bundled_copies(project_root) .await - .remove(&purl), + .remove(&purl) + .map(|copy| { + super::yarn_classic_bundled::bundled_copy_detail( + &coords.name, + &coords.version, + ©, + ) + }), None => None, }; - if let Some(copy) = bundled { + let (candidate_keys, skipped) = + rewritable_candidates(&blocks, &coords.name, &coords.version).map_err(|o| { + let o = + only_other_name_copies(o, &blocks, &coords.name, &coords.version, &other_name); + match &bundled { + // The package IS installed, so "run `yarn install`" + // would be wrong: name the real reason. + Some(detail) + if super::npm_common::refusal_code(&o) == "vendor_lock_entry_not_found" => + { + Box::new(refused("vendor_lock_entry_not_rewritable", detail.clone())) + } + _ => o, + } + })?; + warnings.extend(skipped); + warnings.extend(other_name.into_iter().map(|(w, _)| w)); + if let Some(detail) = bundled { warnings.push(VendorWarning::new( "vendor_bundled_instance_skipped", - super::yarn_classic_bundled::bundled_copy_detail( - &coords.name, - &coords.version, - ©, - ), + detail, )); } Ok(YarnClassicPlan { @@ -2033,6 +2047,44 @@ wow@^1.0.0: } } + /// #758: when the package's only installed copy is a bundled one (no + /// yarn.lock block of its own), vendoring refuses as not rewritable and + /// names the copy, rather than suggesting `yarn install`. + #[tokio::test] + async fn issue_758_only_a_bundled_copy_is_refused_as_not_rewritable() { + let lock = "# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.\n\ + # yarn lockfile v1\n\n\nbund@^1.0.0:\n version \"1.0.0\"\n \ + resolved \"https://registry.yarnpkg.com/bund/-/bund-1.0.0.tgz#aaaa\"\n"; + let fx = fixture_with_lock(lock).await; + let parent = fx.root().join("node_modules/bund"); + tokio::fs::create_dir_all(parent.join("node_modules/left-pad")) + .await + .unwrap(); + tokio::fs::write( + parent.join("package.json"), + r#"{"name":"bund","version":"1.0.0","bundleDependencies":["left-pad"]}"#, + ) + .await + .unwrap(); + tokio::fs::write( + parent.join("node_modules/left-pad/package.json"), + r#"{"name":"left-pad","version":"1.3.0"}"#, + ) + .await + .unwrap(); + match fx.vendor(false).await { + VendorOutcome::Refused { code, detail } => { + assert_eq!(code, "vendor_lock_entry_not_rewritable", "{detail}"); + assert!( + detail.contains("node_modules/bund/node_modules/left-pad"), + "{detail}" + ); + } + other => panic!("expected a refusal, got {other:?}"), + } + assert_eq!(fx.lock_text().await, lock, "nothing is written"); + } + /// #1236 follow-up: a `file:` TARBALL declared under another dependency /// name (`"lp2": "file:./forks/left-pad-1.3.0.tgz"`) is read from the /// tarball's own `package.json` and named too; the registry block is diff --git a/crates/socket-patch-core/src/vex/discover/mod.rs b/crates/socket-patch-core/src/vex/discover/mod.rs index dd766d091..fd3755207 100644 --- a/crates/socket-patch-core/src/vex/discover/mod.rs +++ b/crates/socket-patch-core/src/vex/discover/mod.rs @@ -532,6 +532,9 @@ pub struct UnpatchedCopy { pub key: String, /// How that entry installs, completing "lock entry `` …". pub how: String, + /// What makes the build patch that copy, when it is not the usual + /// "depend on the registry release instead" (a bundled copy, #758). + pub remedy: Option, } /// A lock entry that installs SOME package (at `version`, when known) under @@ -1023,9 +1026,22 @@ impl Discovery { file: PathBuf::from(file), key: key.to_string(), how: how.to_string(), + remedy: None, }); } + /// [`Discovery::unpatched_copy`] for a copy lock entry `key` unpacks + /// from its own tarball (`bundleDependencies`, #758): no dependency the + /// user declares installs it, so the remedy is the bundling package. + pub(crate) fn bundled_copy(&mut self, file: &str, purl: Option, key: &str, how: &str) { + self.unpatched_copy(file, purl, key, how); + if let Some(copy) = self.unpatched_copies.last_mut() { + copy.remedy = Some(format!( + "Vendor or update the bundling package (lock entry `{key}`) to cover it" + )); + } + } + /// A lock entry of `file` that installs a copy of some package at /// `version` whose package can't be read (a non-registry url, a missing /// directory, a tarball with no readable `package.json`, a berry @@ -1080,6 +1096,7 @@ impl Discovery { file: u.file.clone(), key: u.key.clone(), how: u.how.clone(), + remedy: None, }); } } diff --git a/crates/socket-patch-core/src/vex/discover/testing/golden.rs b/crates/socket-patch-core/src/vex/discover/testing/golden.rs index c18c68fe7..54509be0e 100644 --- a/crates/socket-patch-core/src/vex/discover/testing/golden.rs +++ b/crates/socket-patch-core/src/vex/discover/testing/golden.rs @@ -254,13 +254,18 @@ fn render(out: &Discovery, root: &Path) -> Value { file, key, how, + remedy, } = c; - json!({ + let mut copy = json!({ "purl": purl, "file": path_str(file), "key": key, "how": normalize(how, &roots), - }) + }); + if let Some(remedy) = remedy { + copy["remedy"] = json!(remedy); + } + copy }) .collect::>() .into(); @@ -295,13 +300,18 @@ fn render(out: &Discovery, root: &Path) -> Value { file, key, how, + remedy, } = c; - json!({ + let mut copy = json!({ "purl": purl, "file": path_str(file), "key": key, "how": normalize(how, &roots), - }) + }); + if let Some(remedy) = remedy { + copy["remedy"] = json!(remedy); + } + copy }) .collect::>() .into(); diff --git a/crates/socket-patch-core/src/vex/discover/yarn.rs b/crates/socket-patch-core/src/vex/discover/yarn.rs index 7ddfb1844..5a545a2e1 100644 --- a/crates/socket-patch-core/src/vex/discover/yarn.rs +++ b/crates/socket-patch-core/src/vex/discover/yarn.rs @@ -191,7 +191,7 @@ async fn record_bundled_copies(ctx: &DiscoverCtx<'_>, entries: &[YarnEntry], out }) .map(|e| e.block.key.clone()) .unwrap_or_else(|| format!("{}@{}", copy.parent_name, copy.parent_version)); - out.unpatched_copy( + out.bundled_copy( YARN_LOCK, Some(purl), &parent_key,