Fix gem unwind leaving patched archive in vendor/cache (#1260)

crates/socket-patch-cli/CLI_CONTRACT.md

@@ -962,7 +962,7 @@ v5.0 replaces v4's per-purl reverts and whole-ledger reverse replay (`revert_rem
   * **cargo** — `Cargo.lock` back on crates.io (source + the sparse index's checksum, `SOCKET_CRATES_INDEX`); every `Cargo.toml` declaration loses its `registry = "socket-patch-<uuid>"` pin (the shorthand the rewriter produced collapses back); every `[registries.socket-patch-<uuid>]` block no manifest or lock still references leaves the project cargo config — including a superseded patch generation's block an earlier re-pin left behind (#864). A declaration it cannot unpin refuses.
   * **golang** — the hosted `replace` and the socket module's go.sum lines go; the upstream module's two go.sum lines come back, hashed from the module proxy (`SOCKET_GOPROXY`, else `GOPROXY` / `GONOPROXY` / `GOPRIVATE` as go reads them) and cross-checked against the checksum database (`SOCKET_GOSUMDB_URL`, else `sum.golang.org` unless `GOSUMDB=off` / `GONOSUMDB` / `GOPRIVATE` say go would not ask it). A `replace` the user had before the hosted run is not recorded anywhere, so the restore lands on the plain upstream module.
   * **pypi** — `Pipfile.lock`, `requirements.txt` (+ in-root `-r` includes), Hatch PEP 508 direct references (`pyproject.toml` / `hatch.toml`), `poetry.lock`, `pdm.lock`, `uv.lock`, PEP 723 script locks and PEP 751 `pylock*.toml` (+ the paired `pyproject.toml` / script metadata): hashes re-derived from PyPI's JSON API (`SOCKET_PYPI_JSON_API`). A restored `requirements.txt` line gets `--hash` options only when the file is in pip's hash-checking mode. The mode is read off the file's other requirement lines (an `-e` / `--editable` line means unhashed). When every requirement is a hosted pin, it is read off the hosted line itself (`--hash` vs a `#sha256=` url fragment) (#410). Refused: a `pdm.lock` without `cross_platform`, or a uv / script / pylock lock, whose release has a wheel that is not pure Python 3 (which files the lock keeps is not re-derivable); a uv lock whose options filter files (`exclude-newer`, `no-binary`, `no-build`), or whose other registry packages name no registry, several, or one other than PyPI's simple index; a pylock whose other registry packages show neither an `index` nor (as `uv pip compile` writes them) only PyPI files with none, which restores the entry without an `index` too; uv 0.2 `[[distribution]]` locks. Restored artifact fields keep the spelling the lock's other entries show, including the `upload_time` that uv 0.6.15–0.6.17 write. A restored pylock entry's `upload-time`s are whole seconds, as uv writes them, unless the lock's other entries show fractions. Its artifacts come back in the TOML spelling the other entries use: uv's inline `wheels = [{ … }]`, or the standard tables `pip lock` writes (`[[packages.wheels]]` with a `[packages.wheels.hashes]` sub-table, `[packages.sdist]`). A `pip lock` file (`created-by = "pip"`) records only the artifact pip selected, so the entry is restored with only the release's wheel (its sdist when it has none), and a release with several wheels is refused. A transitive `override-dependencies` entry hosted mode added is removed (`upstream_uv_override_removed`).
-  * **gem** — `Gemfile.lock` / `gems.locked` + `Gemfile` / `gems.rb`: the spec moves back into the upstream `GEM` section (or the Socket remote leaves a merged section), the `source "<patch registry>" do … end` block is undone, the `CHECKSUMS` entry is re-pinned from the rubygems.org compact index (`SOCKET_RUBYGEMS_URL`) and the `DEPENDENCIES` pin loses its `!`. The declaration's original constraint is not recorded, so it comes back as the exact pin `gem "<name>", "<version>"`. A transitive gem (one the manifest never declared) gets an appended block with a blank line before it; the restore removes that block, its blank line and the `DEPENDENCIES` entry, so the pair comes back byte for byte. An appended block with no blank line before it (written by a release before this one) can't be told apart from an in-place rewrite, so it still comes back as the exact pin. Refused: an ambiguous upstream section, an upstream remote other than rubygems.org.
+  * **gem** — `Gemfile.lock` / `gems.locked` + `Gemfile` / `gems.rb`: the spec moves back into the upstream `GEM` section (or the Socket remote leaves a merged section), the `source "<patch registry>" do … end` block is undone, the `CHECKSUMS` entry is re-pinned from the rubygems.org compact index (`SOCKET_RUBYGEMS_URL`) and the `DEPENDENCIES` pin loses its `!`. The declaration's original constraint is not recorded, so it comes back as the exact pin `gem "<name>", "<version>"`. A transitive gem (one the manifest never declared) gets an appended block with a blank line before it; the restore removes that block, its blank line and the `DEPENDENCIES` entry, so the pair comes back byte for byte. An appended block with no blank line before it (written by a release before this one) can't be told apart from an in-place rewrite, so it still comes back as the exact pin. Refused: an ambiguous upstream section, an upstream remote other than rubygems.org. The manifest pair can't make a committed bundler cache upstream: a `<name>-<version>.gem` left in Bundler's cache dir that isn't the upstream archive is named by `upstream_gem_stale_cache`, never deleted.
   * **composer** — `composer.lock`: `dist` and the deleted `source` block from packagist's composer v2 metadata (`SOCKET_PACKAGIST_URL`). Refused unless the entry is packagist-sourced and packagist still serves the lock's `dist.reference` for the version.
   * **maven** — `pom.xml` (the `-socket.<hex8>` version suffix, the added `<repository>` / `<dependencyManagement>` entry) and the `.mvn/maven.config` / `.mvn/checksums/checksums.sha256` lines hosted mode writes: **no network**, so it restores under `--offline` too. `.mvn` files holding anything else keep the resolver lines (`maven_trusted_checksums_left`).
   * **nuget** — `nuget.config` loses the `socket-patch-<uuid>` source and its exact-id mapping; every `packages.lock.json` entry of the id gets nuget.org's `contentHash` back (`SOCKET_NUGET_URL`). Refused when the restored config would not resolve the id from nuget.org alone. A config hosted mode created from scratch is kept (`nuget_default_config_left`).
@@ -977,7 +977,7 @@ v5.0 replaces v4's per-purl reverts and whole-ledger reverse replay (`revert_rem
 | Key | Shape | Meaning |
 |---|---|---|
 | `error` | `{code, message}` | Only on `status: "error"` (v5.0, MAJOR: was a string). Codes: `manifest_not_found`, `manifest_invalid`, `manifest_unreadable`, `patch_not_found`, `path_glob_no_match`, `hosted_wiring_contested`, `vendor_ledger_missing`, `rollback_failed`, `lock_held` / `lock_io`, and `path_glob_invalid` (a usage error, exit 2). Per-result `results[*].error` stays a string. |
-| `warnings` | `[{code, detail}]` | Run-level warnings, now populated (previously always empty): `reinstall_required`, `hosted_state_not_preservable`, `out_of_scope_copies_restored`, `vendor_state_unreadable`, `cleanup_failed`, `manifest_write_failed`, `legacy_redirect_ledger_kept`, the upstream-restore advisories (`npm_allow_remote_left`, `pnpm_trust_lockfile_left`, `maven_trusted_checksums_left`, `nuget_default_config_left`, `upstream_uv_override_removed`, `upstream_registry_fallback`, `upstream_pnpm_tarball_setting_guessed`), `ownership_not_restored` (a restored file whose ownership could not be put back — see the apply warnings), `rollback_record_superseded` (a manifest record superseded by a live hosted pin, left to the hosted leg — see Manifest cleanup), plus vendored/hosted leg advisories. New codes are additive (MINOR) |
+| `warnings` | `[{code, detail}]` | Run-level warnings, now populated (previously always empty): `reinstall_required`, `hosted_state_not_preservable`, `out_of_scope_copies_restored`, `vendor_state_unreadable`, `cleanup_failed`, `manifest_write_failed`, `legacy_redirect_ledger_kept`, the upstream-restore advisories (`npm_allow_remote_left`, `pnpm_trust_lockfile_left`, `maven_trusted_checksums_left`, `nuget_default_config_left`, `upstream_uv_override_removed`, `upstream_registry_fallback`, `upstream_pnpm_tarball_setting_guessed`, `upstream_gem_stale_cache`), `ownership_not_restored` (a restored file whose ownership could not be put back — see the apply warnings), `rollback_record_superseded` (a manifest record superseded by a live hosted pin, left to the hosted leg — see Manifest cleanup), plus vendored/hosted leg advisories. New codes are additive (MINOR) |
 | `vendored` | `[purl]` | **Meaning narrowed (MAJOR)**: vendor-owned purls the run did NOT act on — today exactly the corrupt-vendor-ledger skip. |
 | `vendoredReverted` | `[purl]` | Ledger entries cleanly reverted this run (unwired + artifact deleted + entry dropped; previewed on dry-run) |
 | `vendoredPreserved` | `[purl]` | `--preserve-state`: unwired with artifact + ledger entry kept |
@@ -1280,6 +1280,7 @@ Every `--json` invocation emits a single JSON object that follows the **unified
 | `npm_allow_remote_left` / `pnpm_trust_lockfile_left` | rollback/remove `warnings[]`; vendor advisory event (takeover) | upstream restore (v5.0): no npm-family lock entry is hosted any more, but the project `.npmrc` keeps a top-level `allow-remote=all` (resp. `pnpm-workspace.yaml` keeps `trustLockfile: true`) in a file that is not exactly what hosted mode creates; the file is left untouched (v5 records no provenance), remove the line if nothing else needs it. A file that is exactly hosted mode's own is deleted silently. |
 | `maven_trusted_checksums_left` / `nuget_default_config_left` / `upstream_uv_override_removed` | rollback/remove `warnings[]`; vendor advisory event (takeover) | upstream restore (v5.0): `.mvn` config keeps the trusted-checksums resolver lines because it holds more than hosted mode writes; `nuget.config` now holds only the nuget.org source (delete it if hosted mode created it); a transitive `override-dependencies` entry hosted mode added to `pyproject.toml` was removed. |
 | `upstream_registry_fallback` | rollback/remove `warnings[]`; vendor advisory event (takeover) | upstream restore: a yarn berry, pnpm or vlt entry is restored from the version document of the registry the project resolves it against (`.yarnrc.yml` `npmRegistryServer`, the pnpm lock's sibling `.npmrc` `registry` / `@scope:registry` or pnpm-workspace.yaml `registry` / `registries`, vlt's node registry); that registry could not be read (e.g. it needs credentials), so the default registry's document was used and the restored tarball URL may not be the mirror's. |
+| `upstream_gem_stale_cache` | rollback/remove `warnings[]`; vendor advisory event (takeover) | upstream restore (#1260): a restored gem's `<name>-<version>.gem` is still in Bundler's cache dir (`cache_path`, default `vendor/cache`, resolved as for the Gem stale-install guard) and its sha256 is not the upstream one (the restored `CHECKSUMS` entry, else the rubygems.org compact index), or it could not be checked (`--offline`, a registry error). Bundler installs from that dir first, so a `bundle cache` taken while the hosted pin was live makes every later install fail on the upstream checksum (exit 37) or, on bundler < 2.6 frozen installs, keep installing the patched bytes. The detail names the file. Remedy: delete it, then run `bundle cache` to cache the upstream gem in its place (or `bundle install` if the project does not commit its cache; with the cache dir committed, a frozen install reads only the cache). Read-only: the restore never deletes it. Not raised for an archive whose sha256 matches upstream. |
 | `upstream_pnpm_tarball_setting_guessed` | rollback/remove `warnings[]`; vendor advisory event (takeover) | upstream restore (#902): nothing showed which pnpm wrote a hosted `pnpm-lock.yaml` (no unpinned registry entry that shows the setting, no `node_modules/.modules.yaml` install record, no package.json `packageManager` pin; for a Rush lock, no rush.json `pnpmVersion`), so its entries were restored with or without `tarball:` by pnpm 10's reading of `lockfileIncludeTarballUrl` (pnpm-workspace.yaml, else `.npmrc` `lockfile-include-tarball-url`), and pnpm 9 (which reads only `.npmrc`) or pnpm >= 11 (which reads only pnpm-workspace.yaml) would have read it the other way. The detail names the lock, the setting followed, the pnpm that disagrees and the entries. Remedy: pin the pnpm (package.json `packageManager`, or reinstall so the install record names it; rush.json `pnpmVersion` for Rush), or give the two files the same value so every pnpm reads it alike; a rollback or remove can then be redone by restoring the lock from version control and re-running. Not raised when evidence decided, when both files read the same on every pnpm, or when the tarball is one pnpm records regardless. |
 | `legacy_redirect_ledger_kept` | rollback `warnings[]` (+ remove stderr) | v5.0: a pre-v5 `.socket/vendor/redirect-state.json` could not be deleted once no hosted pin was left; the file is inert (never read for planning). Never flips the exit. |
 | `vendor_stale_artifact_removed` | `removed`  | vendor / scan `--mode vendored`: re-vendor under a newer patch uuid removed the previous uuid's orphaned artifact dir. |

crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs

@@ -3311,3 +3311,161 @@ async fn gem_hosted_cap_zero_upgrades_a_superseded_gemfile_only_pin() {
         "the Gemfile must move to the superseding patch:\n{gemfile}"
     );
 }
+
+/// Copy the committable files of `src` (manifest pair, `.bundle`, the
+/// committed bundler cache) into a new dir: a fresh checkout with no
+/// installed tree.
+fn checkout_with_cache(fx: &RedirectFixture, src: &Path, name: &str) -> PathBuf {
+    let fresh = fx.tmp.path().join(name);
+    std::fs::create_dir_all(&fresh).unwrap();
+    for file in [fx.gemfile_name, fx.lock_name] {
+        std::fs::copy(src.join(file), fresh.join(file)).unwrap();
+    }
+    copy_dir_recursive(&src.join(".bundle"), &fresh.join(".bundle"));
+    copy_dir_recursive(&src.join("vendor/cache"), &fresh.join("vendor/cache"));
+    fresh
+}
+
+/// #1260 through one unwind `command` (`rollback` / `remove`): a project
+/// that ran `bundle cache` while the hosted pin was live commits the
+/// PATCHED archive. The unwind puts the manifest pair back on rubygems.org
+/// but cannot make that archive upstream, and bundler installs from its
+/// cache first: a fresh checkout either fails on the restored upstream
+/// checksum (exit 37) or keeps installing the patched bytes. The unwind
+/// must name the file, and deleting it must be the whole remedy.
+fn unwind_names_the_patched_cached_archive(fx: &RedirectFixture, command: &str) {
+    let dir = stage_fresh_checkout(fx, &format!("cache-{command}"));
+    let install = bundle(&dir, &["install"]);
+    assert!(
+        install.status.success(),
+        "{command}: hosted install:\n{}",
+        String::from_utf8_lossy(&install.stderr)
+    );
+    let cache_cmd = if fx.bundler.at_least(2, 0) {
+        "cache"
+    } else {
+        "package"
+    };
+    let cache = bundle(&dir, &[cache_cmd]);
+    assert!(
+        cache.status.success(),
+        "{command}: bundle {cache_cmd}:\n{}",
+        String::from_utf8_lossy(&cache.stderr)
+    );
+    let archive = dir
+        .join("vendor")
+        .join("cache")
+        .join(format!("{DEP}-{DEP_VERSION}.gem"));
+    assert!(
+        archive.is_file(),
+        "{command}: bundle {cache_cmd} wrote no archive"
+    );
+
+    let api = fx._server.uri();
+    let upstream = format!("{api}/upstream");
+    let cwd = dir.to_str().expect("utf8 tmp path");
+    let mut argv = vec![command, PURL, "--json", "--cwd", cwd];
+    if command == "remove" {
+        argv.push("--yes");
+    }
+    argv.extend([
+        "--api-url",
+        &api,
+        "--org",
+        ORG,
+        "--api-token",
+        "fake",
+        "--patch-server-url",
+        &api,
+    ]);
+    let (code, stdout, stderr) = run_socket_env(&dir, &argv, &[("SOCKET_RUBYGEMS_URL", &upstream)]);
+    let env: serde_json::Value = serde_json::from_str(&stdout).unwrap_or_else(|e| {
+        panic!("{command}: not JSON: {e}\nstdout:\n{stdout}\nstderr:\n{stderr}")
+    });
+    assert_eq!(code, 0, "{command}: {env}\nstderr:\n{stderr}");
+    let lock = std::fs::read_to_string(dir.join(fx.lock_name)).unwrap();
+    assert!(
+        !lock.contains(&fx.index_url),
+        "{command}: the lock is back on upstream:\n{lock}"
+    );
+    let hits: Vec<&serde_json::Value> = env["warnings"]
+        .as_array()
+        .map(|w| {
+            w.iter()
+                .filter(|w| w["code"] == "upstream_gem_stale_cache")
+                .collect()
+        })
+        .unwrap_or_default();
+    assert_eq!(hits.len(), 1, "{command}: one stale-cache warning: {env}");
+    let detail = hits[0]["detail"].as_str().unwrap();
+    assert!(
+        detail.contains(&archive.display().to_string()),
+        "{command}: the warning names the cached archive: {detail}"
+    );
+
+    // The defect the warning is about: with the archive left in place a
+    // fresh frozen checkout never installs the upstream bytes.
+    let stale = checkout_with_cache(fx, &dir, &format!("cache-{command}-stale"));
+    let install = bundle_env(&stale, &["install"], &[("BUNDLE_FROZEN", "true")]);
+    let lib = fresh_installed_lib(&stale, &format!("{DEP}-{DEP_VERSION}"), "vuln_gem.rb");
+    assert!(
+        !install.status.success() || std::fs::read(&lib).unwrap() == fx.patched,
+        "{command}: bundler {} must reuse the cached patched archive (test premise)",
+        fx.bundler.version
+    );
+
+    // The remedy the warning prescribes: delete the archive and re-cache
+    // (a frozen install with a committed cache dir reads only the cache
+    // on some bundlers), and a fresh frozen checkout installs upstream.
+    let recached = checkout_with_cache(fx, &dir, &format!("cache-{command}-recached"));
+    std::fs::remove_file(
+        recached
+            .join("vendor/cache")
+            .join(format!("{DEP}-{DEP_VERSION}.gem")),
+    )
+    .unwrap();
+    let cache = bundle(&recached, &[cache_cmd]);
+    assert!(
+        cache.status.success(),
+        "{command}: bundle {cache_cmd} after deleting the archive:\n{}",
+        String::from_utf8_lossy(&cache.stderr)
+    );
+    assert_eq!(
+        std::fs::read_to_string(recached.join(fx.lock_name)).unwrap(),
+        lock,
+        "{command}: re-caching leaves the restored lock alone"
+    );
+    let fixed = checkout_with_cache(fx, &recached, &format!("cache-{command}-fixed"));
+    let install = bundle_env(&fixed, &["install"], &[("BUNDLE_FROZEN", "true")]);
+    assert!(
+        install.status.success(),
+        "{command}: frozen install from the re-cached checkout:\n{}",
+        String::from_utf8_lossy(&install.stderr)
+    );
+    let lib = fresh_installed_lib(&fixed, &format!("{DEP}-{DEP_VERSION}"), "vuln_gem.rb");
+    assert_eq!(
+        std::fs::read(&lib).unwrap(),
+        orig_lib().into_bytes(),
+        "{command}: the upstream bytes are installed"
+    );
+}
+
+#[tokio::test(flavor = "multi_thread")]
+#[ignore = "host capstone: shells out to a real ruby/gem/bundler (>= 1.17; CHECKSUMS arm >= 2.6); \
+            the unpinned `test` job skips it, an e2e job with a pinned toolchain runs it via --ignored"]
+async fn gem_hosted_unwind_names_a_patched_archive_in_vendor_cache() {
+    let Some(fx) = redirect_scanned_project(
+        "cache-unwind",
+        Spelling::Gemfile,
+        false,
+        true,
+        None,
+        Driver::ScanVex,
+    )
+    .await
+    else {
+        return;
+    };
+    unwind_names_the_patched_cached_archive(&fx, "rollback");
+    unwind_names_the_patched_cached_archive(&fx, "remove");
+}