Fix gem rollback leaving superseding Gemfile block (#1626)

crates/socket-patch-cli/CLI_CONTRACT.md

@@ -1069,7 +1069,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`). When the lock already holds a crates.io block for the same `name@version` (a later dependent locked its own copy, #679), the Socket block merges into it instead: it is dropped (with a v1 lock's `[metadata]` line), and dependents' references take the spelling cargo writes, so no duplicate block is left that cargo cannot parse (#863), and no index lookup is needed; 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 `static_urls` `pdm.lock` whose project installs from an index other than PyPI (a `[[tool.pdm.source]]` in the sibling `pyproject.toml`, a project `pypi.url` (`.pdm.toml`, then `pdm.toml`), or, with neither, another package's file URL in the lock off PyPI's file host), since restoring PyPI's file URLs would make `pdm sync` bypass that index (#413); 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 an `exclude-newer-package` cutoff for the restored package; one of `false` exempts it from the global cutoff here and in pylocks, as in uv) (#1604); a pylock from `uv pip compile` whose header command line (`--no-build`, `--only-binary`, `--no-binary`, `--exclude-newer`, `--exclude-newer-package`) or uv settings (the nearest `uv.toml`, else a pyproject's `[tool.uv]` / `[tool.uv.pip]`, then the user's and the system's `uv.toml` unless `UV_NO_CONFIG` is set, merged as uv does with `[pip]` over the top level; none under `--no-config`, and any `--config-file` refuses; `UV_CONFIG_FILE` or `UV_EXCLUDE_NEWER` set in the restoring environment also refuses, since the header does not record uv's environment) filter the restored package's files, or whose header records no `uv pip compile` command line (`--no-header`, `UV_CUSTOM_COMPILE_COMMAND`), since uv 0.9.16 records only the files those filters let through (#1613); a uv lock 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`). Hosted mode keeps no ledger, so it marks the entry it adds with a `# socket-patch hosted: …` comment line above it and removes only a marked entry; a user's own `<name>==<version>` override (which the hosted rewrite reuses rather than adding a second one) is kept, and the lock's `[manifest] overrides` record of it gets its specifier back.
-  * **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 `!`, unless the declaration comes back inside the user's own `source "…" do` block (Bundler writes the `!` for any dependency declared in one, rubygems.org included), where the `!` stays. 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.
+  * **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 (also when a superseding hosted re-scan on a lock with no `CHECKSUMS` section has moved that block to the newer patch of the same name and version while the lock still pins the older one; several such blocks are refused), the `CHECKSUMS` entry is re-pinned from the rubygems.org compact index (`SOCKET_RUBYGEMS_URL`) and the `DEPENDENCIES` pin loses its `!`, unless the declaration comes back inside the user's own `source "…" do` block (Bundler writes the `!` for any dependency declared in one, rubygems.org included), where the `!` stays. 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 lock entry of the id at the pinned version, in every lock the root config governs (member projects' and `packages.<Project>.lock.json` included), 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`).

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

@@ -2272,6 +2272,131 @@ async fn gem_hosted_superseding_patch_keeps_gem_sections_in_bundler_order() {
     );
 }
 
+/// #1626: on a lock with no CHECKSUMS section, a superseding hosted
+/// re-scan moves only the Gemfile's `source … do` block to the new patch;
+/// the lock keeps pinning the old one until a `bundle install`. A
+/// `rollback` / `remove` in that window must undo the new block along with
+/// the lock, or the pair stays inconsistent (frozen installs fail) and the
+/// next unfrozen install re-patches.
+#[tokio::test(flavor = "multi_thread")]
+#[ignore = "host capstone: shells out to a real ruby/gem/bundler; \
+            the unpinned `test` job skips it, an e2e job with a pinned toolchain runs it via --ignored"]
+async fn gem_hosted_unwind_after_unconverged_supersede_undoes_the_new_block() {
+    for command in ["rollback", "remove"] {
+        unwind_after_unconverged_supersede(command).await;
+    }
+}
+
+async fn unwind_after_unconverged_supersede(command: &str) {
+    let Some(fx) = redirect_scanned_project(
+        &format!("unconverged-supersede-{command}"),
+        Spelling::Gemfile,
+        false,
+        true,
+        None,
+        Driver::ScanVex,
+    )
+    .await
+    else {
+        return;
+    };
+    const GEN2: &str = "10000000-1a2b-4a1b-8c2d-3e4f5a6b7c8d";
+    const GEN3: &str = "9a9a9a9a-1a2b-4a1b-8c2d-3e4f5a6b7c8d";
+    let stage = fx.tmp.path().join("generation-stage");
+    let vuln = |uuid: &'static str, lib: String| GenerationGem {
+        name: DEP,
+        uuid,
+        deps: vec![format!("{TRANSITIVE}:>= 0")],
+        lib_file: "vuln_gem.rb",
+        orig: orig_lib(),
+        gem: build_gem(
+            &stage.join(uuid),
+            DEP,
+            DEP_VERSION,
+            "vuln_gem.rb",
+            &lib,
+            &[TRANSITIVE],
+        ),
+        patched: lib,
+    };
+    let api = fx._server.uri();
+    mount_patch_generation(&fx._server, 2, &[vuln(GEN2, patched_lib())]).await;
+    let (code, out, err) = run_hosted_scan(&fx.proj, &api);
+    assert_eq!(code, 0, "{command}: gen 2 scan: {out}\n{err}");
+    let install = bundle(&fx.proj, &["install"]);
+    assert!(
+        install.status.success(),
+        "{command}: converge gen 2:\n{}",
+        String::from_utf8_lossy(&install.stderr)
+    );
+    mount_patch_generation(
+        &fx._server,
+        1,
+        &[vuln(GEN3, patched_lib().replace("PATCHED", "PATCHED-GEN3"))],
+    )
+    .await;
+    let (code, out, err) = run_hosted_scan(&fx.proj, &api);
+    assert_eq!(code, 0, "{command}: gen 3 scan: {out}\n{err}");
+    let gemfile_path = fx.proj.join(fx.gemfile_name);
+    let lock_path = fx.proj.join(fx.lock_name);
+    // The unconverged window this test is about: Gemfile on gen 3, lock on
+    // gen 2.
+    let gemfile = std::fs::read_to_string(&gemfile_path).unwrap();
+    let lock = std::fs::read_to_string(&lock_path).unwrap();
+    assert!(gemfile.contains(GEN3), "{command}: Gemfile:\n{gemfile}");
+    assert!(
+        lock.contains(GEN2) && !lock.contains(GEN3),
+        "{command}: lock:\n{lock}"
+    );
+
+    let upstream = format!("{api}/upstream");
+    let cwd = fx.proj.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(&fx.proj, &argv, &[("SOCKET_RUBYGEMS_URL", &upstream)]);
+    assert_eq!(code, 0, "{command}: {stdout}\nstderr:\n{stderr}");
+    let gemfile = std::fs::read_to_string(&gemfile_path).unwrap();
+    let lock = std::fs::read_to_string(&lock_path).unwrap();
+    assert!(
+        !gemfile.contains("patch-registry"),
+        "{command}: the Gemfile is still wired to a Socket patch:\n{gemfile}"
+    );
+    assert!(
+        !lock.contains("patch-registry"),
+        "{command}: the lock is still wired to a Socket patch:\n{lock}"
+    );
+    assert!(
+        gemfile.contains(&format!("gem \"{DEP}\"")),
+        "{command}: the Gemfile declares the gem again:\n{gemfile}"
+    );
+    // The pair is consistent: a frozen install accepts it as is.
+    let frozen = bundle_env(&fx.proj, &["install"], &[("BUNDLE_FROZEN", "true")]);
+    assert!(
+        frozen.status.success(),
+        "{command}: frozen install after the unwind:\nstdout:\n{}\nstderr:\n{}",
+        String::from_utf8_lossy(&frozen.stdout),
+        String::from_utf8_lossy(&frozen.stderr)
+    );
+    assert_eq!(
+        std::fs::read_to_string(&lock_path).unwrap(),
+        lock,
+        "{command}: the frozen install leaves the restored lock as is"
+    );
+}
+
 /// Bundler's modern `gems.rb`/`gems.locked` spelling, end to end: the
 /// candidate list must read the pair, the rewriter must key its edits to it,
 /// and the real bundler must install the patched gem from the redirected