Fix hosted yarn classic rescan of v4 pins (#1462)

crates/socket-patch-cli/CLI_CONTRACT.md

@@ -1442,6 +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/<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. |
 | `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. |
@@ -1499,7 +1500,7 @@ command's integration tests check its output.
 | `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/<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. Also `failed` on `repair` (and `vendor_check_failed` on `vendor --check`) for a lock an older release already vendored that way, with a block wired to the entry naming a descriptor no block locks and the recorded pre-vendor block did not (#1379); the lock is left as it is. Remedy: `yarn install` (without `--frozen-lockfile`) to lock them, then commit `yarn.lock`. |
-| `redirect_yarn_classic_dep_manifest_unlocked` | `warnings[]` (warning) | scan/get `--mode hosted` (yarn classic): the served tarball's own `package.json` depends on a descriptor no `yarn.lock` block is keyed by. yarn 1 installs only what the lock names, so a pin would install the patched package without that dependency (#591). The dep is not pinned and never confirmed; the lock is left as it was. Same remedy as `vendor_dep_manifest_unlocked`. |
+| `redirect_yarn_classic_dep_manifest_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_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. |

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

@@ -23,6 +23,7 @@ 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::{
     build_document, detect_product, BuildOptions, Document, FailedPatch, VendorContext,
     VerifyOutcome,
@@ -1781,6 +1782,10 @@ fn omission_phrase(reason: &str) -> &'static str {
              against"
         }
         "stale_install" => "the installed copy is not patched",
+        DEPENDENCY_NOT_INSTALLED => {
+            "the patched package.json declares a dependency that is not installed beside \
+             the package"
+        }
         "mirror_overrides_source" => {
             "a Bundler mirror overrides the hosted source and installed bytes were not verified"
         }
@@ -1986,6 +1991,7 @@ mod tests {
             "vendor_manifest_unverifiable",
             "stale_install",
             "mirror_overrides_source",
+            DEPENDENCY_NOT_INSTALLED,
             RECORD_UNAVAILABLE,
             RECORD_MISMATCH,
             VENDOR_UNWIRED,

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

@@ -672,6 +672,52 @@ fn hosted_lockfile_only_matrix() {
     }
 }
 
+/// #1462: a hosted pin an earlier release wrote for a patch whose
+/// package.json adds a dependency (`is-odd`) the lock never locked. yarn
+/// installs the patched bytes without that dependency, so although every
+/// patched file verifies, the patch is omitted until the dependency is
+/// installed beside it.
+#[test]
+fn hosted_patch_adding_an_uninstalled_dependency_is_omitted() {
+    const MANIFEST: &str = "package/package.json";
+    let pristine_manifest = format!(r#"{{"name":"{NAME}","version":"{VERSION}"}}"#);
+    let patched_manifest = format!(
+        r#"{{"name":"{NAME}","version":"{VERSION}","dependencies":{{"is-odd":"^3.0.0"}}}}"#
+    );
+    let mut view = patch_view(UUID, PURL);
+    view["files"][MANIFEST] = serde_json::json!({
+        "beforeHash": compute_git_sha256_from_bytes(pristine_manifest.as_bytes()),
+        "afterHash": compute_git_sha256_from_bytes(patched_manifest.as_bytes()),
+    });
+    let (rt, server) = serve_patch_views(vec![(UUID, view)]);
+    for flavor in FLAVORS {
+        let tmp = tempfile::tempdir().unwrap();
+        let cwd = tmp.path();
+        hosted_checkout(cwd, flavor);
+        install(cwd, PATCHED);
+        put(
+            cwd,
+            "node_modules/left-pad/package.json",
+            patched_manifest.as_bytes(),
+        );
+
+        let (code, env) = vex_json(cwd, &["--proxy-url", &server.uri()]);
+        let cell = format!("{flavor:?} dependency not installed");
+        assert_omitted(cwd, code, &env, "dependency_not_installed", &cell);
+
+        // Hoisted beside it, the dependency resolves: attested.
+        put(
+            cwd,
+            "node_modules/is-odd/package.json",
+            br#"{"name":"is-odd","version":"3.0.1"}"#,
+        );
+        let (code, env) = vex_json(cwd, &["--proxy-url", &server.uri()]);
+        let cell = format!("{flavor:?} dependency installed");
+        assert_attested(cwd, code, &env, UUID, "redirected", &cell);
+    }
+    drop(rt);
+}
+
 // ──────────────────────────────────────────────────────────────────────
 // VENDORED — lockfile only (cells a, b, e)
 // ──────────────────────────────────────────────────────────────────────

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

@@ -341,3 +341,100 @@ async fn mock_api_with_sha1(server: &MockServer, tarball: &[u8]) {
         .await;
     mock_api(server, tarball, Some(tarball.to_vec())).await;
 }
+
+/// The patched tarball of #1462: its package.json adds `is-odd`, which
+/// [`LOCK`] doesn't lock.
+fn tarball_adding_a_dependency() -> Vec<u8> {
+    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"),
+    ])
+}
+
+/// [`write_classic_project`] with `left-pad` already pinned to the hosted
+/// artifact the way v4.0.0 pinned it: `resolved` carries the tarball's
+/// sha1, and the block's dependencies were never checked against the
+/// served package.json. Returns the pinned lock.
+fn write_v4_pinned_project(root: &Path, api: &str, tarball: &[u8]) -> String {
+    write_classic_project(root);
+    let url = format!("{api}/patch/npm/left-pad/1.3.0/{TOKEN}/{UUID}/left-pad-1.3.0.tgz");
+    let lock = format!(
+        "# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.\n\
+         # yarn lockfile v1\n\n\n\
+         left-pad@1.3.0:\n  version \"1.3.0\"\n  \
+         resolved \"{url}#{}\"\n  integrity {}\n",
+        sha1_hex(tarball),
+        sha512_sri(tarball)
+    );
+    std::fs::write(root.join("yarn.lock"), &lock).unwrap();
+    lock
+}
+
+/// #1462: a lock an earlier release pinned to a tarball whose package.json
+/// adds a dependency the lock doesn't lock. Yarn installs the patched
+/// package without it, so the rescan must not confirm the pin: it names
+/// the unlocked dependency and reports the patch as not hosted, whether
+/// the grant carries the sha1 (the tarball would otherwise never be
+/// read) or not.
+async fn assert_v4_pin_with_unlocked_dependency_is_not_confirmed(with_sha1: bool) {
+    let server = MockServer::start().await;
+    let tarball = tarball_adding_a_dependency();
+    if with_sha1 {
+        mock_api_with_sha1(&server, &tarball).await;
+    } else {
+        mock_api(&server, &tarball, Some(tarball.clone())).await;
+    }
+    let tmp = tempfile::tempdir().unwrap();
+    let root = tmp.path().join("proj");
+    let lock = write_v4_pinned_project(&root, &server.uri(), &tarball);
+
+    let (code, doc, stderr) = scan(&root, &server.uri());
+    let warning = doc["warnings"]
+        .as_array()
+        .unwrap()
+        .iter()
+        .find(|w| w["code"] == "redirect_yarn_classic_dep_manifest_unlocked")
+        .unwrap_or_else(|| panic!("refusal warning (sha1: {with_sha1}): {doc:#}"));
+    let detail = warning["detail"].as_str().unwrap();
+    assert!(
+        detail.contains("is-odd@^3.0.0") && detail.contains("an earlier run's pin of it"),
+        "{detail}"
+    );
+    assert!(
+        common::envelope::hosted_pins(&doc).is_empty(),
+        "the stale pin is not confirmed (sha1: {with_sha1}): {doc:#}"
+    );
+    assert!(
+        !common::envelope::mode_events(&doc, "hosted")
+            .iter()
+            .any(|e| e["purl"] == PURL && e["action"] == "applied"),
+        "no applied event (sha1: {with_sha1}): {doc:#}"
+    );
+    // Like a fresh refusal, the patch is listed as not hosted; the exit
+    // code is unchanged pending the hosted exit-policy decision (#704).
+    assert!(
+        common::envelope::mode_events(&doc, "hosted")
+            .iter()
+            .any(|e| e["purl"] == PURL && e["action"] == "skipped"),
+        "listed as not hosted (sha1: {with_sha1}): {doc:#}"
+    );
+    assert_eq!(code, 0, "{doc:#}\n{stderr}");
+    assert_eq!(
+        std::fs::read_to_string(root.join("yarn.lock")).unwrap(),
+        lock,
+        "the scan never rewrites the earlier pin"
+    );
+}
+
+#[tokio::test]
+async fn issue_1462_v4_pin_with_unlocked_dependency_is_not_confirmed_sha1_grant() {
+    assert_v4_pin_with_unlocked_dependency_is_not_confirmed(true).await;
+}
+
+#[tokio::test]
+async fn issue_1462_v4_pin_with_unlocked_dependency_is_not_confirmed_sha512_only_grant() {
+    assert_v4_pin_with_unlocked_dependency_is_not_confirmed(false).await;
+}