Hosted gem rollback / remove after an unconverged superseding re-scan restore Gemfile.lock but leave the new patch's source … do block in the Gemfile, report success, and the next bundle install re-patches #1626

[agent] Found by the scheduled Bundler (RubyGems) bug-hunt routine (ledger #316).

Summary

On a lock without a CHECKSUMS section (the Bundler < 2.6 default, and the 2.6 default too), a hosted re-scan that picks up a superseding patch (same gem and version, new uuid) rewrites only the Gemfile. That's documented: the scan warns redirect_gem_no_checksums_section / redirect_gem_frozen_install and asks for one unfrozen bundle install. Until that install runs, the pair is mixed: Gemfile.lock still pins the old uuid's patch-registry GEM section, and the Gemfile holds a source "<registry>/<NEW uuid>/" do … end block.

If rollback <purl> or remove <purl> runs in that window:

• Discovery finds the old uuid in the lock and restores the lock only. find_block matches the Gemfile block by uuid, so the new uuid's block isn't found and stays put.
• The command exits 0 with rolledBack: 1 / removed: 1, hosted.editedFiles: 1, and no warning about the Gemfile.
• The pair is now inconsistent. Every frozen install fails ("The list of sources changed … frozen mode is set"). The next unfrozen bundle install re-resolves through the Gemfile's Socket block and installs the new patch's bytes again, rewriting the lock back to the patch registry.
• A second rollback / remove can't see the Gemfile-only pin and fails with manifest_not_found, so the user can't undo the patch with socket-patch.

Impact

An unwind reported as successful leaves the gem hosted-patched in practice: the next routine bundle install puts the patched bytes back. Scheduled or bot-driven scans make this window easy to hit (a scan PR picks up a superseding patch, then someone runs rollback / remove on that branch before a bundle install). Frozen CI on the result fails until someone edits the Gemfile by hand.

Repro

This is a scratch test appended to crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs. It uses that file's own helpers, mount_patch_generation from the #1186 test, and a real bundle install. Run it with SOCKET_PATCH_BUNDLER_E2E_REQUIRED=1 cargo test --release -p socket-patch-cli --test e2e_redirect_gem_build scratch_rollback -- --include-ignored --nocapture.

#[tokio::test(flavor = "multi_thread")]
#[ignore = "scratch"]
async fn scratch_rollback_after_unconverged_supersede() {
    // no CHECKSUMS lock
    let Some(fx) = redirect_scanned_project("scratch", 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("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(), patched: lib.clone(),
        gem: build_gem(&stage.join(uuid), DEP, DEP_VERSION, "vuln_gem.rb", &lib, &[TRANSITIVE]),
    };
    let api = fx._server.uri();
    mount_patch_generation(&fx._server, 2, &[vuln(GEN2, patched_lib())]).await;
    assert_eq!(run_hosted_scan(&fx.proj, &api).0, 0);
    assert!(bundle(&fx.proj, &["install"]).status.success());          // converge gen 2
    mount_patch_generation(&fx._server, 1, &[vuln(GEN3, patched_lib().replace("PATCHED", "PATCHED-GEN3"))]).await;
    assert_eq!(run_hosted_scan(&fx.proj, &api).0, 0);                  // Gemfile -> GEN3, lock still GEN2
    let upstream = format!("{api}/upstream");
    let cwd = fx.proj.to_str().unwrap();
    let argv = ["rollback", PURL, "--json", "--cwd", cwd, "--api-url", &api, "--org", ORG,
                "--api-token", "fake", "--patch-server-url", &api];
    let (code, out, _) = run_socket_env(&fx.proj, &argv, &[("SOCKET_RUBYGEMS_URL", &upstream)]);
    assert_eq!(code, 0, "{out}");                                      // passes: rolledBack 1
    let gemfile = std::fs::read_to_string(fx.proj.join("Gemfile")).unwrap();
    assert!(!gemfile.contains(GEN3), "Gemfile still wired to the new patch:\n{gemfile}"); // FAILS
}

Observed after rollback (and the same after remove --yes):

Gemfile.lock:  GEM remote: …/upstream/   vuln-gem (1.0.0)   DEPENDENCIES vuln-gem (= 1.0.0)   (restored)
Gemfile:       source "…/upstream"
               source "…/patch-registry/gem/<token>/9a9a9a9a-…/" do
                 gem "vuln-gem", "1.0.0"
               end
BUNDLE_FROZEN=true bundle install  -> "The list of sources changed, but the lockfile can't be updated because frozen mode is set"
bundle install (unfrozen, no cache) -> ok; installs "PATCHED-GEN3-…" bytes; lock back on the 9a9a9a9a registry section
second rollback                     -> exit 1, manifest_not_found

Expected vs actual

• Expected: CLI_CONTRACT.md, "Hosted unwind coverage", gem: "the spec moves back into the upstream GEM section …, the source "<patch registry>" do … end block is undone". When the restore acts on a gem, the pair should come back consistent: undo any Socket patch-registry block for that same name/version (the block's uuid differs only because of a superseding scan), or, if that's out of scope, refuse or warn instead of reporting success. The same section lists "a gem wired only in the Gemfile (pre-bundler-2.6 mixed state)" as out of reach. But here the gem is discovered and acted on through its lock pin, and the run reports success while the gem stays patched, with no warning.
• Actual: the lock is restored, the Gemfile block is kept, the exit is 0 with no warning, frozen installs break, the next unfrozen install re-patches, and a second unwind can't see the gem.

Matrix (Linux, Ruby 3.3.6, main 657fce2; each cell run at least twice)

A committed vendor/cache is irrelevant. The defect reproduces with or without it (with one, upstream_gem_stale_cache correctly names the archive as well).

First bad version

Not bisected. The v5 ledgerless restore keys the Gemfile block by the lock-discovered uuid. The pre-v5 ledger replay is a different code path.

Suspect code

• crates/socket-patch-core/src/patch/redirect/upstream/gem.rs:453: find_block keeps only blocks whose registry URL carries gem.uuid (the lock's uuid), so a superseding uuid's block for the same name/version is never matched.
• crates/socket-patch-core/src/patch/redirect/upstream/gem.rs:1377-1450: restore_one then restores the lock (wired == true) and returns the manifest unchanged, with no warning.