WIP: hosted yarn classic served-manifest dep check

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Author: mikolalysenko

crates/socket-patch-cli/CLI_CONTRACT.md

@@ -738,7 +738,7 @@ to **six flavors**.
 | eco / flavor | vendored artifact | committed wiring | consumption proof |
 |---|---|---|---|
 | npm (package-lock) | deterministic patched tarball `[@scope/]<name>-<version>.tgz`, plus `<uuid>/.gitignore` (re-includes the tarball against the project's ignores, such as Node.gitignore's `*.tgz`) and `<uuid>/.gitattributes` (`-text`); 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 <pkg>` re-resolves and drops it |
-| npm / yarn classic | (same tarball) | `yarn.lock` only: matching blocks get `resolved "file:./…#<sha1>"` + `integrity` (both checksums recomputed; merged-key & `npm:`-alias blocks covered) | `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:./…#<sha1>"` + `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 berry (node-modules linker) | (same tarball) | root `package.json` `resolutions` + `yarn.lock` entry with `checksum: 10c0/<sha512>` 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). A project with no `pnpm-workspace.yaml` 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 <pkg>` 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 |
@@ -1350,6 +1350,9 @@ Every `--json` invocation emits a single JSON object that follows the **unified
 | `vendor_vlt_reinstall_required` | `skipped` (advisory; human: `Warning: …`) | vendor / scan / get `--mode vendored` (vlt), wet and dry runs, and in-sync reruns: (a) the run rewires an optional dependency, or an importer's `node_modules/<name>` 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/<name>`, `<member dir>/node_modules/<name>`) 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 `redirect.warnings[]`): (a) the revert moves an `optionalDependencies` spec back from the `file:` dir, or an optional importer's `node_modules/<name>` 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` `gc.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/<name>` 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 <descriptor>`), then re-run. A dry run, which stages nothing, does not foresee it. |
+| `redirect_yarn_classic_dep_manifest_unlocked` | `redirect.warnings[]` (warning) | scan/get `--mode hosted` (yarn classic): the served tarball's own `package.json` depends on a descriptor no `yarn.lock` block is keyed by. yarn 1 installs only what the lock names, so a pin would install the patched package without that dependency (#591). The dep is not pinned and never confirmed; the lock is left as it was. Same remedy as `vendor_dep_manifest_unlocked`. |
+| `redirect_yarn_classic_dep_manifest_rewritten` | `redirect.warnings[]` (warning) | scan/get `--mode hosted` (yarn classic): the served tarball's own `package.json` declares other dependencies than the pinned block's sub-maps, every descriptor already locked; the block's `dependencies:` / `optionalDependencies:` sub-maps are rewritten to match (#591). |
 | `vendor_artifact_gitignored` | `failed` | vendor (vlt and the npm-family tarball flavors: npm, pnpm, bun, yarn classic, yarn berry): inside a git work tree, `git check-ignore --no-index` reports the new artifact's uuid directory 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. A file rule such as `*.tgz` is overridden by the `<uuid>/.gitignore` vendoring writes; if the written artifact still reads as ignored, the run refuses and removes the uuid dir it created. |
 | `vendor_artifact_gitignore_unchecked` | warning | vendor (vlt and the npm-family tarball flavors): 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. |

crates/socket-patch-cli/src/commands/scan/hosted.rs

@@ -1042,28 +1042,37 @@ pub(crate) async fn run_redirect_selected(
             }
         }
     }
-    // A yarn classic pin needs the served tarball's sha1 as its `resolved`
-    // fragment: yarn 1 keys its cache slot by it (#558). When the grant
-    // carries only a sha512, the scan downloads the tarball, checks it
-    // against that sha512 and pins the sha1 of those bytes. A tarball that
-    // cannot be fetched or verified drops its patch rather than pin a
-    // fragmentless URL a warm cache serves stale bytes for.
-    let sha1_targets: Vec<(String, String, DepOverride)> =
-        engine::yarn_classic_sha1_targets(&candidates, &read.files)
+    // A yarn classic pin reads the served tarball: its sha1 is the
+    // `resolved` fragment yarn 1 keys its cache slot on when the grant
+    // carries none (#558), and its package.json's dependencies must match
+    // the lock block's sub-maps, every new descriptor locked (#591). The
+    // tarball is checked against the grant's sha512; one that cannot be
+    // fetched, verified or read drops its patch.
+    let classic_targets: Vec<(String, String, DepOverride)> =
+        engine::yarn_classic_artifact_targets(
+            &candidates,
+            &read.files,
+            &resolve_outer_yarn_mirror_for_process(&common.cwd),
+        )
             .into_iter()
             .filter_map(|dep| {
                 let sha512 = dep.integrity.sha512.clone()?;
                 Some((dep.artifact_url.clone(), sha512, dep.clone()))
             })
             .collect();
