Fix yarn classic VEX for unlocked dep ranges (#1538)

crates/socket-patch-cli/CLI_CONTRACT.md

@@ -1442,7 +1442,8 @@ 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/<eco>/<uuid>/` 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 -- <lockfile>` 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/<name>` in it or above its real path), for example because the lockfile never locked it (a yarn classic pin written before the served-manifest check, #1462). Omitted until the dependency is installed. |
+| `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/<name>` 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_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. |
 | `vendor_unwired`          | `skipped`        | vex: a vendor-ledger entry whose committed artifact no lockfile/config wires any more (reverted lock, leftover ledger or artifact). Applies under `--no-verify` too. |

crates/socket-patch-cli/src/commands/vex.rs

@@ -16,14 +16,14 @@
 //! * `--json` (requires `--output`): VEX JSON to file, envelope JSON
 //!   to stdout. This is the CI integration shape.
 
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 use std::path::{Path, PathBuf};
 
 use clap::Args;
 use socket_patch_core::api::client::ApiClient;
 use socket_patch_core::manifest::schema::{PatchManifest, PatchRecord};
 use socket_patch_core::telemetry::{track_vex_failed, track_vex_generated, TelemetryAuth};
-use socket_patch_core::vex::verify::DEPENDENCY_NOT_INSTALLED;
+use socket_patch_core::vex::verify::{npm_manifest_after_hash, DEPENDENCY_NOT_INSTALLED};
 use socket_patch_core::vex::{
     build_document, detect_product, BuildOptions, Document, FailedPatch, VendorContext,
     VerifyOutcome,
@@ -541,6 +541,118 @@ async fn hosted_gem_mirror_captured(
     captured
 }
 
+/// Lock-only verdicts of hosted yarn classic pins (#1538): `purls` the
+/// plan may attest from the lock alone, mapped to the omission tag of each
+/// whose patch changes the package's `package.json` while the pinned
+/// block's dependency sub-maps are not exactly the ones that manifest
+/// declares, each locked. Yarn 1 installs what the lock names, so such a
+/// pin (one an earlier release wrote, #1462) installs the patched package
+/// without its patched dependencies: `dependency_not_installed`. A patched
+/// `package.json` that can be read neither from `.socket/blobs` nor from
+/// the patch API is `dependency_manifest_unverifiable`.
+async fn yarn_classic_lock_dependency_gaps(
+    common: &GlobalArgs,
+    params: &VexBuildParams,
+    plan: &Plan,
+    purls: &[&str],
+) -> HashMap<String, &'static str> {
+    let mut gaps = HashMap::new();
+    for purl in purls {
+        let blocks: Vec<_> = plan
+            .hosted
+            .get(*purl)
+            .into_iter()
+            .flat_map(|wiring| &wiring.refs)
+            .filter_map(|r| r.yarn_classic_block.as_ref())
+            .collect();
+        let Some(after_hash) = plan
+            .view
+            .patches
+            .get(*purl)
+            .and_then(npm_manifest_after_hash)
+        else {
+            continue;
+        };
+        if blocks.is_empty() {
+            continue;
+        }
+        let pkg = patched_blob(common, params, after_hash)
+            .await
+            .and_then(|bytes| serde_json::from_slice::<serde_json::Value>(&bytes).ok())
+            .filter(serde_json::Value::is_object);
+        let gap = match pkg {
+            None => DEPENDENCY_MANIFEST_UNVERIFIABLE,
+            Some(pkg) if blocks.iter().any(|b| !b.installs_dependencies_of(&pkg)) => {
+                DEPENDENCY_NOT_INSTALLED
+            }
+            Some(_) => continue,
+        };
+        gaps.insert(purl.to_string(), gap);
+    }
+    gaps
+}
+
+/// Omission tag: a hosted yarn classic pin attested from the lock alone
+/// whose patch changes `package.json`, which could not be read to check
+/// the dependencies the lock installs beside it (#1538).
+const DEPENDENCY_MANIFEST_UNVERIFIABLE: &str = "dependency_manifest_unverifiable";
+
+/// Largest patched `package.json` blob [`patched_blob`] reads.
+const MAX_PATCHED_MANIFEST_BYTES: usize = 4 * 1024 * 1024;
+
+/// The patched file whose git-sha256 is `after_hash`: from the local
+/// `.socket/blobs` store, else (unless `--offline`) from the patch API.
+/// `None` when neither has it intact.
+async fn patched_blob(
+    common: &GlobalArgs,
+    params: &VexBuildParams,
+    after_hash: &str,
+) -> Option<Vec<u8>> {
+    use socket_patch_core::hash::git_sha256::compute_git_sha256_from_bytes;
+    // A record's hash names a file in the blob store: never a path.
+    if after_hash.len() != 64 || !after_hash.bytes().all(|b| b.is_ascii_hexdigit()) {
+        return None;
+    }
+    let intact = |bytes: &Vec<u8>| {
+        bytes.len() <= MAX_PATCHED_MANIFEST_BYTES
+            && compute_git_sha256_from_bytes(bytes).eq_ignore_ascii_case(after_hash)
+    };
+    let local = common.socket_dir().join("blobs").join(after_hash);
+    let small = tokio::fs::metadata(&local)
+        .await
+        .is_ok_and(|meta| meta.len() <= MAX_PATCHED_MANIFEST_BYTES as u64);
+    if small {
+        if let Some(bytes) = socket_patch_core::utils::fs::read_regular_to_bytes(&local)
+            .await
+            .ok()
+            .filter(intact)
+        {
+            return Some(bytes);
+        }
+    }
+    if common.offline {
+        return None;
+    }
+    let overrides = common.api_client_overrides();
+    let client = params
+        .api_client
+        .get_or_init(|| async {
+            socket_patch_core::api::client::get_api_client_with_overrides(overrides)
+                .await
+                .0
+        })
+        .await;
+    let mut body = client.fetch_blob(after_hash).await.ok()??;
+    let mut bytes = Vec::new();
+    while let Some(chunk) = body.chunk().await.ok()? {
+        bytes.extend_from_slice(chunk.as_ref());
+        if bytes.len() > MAX_PATCHED_MANIFEST_BYTES {
+            return None;
+        }
+    }
+    Some(bytes).filter(intact)
+}
+
 /// Core VEX pipeline shared by the standalone `vex` command and the
 /// embedded `apply`/`vendor`/`scan` `--vex` paths: resolve the product, verify the
 /// plan's record view against disk (unless `no_verify`), build the OpenVEX
@@ -738,17 +850,34 @@ async fn generate_vex(
         };
         let hidden =
             |purl: &str| purl.starts_with("pkg:npm/") && (npm_store_hidden || pnp_unconsumed(purl));
+        let excusable: HashSet<String> = outcome
+            .failed
+            .iter()
+            .filter(|f| {
+                f.reason == "package_not_found"
+                    && plan.lockfile_basis.contains(&f.purl)
+                    && !mirror_refused(&f.purl)
+                    && crawled(&f.purl)
+                    && !hidden(&f.purl)
+            })
+            .map(|f| f.purl.clone())
+            .collect();
+        // A yarn classic pin attests from the lock only when the lock
+        // installs the patched package's dependencies with it (#1538).
+        let excusable_purls: Vec<&str> = excusable.iter().map(String::as_str).collect();
+        let dependency_gaps =
+            yarn_classic_lock_dependency_gaps(common, params, &plan, &excusable_purls).await;
         let mut lockfile_attested = Vec::new();
-        outcome.failed.retain(|f| {
-            let excused = f.reason == "package_not_found"
-                && plan.lockfile_basis.contains(&f.purl)
-                && !mirror_refused(&f.purl)
-                && crawled(&f.purl)
-                && !hidden(&f.purl);
-            if excused {
-                lockfile_attested.push(f.purl.clone());
+        outcome.failed.retain_mut(|f| {
+            if f.reason != "package_not_found" || !excusable.contains(&f.purl) {
+                return true;
+            }
+            if let Some(gap) = dependency_gaps.get(&f.purl) {
+                f.reason = gap.to_string();
+                return true;
             }
-            !excused
+            lockfile_attested.push(f.purl.clone());
+            false
         });
         outcome.applied.extend(lockfile_attested);
         withhold_unpatched_jvm_copies(
@@ -1786,6 +1915,11 @@ fn omission_phrase(reason: &str) -> &'static str {
             "the patched package.json declares a dependency that is not installed beside \
              the package"
         }
+        DEPENDENCY_MANIFEST_UNVERIFIABLE => {
+            "the patch changes package.json, and the patched package.json could be read \
+             neither from .socket/blobs nor from the patch API to check the dependencies the \
+             lockfile installs with it"
+        }
         "mirror_overrides_source" => {
             "a Bundler mirror overrides the hosted source and installed bytes were not verified"
         }
@@ -1992,6 +2126,7 @@ mod tests {
             "stale_install",
             "mirror_overrides_source",
             DEPENDENCY_NOT_INSTALLED,
+            DEPENDENCY_MANIFEST_UNVERIFIABLE,
             RECORD_UNAVAILABLE,
             RECORD_MISMATCH,
             VENDOR_UNWIRED,

crates/socket-patch-cli/tests/e2e_vex_lockfile/yarn.rs

@@ -705,10 +705,114 @@ fn hosted_patch_adding_an_uninstalled_dependency_is_omitted() {
         let (code, env) = vex_json(cwd, &["--proxy-url", &server.uri()]);
         let cell = format!("{flavor:?} dependency installed");
         assert_attested(cwd, code, &env, UUID, "redirected", &cell);
+
+        // #1538: installed, but outside the range the patched package.json
+        // declares (the old dependency a pre-v5 pin's lock still installs).
+        put(
+            cwd,
+            "node_modules/is-odd/package.json",
+            br#"{"name":"is-odd","version":"2.0.0"}"#,
+        );
+        let (code, env) = vex_json(cwd, &["--proxy-url", &server.uri()]);
+        let cell = format!("{flavor:?} dependency out of range");
+        assert_omitted(cwd, code, &env, "dependency_not_installed", &cell);
     }
     drop(rt);
 }
 
+/// #1538: lock-only VEX of a yarn classic hosted pin whose patch changes
+/// `package.json`. yarn 1 installs the dependencies the pinned block's
+/// sub-maps name, so a pin an earlier release wrote without rewriting them
+/// (#1462) installs the patched package beside the old dependencies.
+///
+/// | lock | patched blob | verdict |
+/// |---|---|---|
+/// | no sub-map, patch adds `is-odd ^3.0.0` | patch API | dependency_not_installed |
+/// | sub-map `is-odd ^2.0.0`, patch says `^3.0.0` | patch API | dependency_not_installed |
+/// | sub-map rewritten, `is-odd@^3.0.0` locked | patch API | attests |
+/// | same | `.socket/blobs` only | attests |
+/// | same | nowhere | dependency_manifest_unverifiable |
+#[test]
+fn hosted_classic_lock_only_checks_the_pinned_dependency_maps() {
+    use wiremock::matchers::{method, path};
+    use wiremock::{Mock, ResponseTemplate};
+    const MANIFEST: &str = "package/package.json";
+    let pristine = format!(
+        r#"{{"name":"{NAME}","version":"{VERSION}","dependencies":{{"is-odd":"^2.0.0"}}}}"#
+    );
+    let patched = format!(
+        r#"{{"name":"{NAME}","version":"{VERSION}","dependencies":{{"is-odd":"^3.0.0"}}}}"#
+    );
+    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.clone())]);
+    let (rt_no_blob, server_no_blob) = serve_patch_views(vec![(UUID, view.clone())]);
+    rt.block_on(
+        Mock::given(method("GET"))
+            .and(path(format!("/patch/blob/{after_hash}")))
+            .respond_with(ResponseTemplate::new(200).set_body_bytes(patched.clone().into_bytes()))
+            .mount(&server),
+    );
+
+    let base = hosted_lock(Flavor::Classic, "https://patch.socket.dev", UUID);
+    let is_odd = |range: &str, version: &str| {
+        format!(
+            "\nis-odd@{range}:\n  version \"{version}\"\n  resolved \
+             \"https://registry.yarnpkg.com/is-odd/-/is-odd-{version}.tgz#\
+             0123456789abcdef0123456789abcdef01234567\"\n  integrity \
+             sha512-aXNvZGRpc29kZGlzb2RkaXNvZGQ=\n"
+        )
+    };
+    let with_sub_map = |range: &str| format!("{base}  dependencies:\n    is-odd \"{range}\"\n");
+    let cells = [
+        (
+            "added dependency",
+            base.clone() + &is_odd("^2.0.0", "2.0.0"),
+        ),
+        (
+            "changed range",
+            with_sub_map("^2.0.0") + &is_odd("^2.0.0", "2.0.0"),
+        ),
+    ];
+    for (cell, lock) in cells {
+        let tmp = tempfile::tempdir().unwrap();
+        let cwd = tmp.path();
+        hosted_checkout(cwd, Flavor::Classic);
+        put(cwd, "yarn.lock", lock.as_bytes());
+        let (code, env) = vex_json(cwd, &["--proxy-url", &server.uri()]);
+        assert_omitted(cwd, code, &env, "dependency_not_installed", cell);
+    }
+
+    // The v5 pin: the sub-map rewritten and the new descriptor locked.
+    let tmp = tempfile::tempdir().unwrap();
+    let cwd = tmp.path();
+    hosted_checkout(cwd, Flavor::Classic);
+    let lock = with_sub_map("^3.0.0") + &is_odd("^3.0.0", "3.0.1");
+    put(cwd, "yarn.lock", lock.as_bytes());
+    let (code, env) = vex_json(cwd, &["--proxy-url", &server.uri()]);
+    assert_attested(cwd, code, &env, UUID, "redirected", "rewritten sub-map");
+
+    // The patched manifest from nowhere: never assumed.
+    let (code, env) = vex_json(cwd, &["--proxy-url", &server_no_blob.uri()]);
+    let cell = "rewritten sub-map, no blob";
+    assert_omitted(cwd, code, &env, "dependency_manifest_unverifiable", cell);
+
+    // ...but read from the local blob store.
+    put(
+        cwd,
+        &format!(".socket/blobs/{after_hash}"),
+        patched.as_bytes(),
+    );
+    let (code, env) = vex_json(cwd, &["--proxy-url", &server_no_blob.uri()]);
+    assert_attested(cwd, code, &env, UUID, "redirected", "local blob");
+    drop(rt);
+    drop(rt_no_blob);
+}
+
 // ──────────────────────────────────────────────────────────────────────
 // VENDORED — lockfile only (cells a, b, e)
 // ──────────────────────────────────────────────────────────────────────