From 25ef8aa4d4278d595d64e391e98f8825de245200 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 00:23:19 +0000 Subject: [PATCH 1/3] Start fix for #1558, #1578 Assisted-by: Claude Code:claude-opus-5-5 From 1bb54b1dff1b1b406205be6af71810cc8556753a Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 00:35:10 +0000 Subject: [PATCH 2/3] Read yarn classic patched manifests as yarn does Hosted and vendored yarn classic compared a patch's package.json with yarn.lock more literally than yarn 1 writes it, so correct pins were refused or dropped from VEX. - A `""` range is now a sub-map entry (`name ""`), so lock-only VEX attests a pin whose patched manifest has one (#1558). - The patched manifest goes through yarn 1's cleanDependencies first: a name kept only in the first of optional/regular/dev dependencies, with the first non-trivial range. The hosted writer emits that shape, and lock-only VEX compares against it (#1558). - A dependency every pinned block's sub-map already names is no longer required to have a lock key of its own. yarn keys those blocks by the registry metadata's ranges, which can differ from the tarball's package.json, so a patch that leaves dependencies alone was refused with a remedy that could not clear it (#1578). Vendored mode gets the same exemption. Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 10 +- .../tests/e2e_vex_lockfile/yarn.rs | 63 +++++ .../src/formats/yarn/classic_deps.rs | 246 +++++++++++++++--- .../src/patch/redirect/mod.rs | 111 +++++++- .../src/vendor/yarn_classic_lock.rs | 47 +++- docs/ecosystems.md | 9 +- 6 files changed, 443 insertions(+), 43 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 70f83ee43..2d7b65e9f 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -826,7 +826,7 @@ to **six flavors**. | eco / flavor | vendored artifact | committed wiring | consumption proof | |---|---|---|---| | npm (package-lock) | deterministic patched tarball `[@scope/]-.tgz`, plus `/.gitignore` (re-includes the tarball against the project's ignores, such as Node.gitignore's `*.tgz`) and `/.gitattributes` (`* -text -filter -ident -working-tree-encoding`, so neither EOL conversion nor a project filter such as a Git LFS `*.tgz` rule changes the committed bytes; a pre-#1541 `* -text` file is rewritten by the next vendored run); every tarball flavor below writes the same pair and refuses `vendor_artifact_gitignored` when git would still drop the tarball | `package-lock.json` only (`npm-shrinkwrap.json` wins when present): every entry matching name+version gets `resolved: "file:…"` + recomputed `integrity`. `package.json` untouched | `npm ci` (integrity-verified). Plain `npm install` preserves the entry; `npm update ` re-resolves and drops it | -| npm / yarn classic | (same tarball) | `yarn.lock` only: matching blocks get `resolved "file:./…#"` + `integrity` (both checksums recomputed; merged-key & `npm:`-alias blocks covered); a patch that rewrites the package's `package.json` gets the blocks' `dependencies:` / `optionalDependencies:` sub-maps recomputed, and is refused `vendor_dep_manifest_unlocked` before any write when a dependency it adds or a range it changes to has no lock block of its own (#591) | `yarn install --frozen-lockfile --offline` (sha1 fragment + sha512 SRI both enforced; byte-stable lock) | +| npm / yarn classic | (same tarball) | `yarn.lock` only: matching blocks get `resolved "file:./…#"` + `integrity` (both checksums recomputed; merged-key & `npm:`-alias blocks covered); a patch that rewrites the package's `package.json` gets the blocks' `dependencies:` / `optionalDependencies:` sub-maps recomputed, and is refused `vendor_dep_manifest_unlocked` before any write when a dependency it adds or a range it changes to has no lock block of its own (#591); a descriptor the wired blocks' sub-maps already name is upstream's and needs no block of its own (#1578) | `yarn install --frozen-lockfile --offline` (sha1 fragment + sha512 SRI both enforced; byte-stable lock) | | npm / yarn berry (node-modules linker) | (same tarball) | root `package.json` `resolutions` + `yarn.lock` entry with `checksum: 10c0/` of the berry cache-zip (reproduced from the tarball offline). **PnP is refused** (`.pnp.*` → different artifact pipeline) | `yarn install --immutable --check-cache`, cold cache. Refused if `__metadata.cacheKey ≠ 10c0` or a non-default `compressionLevel`. Both files keep their own layout — a CRLF lock (yarn's output on Windows) is spliced in CRLF, `package.json` is re-serialized with its BOM, indent, line ending and trailing-newline shape — so vendor + `--revert` round-trip byte-exactly; a lock or `package.json` MIXING CRLF and LF is refused before any write (`vendor_yarn_berry_mixed_line_endings`) | | npm / pnpm (lockfileVersion 9) | (same tarball) | root `package.json` `pnpm.overrides` (versioned selector) **+** `pnpm-lock.yaml` surgery (overrides / importer version / packages `resolution.integrity` / snapshots) **+** the same override in `pnpm-workspace.yaml` (pnpm >= 10.5 reads it there; created with a root-only `packages:` scaffold when absent, and given that scaffold too when the existing file has no keys at all, since pnpm 8.x–10.4 refuse a workspace file holding a key but no `packages` (#1096); revert removes it again). A project with no `pnpm-workspace.yaml` (or a keyless one) pinned to pnpm 9.0–10.4 (every pin, read as for the hosted trust config) gets no file: those read package.json, and a root-only workspace makes `pnpm add` fail there (#734); a later vendor on pnpm >= 10.5 adds it. Residual: an unpinned project with no install record still gets the scaffold, so on pnpm 9.0–10.4 it needs `pnpm add -w ` or a `packageManager` pin | `pnpm install --frozen-lockfile --offline`, cold store (integrity-verified; byte-stable on pnpm 9 & 10). Other lockfileVersions: 5.4/6.0 route to the legacy backend below; anything else refused | | npm / pnpm LEGACY (lockfileVersion 5.4 = pnpm 7, 6.0 = pnpm 8; flavor `pnpm-legacy`) | (same tarball) | root `package.json` `pnpm.overrides` **+** legacy lock surgery (overrides / root dep + specifiers / packages rekey to a bare `file:` key with recomputed integrity / in-package dep refs). **No `pnpm-workspace.yaml` is written** (pnpm ≤ 8 reads overrides only from package.json). The lock's SPECIFIER is machine-ABSOLUTE — pnpm ≤ 8 absolutizes `file:` overrides itself — surfaced as `vendor_pnpm_legacy_absolute_specifier`. Legacy WORKSPACE locks (`importers:`) refused | same-path `pnpm install --frozen-lockfile --offline`, cold store (byte-stable on pnpm 7.33.5 / 8.15.9). A checkout at a DIFFERENT path fails the frozen check (path-bound specifier) and must run `pnpm install --offline --no-frozen-lockfile` once (the flag matters on CI, where pnpm defaults frozen on), which installs the vendored tarball and re-resolves only the specifier line | @@ -1442,7 +1442,7 @@ command's integration tests check its output. | `vendor_ledger_missing` | `failed` (artifact-level: `uuid` + `details.{ecosystem,path}`, no purl) | repair (v5.0) and `vendor --check`: a lockfile references `.socket/vendor///` but the vendor ledger has no entry for it; repair no longer rebuilds ledger entries from lockfiles. Recovery: restore `.socket/vendor/state.json` from version control and re-run `repair`, or `git checkout -- ` and re-vendor. | | `vendor_wiring_unknown_revert_blocked` | `skipped` (beside the `failed`/`revert_failed` event) | vendor --revert: the ledger entry was reconstructed by a pre-v5 `repair` without wiring records and the live lockfile still resolves through the artifact — the revert refuses (fail-closed) instead of deleting a tarball the lock points at. Recovery: `socket-patch repair`, then restore the pre-vendor lock (or re-lock without the override) and re-run the revert. repair: an npm ledger entry whose `flavor` this release does not know (written by a newer socket-patch) is skipped, never health-checked or rebuilt, and the artifact, wiring and ledger stay as found (a lone `skipped` event; the run's exit is unaffected). Recovery: upgrade socket-patch. | | `stale_install` | `skipped` | vex (in-run `scan --mode hosted --vex`): a hosted stale-install probe found positively unpatched installed bytes, so the purl is omitted even under `--vex-no-verify` (see the gem / Python stale-install guards). | -| `dependency_not_installed` | `skipped` | vex: a hosted npm-family copy carries every patched file, but the patch changes its `package.json` and a dependency it declares (other than an optional one) does not resolve from the copy (no `node_modules/` in it or above its real path), for example because the lockfile never locked it (a yarn classic pin written before the served-manifest check, #1462), or the nearest copy that resolves (an installed optional one included) is outside the range the patched `package.json` declares, unless the project forces that dependency with npm `overrides`, `pnpm.overrides` or yarn `resolutions` (#1538). Lock-only (no installed copy): a yarn classic pin whose block's `dependencies:` / `optionalDependencies:` sub-maps are not exactly the ones the patched `package.json` declares, each locked (#1538). Omitted until the dependency is installed or locked. | +| `dependency_not_installed` | `skipped` | vex: a hosted npm-family copy carries every patched file, but the patch changes its `package.json` and a dependency it declares (other than an optional one) does not resolve from the copy (no `node_modules/` in it or above its real path), for example because the lockfile never locked it (a yarn classic pin written before the served-manifest check, #1462), or the nearest copy that resolves (an installed optional one included) is outside the range the patched `package.json` declares, unless the project forces that dependency with npm `overrides`, `pnpm.overrides` or yarn `resolutions` (#1538). Lock-only (no installed copy): a yarn classic pin whose block's `dependencies:` / `optionalDependencies:` sub-maps are not exactly the ones the patched `package.json` declares, each locked (#1538). The manifest is read as yarn 1 locks it: through yarn's `cleanDependencies` normaliser (a name only in the first of `optionalDependencies`, `dependencies`, `devDependencies`, with the first range that isn't `""` or `*`), and a `""` range is an entry like any other (#1558). Omitted until the dependency is installed or locked. | | `dependency_manifest_unverifiable` | `skipped` | vex (lock-only): a yarn classic hosted pin whose patch changes `package.json`, and the patched `package.json` could be read neither from `.socket/blobs` nor from the patch API (`--offline`, a fetch failure), so the dependencies the lock installs with it cannot be checked (#1538). | | `record_unavailable` | `skipped` | vex (manifest-less): a lockfile-wired patch has no local record (manifest, this run's hosted records or a pre-v5 redirect ledger, vendor ledger) and none could be fetched — `--offline`, transport error, 404, or a refused (paid) patch. Omitted, never attested from the `socket-patch.vendor.json` marker. | | `record_mismatch` | `skipped` | vex (manifest-less): the record found for a wired patch names another package or another patch uuid than the wiring. | @@ -1501,9 +1501,9 @@ command's integration tests check its output. | `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 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. The served tarball is read on every run, so a pin an earlier release wrote without this check (#1462) gets the same warning and is never confirmed either; it stays in the lock until the dependency is locked or the pin is rolled back. Same remedy as `vendor_dep_manifest_unlocked`. | -| `redirect_yarn_classic_dep_manifest_rewritten` | `warnings[]` (warning) | scan/get `--mode hosted` (yarn classic): the served tarball's own `package.json` declares other dependencies than the pinned block's sub-maps, every descriptor already locked; the block's `dependencies:` / `optionalDependencies:` sub-maps are rewritten to match (#591). | +| `vendor_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. A descriptor every wired block's sub-map already names is not counted: yarn keys those blocks by the registry packument's ranges, which can differ from the tarball's `package.json` (#1578). 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 and that the sub-maps of the registry blocks being pinned don't already name (yarn keys those by the packument's ranges, #1578). yarn 1 installs only what the lock names, so a pin would install the patched package without that dependency (#591). The dep is not pinned and never confirmed; the lock is left as it was. The served tarball is read on every run, so a pin an earlier release wrote without this check (#1462) gets the same warning and is never confirmed either; it stays in the lock until the dependency is locked or the pin is rolled back. Same remedy as `vendor_dep_manifest_unlocked`. | +| `redirect_yarn_classic_dep_manifest_rewritten` | `warnings[]` (warning) | scan/get `--mode hosted` (yarn classic): the served tarball's own `package.json` declares other dependencies than the pinned block's sub-maps, every descriptor already locked; the block's `dependencies:` / `optionalDependencies:` sub-maps are rewritten to match, in the shape yarn 1 writes for that manifest (#591, #1558). | | `vendor_artifact_gitignored` | `failed` | vendor (vlt, the npm-family tarball flavors — npm, pnpm, bun, yarn classic, yarn berry — NuGet, and the JVM trees of Maven, Gradle, sbt and scala-cli): inside a git work tree, `git check-ignore --no-index` reports the new artifact's uuid directory (JVM: its tree root, such as `.socket/vendor/maven2`) as ignored by a rule its own `.gitignore` cannot override (such as a root `.socket/` or `vendor/` rule; the detail names the rule). Remedy: drop that rule for `.socket/vendor/`. Refused before any write, dry run included. A file rule such as `*.tgz`, `*.nupkg` or `*.jar` is overridden by the `!*` `.gitignore` vendoring writes into the uuid dir or tree root; if a written tarball, directory payload or nupkg still reads as ignored, the run refuses and removes the uuid dir it created. | | `vendor_artifact_gitignore_unchecked` | warning | vendor (vlt, the npm-family tarball flavors and NuGet): git is installed but could not answer the ignore check for the written vendored directory (it failed to start, ran past 30 s, or `rev-parse` / `check-ignore` exited with an error); the package is vendored and the detail names what failed. Remedy: make sure no ignore rule covers `.socket/` before committing. Git absent, or a project outside any work tree, raises nothing. | | `vendor_ledger_entry_missing` | `failed` | vendor (vlt): the only installed copy is vlt's link to a committed vendored directory, but the vendor ledger has no entry for the package; restore `.socket/vendor/state.json` from version control (v5.0: `repair` no longer re-synthesizes it). Replaces the `package_not_installed` skip. | diff --git a/crates/socket-patch-cli/tests/e2e_vex_lockfile/yarn.rs b/crates/socket-patch-cli/tests/e2e_vex_lockfile/yarn.rs index 87bb1d8ef..520900e4f 100644 --- a/crates/socket-patch-cli/tests/e2e_vex_lockfile/yarn.rs +++ b/crates/socket-patch-cli/tests/e2e_vex_lockfile/yarn.rs @@ -813,6 +813,69 @@ fn hosted_classic_lock_only_checks_the_pinned_dependency_maps() { drop(rt_no_blob); } +/// #1558: lock-only VEX reads the patched `package.json` the way yarn 1 +/// locks it. Each lock below is what yarn writes for the patched manifest, +/// and a frozen install of it is patched, so each pin attests. +/// +/// | patched dependencies | pinned block | verdict | +/// |---|---|---| +/// | `is-odd: ""` | `dependencies: is-odd ""`, `is-odd@:` locked | attests | +/// | `is-odd` in both deps and optionalDeps | only `optionalDependencies` | attests | +/// | deps `is-odd: "*"`, devDeps `is-odd: ^3.0.0` | `dependencies: is-odd "^3.0.0"` | attests | +#[test] +fn issue_1558_hosted_classic_lock_only_reads_the_manifest_as_yarn_does() { + const MANIFEST: &str = "package/package.json"; + let pristine = format!(r#"{{"name":"{NAME}","version":"{VERSION}"}}"#); + let base = hosted_lock(Flavor::Classic, "https://patch.socket.dev", UUID); + let is_odd = |key: &str| { + format!( + "\nis-odd@{key}:\n version \"3.0.1\"\n resolved \ + \"https://registry.yarnpkg.com/is-odd/-/is-odd-3.0.1.tgz#\ + 0123456789abcdef0123456789abcdef01234567\"\n integrity \ + sha512-aXNvZGRpc29kZGlzb2RkaXNvZGQ=\n" + ) + }; + let cells = [ + ( + "empty range", + r#""dependencies":{"is-odd":""}"#, + format!("{base} dependencies:\n is-odd \"\"\n") + &is_odd(""), + ), + ( + "optional and regular", + r#""dependencies":{"is-odd":"^3.0.0"},"optionalDependencies":{"is-odd":"^3.0.0"}"#, + format!("{base} optionalDependencies:\n is-odd \"^3.0.0\"\n") + &is_odd("^3.0.0"), + ), + ( + "star beside a dev range", + r#""dependencies":{"is-odd":"*"},"devDependencies":{"is-odd":"^3.0.0"}"#, + format!("{base} dependencies:\n is-odd \"^3.0.0\"\n") + &is_odd("^3.0.0"), + ), + ]; + for (cell, deps, lock) in cells { + let patched = format!(r#"{{"name":"{NAME}","version":"{VERSION}",{deps}}}"#); + let after_hash = compute_git_sha256_from_bytes(patched.as_bytes()); + let mut view = patch_view(UUID, PURL); + view["files"][MANIFEST] = serde_json::json!({ + "beforeHash": compute_git_sha256_from_bytes(pristine.as_bytes()), + "afterHash": after_hash, + }); + let (rt, server) = serve_patch_views(vec![(UUID, view)]); + let tmp = tempfile::tempdir().unwrap(); + let cwd = tmp.path(); + hosted_checkout(cwd, Flavor::Classic); + put(cwd, "yarn.lock", lock.as_bytes()); + put( + cwd, + &format!(".socket/blobs/{after_hash}"), + patched.as_bytes(), + ); + let (code, env) = vex_json(cwd, &["--proxy-url", &server.uri()]); + assert_attested(cwd, code, &env, UUID, "redirected", cell); + drop(rt); + } +} + // ────────────────────────────────────────────────────────────────────── // VENDORED — lockfile only (cells a, b, e) // ────────────────────────────────────────────────────────────────────── 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 149691851..f85b621de 100644 --- a/crates/socket-patch-core/src/formats/yarn/classic_deps.rs +++ b/crates/socket-patch-core/src/formats/yarn/classic_deps.rs @@ -12,6 +12,14 @@ //! Neither writer can resolve a new descriptor itself, so they rewrite //! the sub-maps only when every descriptor is already locked, and refuse //! the patch otherwise. +//! +//! Both sides are read the way yarn 1 writes them: a manifest's maps go +//! through yarn's own normaliser first (`cleanDependencies`), and a `""` +//! range is an entry like any other (#1558). Yarn keys the blocks a +//! package's dependencies resolve through by the registry packument's +//! ranges, which can differ from the tarball's `package.json` that fills +//! the sub-map, so a descriptor the block's sub-map already names is +//! upstream's, not the patch's, and needs no block keyed by it (#1578). use serde_json::Value; @@ -50,10 +58,23 @@ pub(crate) fn with_manifest_dep_maps(lines: &[String], pkg: &Value) -> Vec Vec { - unlocked_among(blocks, manifest_descriptors(pkg)) +/// block of `blocks` is keyed by and that the sub-maps of `carried` (the +/// lines of each block the patch is wired into) do not all name already, +/// sorted and deduplicated: the ones a lock rewritten to the patched +/// manifest would leave unresolved. +pub(crate) fn unlocked_descriptors( + blocks: &[LockBlock], + pkg: &Value, + carried: &[&[String]], +) -> Vec { + let locked: std::collections::HashSet = blocks + .iter() + .flat_map(|b| split_key_patterns(&b.key)) + .collect(); + let carried = carried_descriptors(carried); + sorted_missing(manifest_descriptors(pkg), |d| { + locked.contains(d) || carried.contains(d) + }) } /// [`unlocked_descriptors`] against `locked`, every pattern the lock's @@ -62,8 +83,26 @@ pub(crate) fn unlocked_descriptors(blocks: &[LockBlock], pkg: &Value) -> Vec, pkg: &Value, + carried: &[&[String]], ) -> Vec { - sorted_missing(manifest_descriptors(pkg), |d| locked.contains(d)) + let carried = carried_descriptors(carried); + sorted_missing(manifest_descriptors(pkg), |d| { + locked.contains(d) || carried.contains(d) + }) +} + +/// The descriptors every block of `carried` names in its sub-maps (none +/// when `carried` is empty): yarn already resolves each of them for the +/// package, through whatever block it keyed them by, and a pin that keeps +/// them changes nothing about how (#1578). +fn carried_descriptors(carried: &[&[String]]) -> std::collections::HashSet { + let mut sets = carried + .iter() + .map(|lines| block_dep_descriptors(lines).into_iter().collect()); + let first: std::collections::HashSet = sets.next().unwrap_or_default(); + sets.fold(first, |acc, set: std::collections::HashSet| { + acc.intersection(&set).cloned().collect() + }) } /// Every descriptor of `descriptors` no block of `blocks` is keyed by, @@ -122,15 +161,23 @@ fn block_dep_entries(lines: &[String]) -> Vec<(String, String)> { 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.extend(sub_map_entry(line)); } out } +/// One `name range` line of a block's sub-map. yarn 1 writes an empty +/// range as `name ""` (#1558): that is an entry, only a line with no +/// range token at all is not. +fn sub_map_entry(line: &str) -> Option<(String, String)> { + let (name, rest) = yarn_token(line.trim_start()); + let rest = rest.trim_start(); + if name.is_empty() || rest.is_empty() { + return None; + } + Some((name, yarn_token(rest).0)) +} + /// One classic-grammar token from the front of `s` — a `"`-quoted string /// 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 @@ -184,11 +231,9 @@ fn block_dep_maps(lines: &[String]) -> Vec<(&'static str, Vec<(String, String)>) let Some(field) = field else { continue; }; - let (name, rest) = yarn_token(line.trim_start()); - let (range, _) = yarn_token(rest.trim_start()); - if name.is_empty() || range.is_empty() { + let Some((name, range)) = sub_map_entry(line) else { continue; - } + }; match maps.iter_mut().find(|(f, _)| *f == field) { Some((_, deps)) => deps.push((name, range)), None => maps.push((field, vec![(name, range)])), @@ -205,24 +250,55 @@ fn is_dep_map_header(rest: &str) -> bool { DEP_FIELDS.iter().any(|f| rest.strip_suffix(':') == Some(f)) } -/// `pkg`'s non-empty dependency maps, each sorted by name. A non-string -/// range is skipped, as yarn skips it. +/// The manifest fields yarn 1's normaliser (`cleanDependencies`) reads, +/// in its precedence order. +const NORMALISED_FIELDS: [&str; 3] = ["optionalDependencies", "dependencies", "devDependencies"]; + +/// `pkg`'s non-empty dependency maps as yarn 1 records them in the +/// package's lock block, each sorted by name, in [`DEP_FIELDS`] order. +/// +/// Yarn normalises a manifest before it locks it (`cleanDependencies` in +/// its normalize-manifest `fix`, #1558): across [`NORMALISED_FIELDS`] in +/// that order, a name stays only in the first field that declares it, +/// and its range becomes the first non-trivial one (not `""` or `"*"`) +/// any of those fields gives it. So a name in both `optionalDependencies` +/// and `dependencies` is locked only as optional, and `"*"` beside a +/// `devDependencies` range is locked as that range. A non-string range is +/// skipped, as yarn skips it. fn manifest_dep_maps(pkg: &Value) -> Vec<(&'static str, Vec<(String, String)>)> { - DEP_FIELDS + let maps: Vec<(&'static str, &serde_json::Map)> = NORMALISED_FIELDS .iter() - .filter_map(|&field| { - let map = pkg.get(field).and_then(Value::as_object)?; - let mut deps: Vec<(String, String)> = map - .iter() - .filter_map(|(k, v)| Some((k.clone(), v.as_str()?.to_string()))) - .collect(); - if deps.is_empty() { - return None; + .filter_map(|&field| Some((field, pkg.get(field)?.as_object()?))) + .collect(); + let mut non_trivial: std::collections::HashMap<&str, &str> = std::collections::HashMap::new(); + for (_, map) in &maps { + for (name, range) in map.iter() { + if let Some(range) = range.as_str().filter(|r| !r.is_empty() && *r != "*") { + non_trivial.entry(name.as_str()).or_insert(range); } + } + } + let mut seen: std::collections::HashSet<&str> = std::collections::HashSet::new(); + let mut out: Vec<(&'static str, Vec<(String, String)>)> = Vec::new(); + for (field, map) in &maps { + let mut deps: Vec<(String, String)> = Vec::new(); + for (name, range) in map.iter() { + if !seen.insert(name.as_str()) { + continue; + } + let Some(range) = range.as_str() else { + continue; + }; + let range = non_trivial.get(name.as_str()).copied().unwrap_or(range); + deps.push((name.clone(), range.to_string())); + } + if DEP_FIELDS.contains(field) && !deps.is_empty() { deps.sort_unstable(); - Some((field, deps)) - }) - .collect() + out.push((field, deps)); + } + } + out.sort_by_key(|(f, _)| DEP_FIELDS.iter().position(|d| d == f)); + out } /// A sub-map key the way yarn 1's serializer writes it: quoted when it @@ -291,12 +367,12 @@ is-odd@3.0.1:\n version \"3.0.1\"\n dependencies:\n is-number \"^6.0.0\"\n" "optionalDependencies": {"@scope/opt": "^2.0.0"}, }); assert_eq!( - unlocked_descriptors(&blocks, &pkg), + unlocked_descriptors(&blocks, &pkg, &[]), vec!["is-number@^7.0.0", "wow@^1.0.0"] ); let unchanged = serde_json::json!({"dependencies": {"is-number": "^6.0.0"}}); - assert!(unlocked_descriptors(&blocks, &unchanged).is_empty()); - assert!(unlocked_descriptors(&blocks, &serde_json::json!({})).is_empty()); + assert!(unlocked_descriptors(&blocks, &unchanged, &[]).is_empty()); + assert!(unlocked_descriptors(&blocks, &serde_json::json!({}), &[]).is_empty()); } /// #1423: the pre-split form agrees with [`unlocked_descriptors`]. @@ -317,8 +393,8 @@ is-odd@3.0.1:\n version \"3.0.1\"\n dependencies:\n is-number \"^6.0.0\"\n" serde_json::json!({}), ] { assert_eq!( - unlocked_descriptors_in(&locked, &pkg), - unlocked_descriptors(&blocks, &pkg), + unlocked_descriptors_in(&locked, &pkg, &[]), + unlocked_descriptors(&blocks, &pkg, &[]), "{pkg}" ); } @@ -347,6 +423,110 @@ is-odd@3.0.1:\n version \"3.0.1\"\n dependencies:\n is-number \"^6.0.0\"\n" assert!(unlocked_among(&blocks, block_dep_descriptors(&odd.lines).into_iter()).is_empty()); } + /// #1558 shape 1: yarn writes an empty range as `name ""`. The block + /// declares exactly the manifest's `""` dependency, and its descriptor + /// is the `name@` key yarn locks it under. + #[test] + fn issue_1558_an_empty_range_is_a_sub_map_entry() { + let block = + lines("is-odd@3.0.1:\n version \"3.0.1\"\n dependencies:\n is-number \"\"\n"); + let pkg = serde_json::json!({"dependencies": {"is-number": ""}}); + assert!(dep_maps_declare(&block, &pkg)); + assert!(dep_maps_match(&block, &pkg)); + assert_eq!(block_dep_descriptors(&block), vec!["is-number@"]); + let blocks = scan_blocks("is-number@:\n version \"7.0.0\"\n"); + assert!(unlocked_descriptors(&blocks, &pkg, &[]).is_empty()); + // A line with no range token at all is still not an entry. + assert!(block_dep_descriptors(&lines("a@1:\n dependencies:\n b\n")).is_empty()); + } + + /// #1558 shapes 2 and 3: the manifest is read through yarn 1's + /// `cleanDependencies` before it is compared or written. A name in + /// both `optionalDependencies` and `dependencies` is locked only as + /// optional; a `"*"` range takes a `devDependencies` range. + #[test] + fn issue_1558_manifest_maps_follow_yarns_normaliser() { + let both = serde_json::json!({ + "dependencies": {"is-number": "^6.0.0", "ms": "2.1.3"}, + "optionalDependencies": {"is-number": "^6.0.0"}, + }); + let yarn_shape = lines( + "is-odd@3.0.1:\n version \"3.0.1\"\n dependencies:\n ms \"2.1.3\"\n \ + optionalDependencies:\n is-number \"^6.0.0\"", + ); + assert!(dep_maps_declare(&yarn_shape, &both)); + assert!(dep_maps_match(&yarn_shape, &both)); + let stale = lines("is-odd@3.0.1:\n version \"3.0.1\""); + assert_eq!(with_manifest_dep_maps(&stale, &both), yarn_shape); + + let star = serde_json::json!({ + "dependencies": {"is-number": "*"}, + "devDependencies": {"is-number": "^6.0.0", "dev-only": "1"}, + }); + let block = + lines("is-odd@3.0.1:\n version \"3.0.1\"\n dependencies:\n is-number \"^6.0.0\""); + assert!(dep_maps_declare(&block, &star)); + assert!(!dep_maps_declare( + &lines("is-odd@3.0.1:\n dependencies:\n is-number \"*\""), + &star + )); + // The first non-trivial range wins, in yarn's field order. + let opt_first = serde_json::json!({ + "dependencies": {"a": "^2.0.0"}, + "optionalDependencies": {"a": "*"}, + }); + assert_eq!( + manifest_dep_maps(&opt_first), + vec![( + "optionalDependencies", + vec![("a".to_string(), "^2.0.0".to_string())] + )] + ); + } + + /// #1578: yarn keys a package's dependency blocks by the registry + /// packument's ranges (`url@*`), while the sub-map holds the tarball's + /// (`url "^0.11.0"`). A descriptor every pinned block already carries + /// is not the patch's to lock; one the patch adds still is. + #[test] + fn issue_1578_a_carried_descriptor_needs_no_key_of_its_own() { + let lock = "url@*:\n version \"0.11.0\"\n\n\ + stubs@1.2.3:\n version \"1.2.3\"\n dependencies:\n url \"^0.11.0\"\n"; + let blocks = scan_blocks(lock); + let stubs = blocks.iter().find(|b| b.key == "stubs@1.2.3").unwrap(); + let carried = [stubs.lines.as_slice()]; + let pkg = serde_json::json!({"dependencies": {"url": "^0.11.0"}}); + assert_eq!( + unlocked_descriptors(&blocks, &pkg, &[]), + vec!["url@^0.11.0"], + "without the carried block the key check alone refuses" + ); + assert!(unlocked_descriptors(&blocks, &pkg, &carried).is_empty()); + let patterns: Vec = blocks + .iter() + .flat_map(|b| split_key_patterns(&b.key)) + .collect(); + let locked: std::collections::HashSet<&str> = patterns.iter().map(String::as_str).collect(); + assert!(unlocked_descriptors_in(&locked, &pkg, &carried).is_empty()); + + let added = serde_json::json!({"dependencies": {"url": "^0.11.0", "wow": "^1.0.0"}}); + assert_eq!( + unlocked_descriptors_in(&locked, &added, &carried), + vec!["wow@^1.0.0"] + ); + let bumped = serde_json::json!({"dependencies": {"url": "^0.12.0"}}); + assert_eq!( + unlocked_descriptors(&blocks, &bumped, &carried), + vec!["url@^0.12.0"] + ); + // Every pinned block must carry it: one that doesn't keeps it unlocked. + let bare = lines("stubs@1.2.3:\n version \"1.2.3\""); + assert_eq!( + unlocked_descriptors(&blocks, &pkg, &[stubs.lines.as_slice(), bare.as_slice()]), + vec!["url@^0.11.0"] + ); + } + #[test] fn yarn_token_reads_quoted_and_bare_tokens() { assert_eq!( diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index 8e060bd9c..60d61df7f 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -4141,7 +4141,22 @@ fn rewrite_yarn_classic_with( .flat_map(|(patterns, _)| patterns.iter().map(String::as_str)) .collect() }); - let missing = crate::formats::yarn::classic_deps::unlocked_descriptors_in(locked, pkg); + // The registry copies of this version the pin rewrites: a + // descriptor their sub-maps already carry is upstream's, keyed + // by the packument's range rather than the tarball's (#1578). + let carried: Vec<&[String]> = blocks + .iter() + .zip(&heads) + .filter(|(b, (patterns, real_name))| { + real_name.as_deref() == Some(fname.as_str()) + && classic_field(&b.lines, "version") == Some(dep.version.as_str()) + && classic_copy_source(patterns, classic_field(&b.lines, "resolved")) + == CopySource::Registry + }) + .map(|(b, _)| b.lines.as_slice()) + .collect(); + let missing = + crate::formats::yarn::classic_deps::unlocked_descriptors_in(locked, pkg, &carried); if !missing.is_empty() { result .refused_yarn_classic_uuids @@ -22527,6 +22542,100 @@ packages: assert!(r.refused_yarn_classic_uuids.is_empty()); } + /// Run the hosted classic rewriter over `lock` for `ovr`, serving + /// `manifest` as its tarball's package.json. + fn classic_with_manifest(lock: &str, ovr: &DepOverride, manifest: &str) -> RewriteResult { + let files = BTreeMap::from([("yarn.lock".to_string(), lock.to_string())]); + let manifests = BTreeMap::from([(ovr.artifact_url.clone(), manifest.to_string())]); + let mut r = RewriteResult::default(); + rewrite_yarn_classic_with( + &files, + std::slice::from_ref(ovr), + &manifests, + &yarnrc::OuterYarnMirror::default(), + &mut r, + ); + r + } + + /// #1578: yarn 1 keys a package's dependency blocks by the registry + /// packument's ranges (`url@*`) and fills the block's sub-map from the + /// tarball (`url "^0.11.0"`). A patch that leaves package.json alone + /// is pinned, sub-map untouched; one that adds an unlocked dependency + /// is still refused, naming only that one. + #[test] + fn issue_1578_hosted_classic_pins_over_packument_keyed_dependencies() { + let url = "http://p.test/meteor-node-stubs-1.2.3.tgz"; + let ovr = npm_override("meteor-node-stubs", "1.2.3", url, "sha512-P=="); + let lock = "# yarn lockfile v1\n\n\n\ + meteor-node-stubs@1.2.3:\n version \"1.2.3\"\n \ + resolved \"https://registry.yarnpkg.com/meteor-node-stubs/-/meteor-node-stubs-1.2.3.tgz#bbbb\"\n \ + integrity sha512-UP==\n dependencies:\n assert \"^2.0.0\"\n url \"^0.11.0\"\n\n\ + assert@*:\n version \"2.1.0\"\n \ + resolved \"https://registry.yarnpkg.com/assert/-/assert-2.1.0.tgz#cccc\"\n\n\ + url@*:\n version \"0.11.4\"\n \ + resolved \"https://registry.yarnpkg.com/url/-/url-0.11.4.tgz#dddd\"\n"; + let served = + r#"{"name":"meteor-node-stubs","dependencies":{"url":"^0.11.0","assert":"^2.0.0"}}"#; + let r = classic_with_manifest(lock, &ovr, served); + assert!(r.warnings.is_empty(), "{:?}", r.warnings); + assert!(r.refused_yarn_classic_uuids.is_empty()); + assert!( + r.files["yarn.lock"].contains(&format!( + " resolved \"{url}#{NPM_SHA1}\"\n integrity sha512-P==\n \ + dependencies:\n assert \"^2.0.0\"\n url \"^0.11.0\"\n" + )), + "{:?}", + r.files + ); + + let added = r#"{"name":"meteor-node-stubs","dependencies":{"url":"^0.11.0","assert":"^2.0.0","wow":"^1.0.0"}}"#; + let r = classic_with_manifest(lock, &ovr, added); + assert!(r.files.is_empty() && r.edits.is_empty(), "{:?}", r.files); + assert_eq!( + warning_codes(&r), + vec!["redirect_yarn_classic_dep_manifest_unlocked"] + ); + assert!( + r.warnings[0] + .detail + .contains("depends on wow@^1.0.0, which"), + "{:?}", + r.warnings + ); + } + + /// #1558 shape 2: a served manifest that declares a name in both + /// `optionalDependencies` and `dependencies` is written the way yarn 1 + /// writes it, only under `optionalDependencies`, so yarn's next save + /// leaves the block as it is. + #[test] + fn issue_1558_hosted_classic_writes_yarns_normalised_sub_maps() { + let url = "http://p.test/is-odd-3.0.1.tgz"; + let ovr = npm_override("is-odd", "3.0.1", url, "sha512-P=="); + let lock = "# yarn lockfile v1\n\n\n\ + is-number@^6.0.0:\n version \"6.0.0\"\n \ + resolved \"https://registry.yarnpkg.com/is-number/-/is-number-6.0.0.tgz#aaaa\"\n\n\ + is-odd@3.0.1:\n version \"3.0.1\"\n \ + resolved \"https://registry.yarnpkg.com/is-odd/-/is-odd-3.0.1.tgz#bbbb\"\n \ + integrity sha512-UP==\n"; + let served = r#"{"name":"is-odd","dependencies":{"is-number":"^6.0.0"},"optionalDependencies":{"is-number":"^6.0.0"}}"#; + let r = classic_with_manifest(lock, &ovr, served); + assert_eq!( + warning_codes(&r), + vec!["redirect_yarn_classic_dep_manifest_rewritten"] + ); + let text = &r.files["yarn.lock"]; + assert!( + text.contains(&format!( + " resolved \"{url}#{NPM_SHA1}\"\n integrity sha512-P==\n \ + optionalDependencies:\n is-number \"^6.0.0\"\n" + )), + "{text}" + ); + assert!(!text.contains(" dependencies:"), "{text}"); + } + /// #558: a yarn classic pin without a sha1 would have no `#` /// fragment, so yarn 1 would file the hosted tarball in the cache slot /// of a fragmentless upstream copy and install its bytes. The rewriter 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 276bf4a39..5190f9b7c 100644 --- a/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs +++ b/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs @@ -234,7 +234,14 @@ impl NpmLockBackend for YarnClassicBackend { coords: &NpmCoords, ) -> Option { let blocks = scan_blocks_shared(&plan.text); - let missing = unlocked_descriptors(&blocks, staged_pkg); + // The blocks this run wires: a descriptor their sub-maps already + // carry is upstream's, keyed by the packument's range (#1578). + let carried: Vec<&[String]> = blocks + .iter() + .filter(|b| plan.candidate_keys.contains(&b.key)) + .map(|b| b.lines.as_slice()) + .collect(); + let missing = unlocked_descriptors(&blocks, staged_pkg, &carried); if missing.is_empty() { return None; } @@ -1582,6 +1589,44 @@ wow@^1.0.0: ); } + /// #1578: yarn 1 keys left-pad's dependency block by the packument's + /// range (`url@*`) while its sub-map holds the tarball's + /// (`url "^0.11.0"`). A patch whose package.json keeps that dependency + /// as it was vendors; one that adds an unlocked dependency is refused, + /// naming only the new one. + #[tokio::test] + async fn issue_1578_a_dependency_the_block_already_carries_is_not_refused() { + let lock = format!( + "{Y2_BEFORE} dependencies:\n url \"^0.11.0\"\n\n\ + url@*:\n version \"0.11.4\"\n" + ); + let mut fx = fixture_with_lock(&lock).await; + patch_manifest( + &mut fx, + br#"{"name":"left-pad","version":"1.3.0","dependencies":{"url":"^0.11.0","wow":"^1.0.0"}}"#, + ) + .await; + let detail = expect_refused(fx.vendor(false).await, "vendor_dep_manifest_unlocked"); + assert!( + detail.contains("wow@^1.0.0") && !detail.contains("url@"), + "{detail}" + ); + assert_eq!(fx.lock_text().await, lock, "lock untouched"); + + patch_manifest( + &mut fx, + br#"{"name":"left-pad","version":"1.3.0","description":"patched","dependencies":{"url":"^0.11.0"}}"#, + ) + .await; + let (result, entry, _) = expect_done(fx.vendor(false).await); + assert!(result.success && entry.is_some(), "{:?}", result.error); + let text = fx.lock_text().await; + assert!( + text.contains(" dependencies:\n url \"^0.11.0\"\n"), + "{text}" + ); + } + /// #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. diff --git a/docs/ecosystems.md b/docs/ecosystems.md index c35f6d056..494d2db50 100644 --- a/docs/ecosystems.md +++ b/docs/ecosystems.md @@ -225,9 +225,12 @@ Each Legacy format has an upgrade path and an undo path. Both work in v5: names, so when the patch adds a dependency (or changes a range) that no `yarn.lock` block locks, the pin is refused with `redirect_yarn_classic_dep_manifest_unlocked` (lock the new descriptor first, - for example with `yarn add`, then re-run). When every descriptor is already - locked, the entry's dependency sub-maps are rewritten to match - (`redirect_yarn_classic_dep_manifest_rewritten`). Vendored mode refuses the + for example with `yarn add`, then re-run). A dependency the entry's sub-map + already names is left alone even when no block is keyed by it: yarn keys + those blocks by the registry metadata's ranges, which can differ from the + tarball's `package.json`. When every descriptor is already locked, the + entry's dependency sub-maps are rewritten to match, normalised the way yarn + 1 writes them (`redirect_yarn_classic_dep_manifest_rewritten`). Vendored mode refuses the same case with `vendor_dep_manifest_unlocked`. If the download, the check or the `package.json` read fails, the patch is skipped as `npm_tarball_unavailable`. A project that sets `yarn-offline-mirror` From 4d3698c13bb19fe9ebd9a3bfb28b95bd42a3b7b7 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 00:54:27 +0000 Subject: [PATCH 3/3] Tighten the carried-dependency exemption Follow-up from review of the yarn classic manifest fix. - A dependency the pinned block already names is exempt from the lock-key check only while some block locks a package of that name. A sub-map entry an older release wrote and never locked (#1379), or whose block was since dropped, is still refused. - The hosted exemption ignores blocks reached only through an npm: alias, which the rewriter never pins. - Lock-only VEX accepts either the yarn-normalised sub-maps or the manifest's literal maps, so pins an earlier release wrote in the literal shape keep attesting. - unlocked_descriptors delegates to unlocked_descriptors_in. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/formats/yarn/classic_deps.rs | 73 ++++++++++++++++--- .../src/patch/redirect/mod.rs | 3 + 2 files changed, 64 insertions(+), 12 deletions(-) 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 f85b621de..d4ba9b619 100644 --- a/crates/socket-patch-core/src/formats/yarn/classic_deps.rs +++ b/crates/socket-patch-core/src/formats/yarn/classic_deps.rs @@ -24,7 +24,7 @@ use serde_json::Value; use super::blocks::{body_field_line, LockBlock}; -use super::patterns::split_key_patterns; +use super::patterns::{split_classic_pattern, split_key_patterns}; /// The block fields yarn 1 mirrors from a package's manifest, in the /// order it writes them. @@ -67,14 +67,12 @@ pub(crate) fn unlocked_descriptors( pkg: &Value, carried: &[&[String]], ) -> Vec { - let locked: std::collections::HashSet = blocks + let patterns: Vec = blocks .iter() .flat_map(|b| split_key_patterns(&b.key)) .collect(); - let carried = carried_descriptors(carried); - sorted_missing(manifest_descriptors(pkg), |d| { - locked.contains(d) || carried.contains(d) - }) + let locked: std::collections::HashSet<&str> = patterns.iter().map(String::as_str).collect(); + unlocked_descriptors_in(&locked, pkg, carried) } /// [`unlocked_descriptors`] against `locked`, every pattern the lock's @@ -87,14 +85,14 @@ pub(crate) fn unlocked_descriptors_in( ) -> Vec { let carried = carried_descriptors(carried); sorted_missing(manifest_descriptors(pkg), |d| { - locked.contains(d) || carried.contains(d) + locked.contains(d) || (carried.contains(d) && name_is_locked(locked, d)) }) } /// The descriptors every block of `carried` names in its sub-maps (none /// when `carried` is empty): yarn already resolves each of them for the -/// package, through whatever block it keyed them by, and a pin that keeps -/// them changes nothing about how (#1578). +/// package, through the block it keyed by the packument's range, and a pin +/// that keeps them changes nothing about how (#1578). fn carried_descriptors(carried: &[&[String]]) -> std::collections::HashSet { let mut sets = carried .iter() @@ -105,6 +103,20 @@ fn carried_descriptors(carried: &[&[String]]) -> std::collections::HashSet, descriptor: &str) -> bool { + let Some((name, _)) = split_classic_pattern(descriptor) else { + return false; + }; + locked + .iter() + .any(|p| split_classic_pattern(p).is_some_and(|(n, _)| n == name)) +} + /// Every descriptor of `descriptors` no block of `blocks` is keyed by, /// sorted and deduplicated. pub(crate) fn unlocked_among( @@ -210,9 +222,12 @@ pub(crate) fn dep_maps_match(lines: &[String], pkg: &Value) -> bool { /// Whether the sub-maps `lines` carries declare exactly the dependencies /// `pkg` declares, read back however yarn 1 quoted them (`foo latest` /// and `foo "latest"` alike): what yarn installs from the block is what -/// the manifest asks for. +/// the manifest asks for. Either shape counts: the one yarn 1 writes +/// after normalising the manifest (#1558), and the manifest's literal +/// maps, which an earlier release wrote and yarn installs as well. pub(crate) fn dep_maps_declare(lines: &[String], pkg: &Value) -> bool { - block_dep_maps(lines) == manifest_dep_maps(pkg) + let block = block_dep_maps(lines); + block == manifest_dep_maps(pkg) || block == literal_dep_maps(pkg) } /// A block's non-empty dependency sub-maps, each sorted by name, in @@ -250,6 +265,27 @@ fn is_dep_map_header(rest: &str) -> bool { DEP_FIELDS.iter().any(|f| rest.strip_suffix(':') == Some(f)) } +/// `pkg`'s non-empty dependency maps exactly as the manifest spells them, +/// each sorted by name, skipping non-string ranges: the shape the writer +/// used before it followed yarn's normaliser. +fn literal_dep_maps(pkg: &Value) -> Vec<(&'static str, Vec<(String, String)>)> { + DEP_FIELDS + .iter() + .filter_map(|&field| { + let map = pkg.get(field).and_then(Value::as_object)?; + let mut deps: Vec<(String, String)> = map + .iter() + .filter_map(|(k, v)| Some((k.clone(), v.as_str()?.to_string()))) + .collect(); + if deps.is_empty() { + return None; + } + deps.sort_unstable(); + Some((field, deps)) + }) + .collect() +} + /// The manifest fields yarn 1's normaliser (`cleanDependencies`) reads, /// in its precedence order. const NORMALISED_FIELDS: [&str; 3] = ["optionalDependencies", "dependencies", "devDependencies"]; @@ -466,10 +502,15 @@ is-odd@3.0.1:\n version \"3.0.1\"\n dependencies:\n is-number \"^6.0.0\"\n" let block = lines("is-odd@3.0.1:\n version \"3.0.1\"\n dependencies:\n is-number \"^6.0.0\""); assert!(dep_maps_declare(&block, &star)); - assert!(!dep_maps_declare( + // The literal shape an earlier release wrote still declares it. + assert!(dep_maps_declare( &lines("is-odd@3.0.1:\n dependencies:\n is-number \"*\""), &star )); + assert!(!dep_maps_declare( + &lines("is-odd@3.0.1:\n dependencies:\n is-number \"^5.0.0\""), + &star + )); // The first non-trivial range wins, in yarn's field order. let opt_first = serde_json::json!({ "dependencies": {"a": "^2.0.0"}, @@ -519,6 +560,14 @@ is-odd@3.0.1:\n version \"3.0.1\"\n dependencies:\n is-number \"^6.0.0\"\n" unlocked_descriptors(&blocks, &bumped, &carried), vec!["url@^0.12.0"] ); + // A carried descriptor with no block of its name at all (an older + // release's unlocked sub-map entry, #1379) still needs locking. + let dangling = lines("stubs@1.2.3:\n dependencies:\n wow \"^1.0.0\""); + let wow = serde_json::json!({"dependencies": {"wow": "^1.0.0"}}); + assert_eq!( + unlocked_descriptors(&blocks, &wow, &[dangling.as_slice()]), + vec!["wow@^1.0.0"] + ); // Every pinned block must carry it: one that doesn't keeps it unlocked. let bare = lines("stubs@1.2.3:\n version \"1.2.3\""); assert_eq!( diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index 60d61df7f..2cf8b79da 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -4152,6 +4152,9 @@ fn rewrite_yarn_classic_with( && classic_field(&b.lines, "version") == Some(dep.version.as_str()) && classic_copy_source(patterns, classic_field(&b.lines, "resolved")) == CopySource::Registry + && patterns + .iter() + .any(|p| split_classic_pattern(p).is_some_and(|(n, _)| n == fname)) }) .map(|(b, _)| b.lines.as_slice()) .collect();