-    for (url, sha512, dep) in sha1_targets {
+    for (url, sha512, dep) in classic_targets {
         status.set(format!("Fetching hosted tarball for {}...", dep.name));
-        match socket_patch_core::hosted::npm_manifest::fetch_hosted_npm_sha1(
+        match socket_patch_core::hosted::npm_manifest::fetch_hosted_classic_artifact(
             api_client, &url, &sha512,
         )
         .await
         {
-            Ok(sha1) => engine::set_derived_sha1(&mut candidates, &url, &sha1),
+            Ok(artifact) => engine::record_classic_artifact(
+                &mut candidates,
+                &mut python_metadata,
+                &url,
+                &artifact,
+            ),
             Err(detail) => {
                 unavailable_python_artifacts.insert(url.clone());
                 skipped.push(engine::npm_tarball_unavailable(&dep, &detail));
@@ -2063,7 +2072,7 @@ fn describe_skip_reason(reason: &str) -> String {
             "the hosted tarball's package.json could not be fetched".into()
         }
         "npm_tarball_unavailable" => {
-            "the hosted tarball could not be fetched or did not match its sha512".into()
+            "the hosted tarball could not be fetched, verified or read".into()
         }
         "redirect_bun_lock_unsupported" | "redirect_bun_lockb_invalid" => {
             "the Bun lockfile blocks the vendored-to-hosted migration (see the warning)".into()

crates/socket-patch-cli/tests/scan/hosted_yarn_classic_sha1.rs

@@ -262,3 +262,70 @@ fn assert_skipped_untouched(root: &Path, server: &MockServer, doc: &Value, stder
         "no fragmentless hosted pin is written"
     );
 }
+
+/// #591: the served tarball's package.json adds a dependency yarn.lock
+/// doesn't lock. Yarn 1 installs only what the lock names, so a pin would
+/// install the patched package without it (and `vex` would attest it):
+/// the scan refuses the pin with an actionable warning and leaves the
+/// lock alone. The grant carries a sha1 here, so only the dependency
+/// check needs the tarball.
+#[tokio::test]
+async fn issue_591_served_dependency_the_lock_does_not_lock_is_refused() {
+    let server = MockServer::start().await;
+    let tarball = tgz(&[
+        (
+            "package/package.json",
+            br#"{"name":"left-pad","version":"1.3.0","dependencies":{"is-odd":"^3.0.0"}}"#,
+        ),
+        ("package/index.js", b"module.exports = require('is-odd');\n"),
+    ]);
+    mock_api_with_sha1(&server, &tarball).await;
+    let tmp = tempfile::tempdir().unwrap();
+    let root = tmp.path().join("proj");
+    write_classic_project(&root);
+
+    let (code, doc, stderr) = scan(&root, &server.uri());
+    assert_eq!(code, 0, "{doc:#}\n{stderr}");
+    assert_eq!(doc["redirect"]["redirected"], 0, "{doc:#}");
+    let warning = doc["redirect"]["warnings"]
+        .as_array()
+        .unwrap()
+        .iter()
+        .find(|w| w["code"] == "redirect_yarn_classic_dep_manifest_unlocked")
+        .unwrap_or_else(|| panic!("refusal warning: {doc:#}"));
+    let detail = warning["detail"].as_str().unwrap();
+    assert!(
+        detail.contains("is-odd@^3.0.0") && detail.contains("yarn add is-odd@^3.0.0"),
+        "{detail}"
+    );
+    assert_eq!(
+        std::fs::read_to_string(root.join("yarn.lock")).unwrap(),
+        LOCK,
+        "nothing is pinned with an incomplete dependency graph"
+    );
+}
+
+/// [`mock_api`] with a grant that carries the tarball's sha1 too.
+async fn mock_api_with_sha1(server: &MockServer, tarball: &[u8]) {
+    let artifact = format!("/patch/npm/left-pad/1.3.0/{TOKEN}/{UUID}/left-pad-1.3.0.tgz");
+    let url = format!("{}{artifact}", server.uri());
+    Mock::given(method("POST"))
+        .and(path(format!("/v0/orgs/{ORG}/patches/package")))
+        .respond_with(ResponseTemplate::new(200).set_body_json(json!({
+            "results": {
+                UUID: {
+                    "status": "granted",
+                    "url": url,
+                    "purl": PURL,
+                    "artifacts": [{
+                        "kind": "tarball", "url": url,
+                        "integrity": { "sha512": sha512_sri(tarball), "sha1": sha1_hex(tarball) }
+                    }],
+                    "registryOverride": null
+                }
+            }
+        })))
+        .mount(server)
+        .await;
+    mock_api(server, tarball, Some(tarball.to_vec())).await;
+}