From 86ee383b9d83fee25a16564155567d635b3c21e0 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 20:30:34 +0000 Subject: [PATCH 1/3] Start fix for #779 Assisted-by: Claude Code:claude-opus-5-5 From 4008edcf0f1a6a27d6ccf7820f89f0637496157f Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 20:46:24 +0000 Subject: [PATCH 2/3] Vendor gems from any GEM section of the lock A project with a second gem source (a private gem server in a `source "..." do` block) gets one GEM section per source in Gemfile.lock, and rubygems.org can come second. Vendored mode looked only in the first GEM section, so every public gem in such a project failed to vendor with "GEM specs has no entry", although scan and hosted mode saw it. Vendor now finds the spec in whichever GEM section holds it (refusing a spec listed under two sources, and checking every section for a platform-suffixed sibling). When the spec came from a section other than the first, the ledger records that section's remote: line, so revert and the converged check put the spec back where it came from and restore the lock byte for byte. Locks whose spec is in the first section keep the exact ledger shape they had before. Fixes #779 Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/e2e_vendor_gem_build.rs | 229 +++++++++++ crates/socket-patch-core/src/vendor/gem.rs | 360 ++++++++++++++++-- 2 files changed, 547 insertions(+), 42 deletions(-) diff --git a/crates/socket-patch-cli/tests/e2e_vendor_gem_build.rs b/crates/socket-patch-cli/tests/e2e_vendor_gem_build.rs index cd6e29cd7..2376ba61c 100644 --- a/crates/socket-patch-cli/tests/e2e_vendor_gem_build.rs +++ b/crates/socket-patch-cli/tests/e2e_vendor_gem_build.rs @@ -1637,3 +1637,232 @@ fn gem_vendor_refuses_an_eval_gemfile_direct_dep() { ); } } + +/// A static `file://` rubygems source serving one empty gem, +/// `aaa-internal 1.0.0`. It writes the full-index files bundler reads from +/// a file remote (`specs.4.8.gz`, the quick gemspec, the `.gem`), so it +/// works without `gem generate_index` (no longer bundled with RubyGems). +fn private_gem_source(dir: &Path) { + std::fs::create_dir_all(dir).unwrap(); + let script = r#" +require "rubygems/package" +require "zlib" +require "fileutils" +spec = Gem::Specification.new do |s| + s.name = "aaa-internal" + s.version = "1.0.0" + s.summary = "fixture" + s.authors = ["fixture"] + s.files = [] +end +FileUtils.mkdir_p(["gems", "quick/Marshal.4.8"]) +file = Gem::Package.build(spec) +FileUtils.mv(file, "gems/#{file}") +tuple = [[spec.name, spec.version, "ruby"]] +{ "specs.4.8.gz" => tuple, "latest_specs.4.8.gz" => tuple, "prerelease_specs.4.8.gz" => [] }.each do |f, v| + Zlib::GzipWriter.open(f) { |gz| gz.write(Marshal.dump(v)) } +end +File.binwrite("quick/Marshal.4.8/#{spec.full_name}.gemspec.rz", Zlib::Deflate.deflate(Marshal.dump(spec))) +"#; + let mut ruby = Command::new("ruby"); + ruby.args(["-e", script]).current_dir(dir); + cache_env::isolate(&mut ruby); + let out = ruby.output().expect("failed to run ruby"); + assert!( + out.status.success(), + "building the private gem source failed:\n{}", + String::from_utf8_lossy(&out.stderr) + ); +} + +/// #779: a Gemfile with a second rubygems source. Bundler 2+ writes one +/// GEM section per source, sorted by remote, so the `file://` source comes +/// first and rack sits in the second GEM section. Vendor used to fail with +/// "GEM specs has no entry". It must wire rack so a fresh frozen install +/// loads the patched copy, and `vendor --revert` must put rack back into +/// its own section, restoring the lock byte for byte. +#[test] +#[ignore = "host capstone: shells out to a real bundler >= 1.17; the unpinned `test` job \ + skips it, the e2e job runs it with a pinned toolchain via --ignored"] +fn gem_vendor_second_gem_section_fresh_checkout_and_revert() { + let Some(bundler) = gate("second GEM section") else { + return; + }; + let tmp = tempfile::tempdir().unwrap(); + let repo = tmp.path().join("private-gems"); + private_gem_source(&repo); + let proj = tmp.path().join("proj"); + std::fs::create_dir_all(&proj).unwrap(); + let gemfile_text = format!( + "source \"https://rubygems.org\"\n\ngem \"rack\", \"~> 3.1\"\n\nsource \"file://{}\" do\n gem \"aaa-internal\"\nend\n", + repo.display() + ); + std::fs::write(proj.join("Gemfile"), &gemfile_text).unwrap(); + let config = bundle( + &proj, + &argv(&bundler.config_local_args("path", "vendor/bundle")), + false, + ); + let install = config + .status + .success() + .then(|| bundle(&proj, &["install"], false)); + if !install.as_ref().is_some_and(|i| i.status.success()) { + println!( + "SKIP e2e_vendor_gem_build (second GEM section): fixture `bundle install` failed:\n{}", + install + .map(|i| String::from_utf8_lossy(&i.stderr).into_owned()) + .unwrap_or_default() + ); + return; + } + let lock_path = proj.join("Gemfile.lock"); + let gemfile_path = proj.join("Gemfile"); + let lock_before = std::fs::read(&lock_path).unwrap(); + let gemfile_before = std::fs::read(&gemfile_path).unwrap(); + let lock_text = String::from_utf8_lossy(&lock_before).into_owned(); + let version = locked_gem_version(&lock_text, DEP).expect("resolved rack version"); + if bundler.at_least(2, 0) { + // The premise: rack is NOT in the first GEM section. (Bundler 1.x + // merges every rubygems remote into one GEM section.) + let sections: Vec<&str> = lock_text + .split("\n\n") + .filter(|s| s.starts_with("GEM\n")) + .collect(); + let spec = format!("\n {DEP} ({version})"); + assert!( + sections.len() == 2 && !sections[0].contains(&spec) && sections[1].contains(&spec), + "rack must sit in the second of two GEM sections (test premise):\n{lock_text}" + ); + } + + let mut ruby = Command::new("ruby"); + ruby.args(["-e", "puts Gem.ruby_api_version"]); + cache_env::isolate(&mut ruby); + let api = ruby.output().expect("failed to run ruby"); + let api = String::from_utf8_lossy(&api.stdout).trim().to_string(); + let installed_rb = proj + .join("vendor/bundle/ruby") + .join(&api) + .join("gems") + .join(format!("{DEP}-{version}")) + .join("lib/rack.rb"); + let orig = std::fs::read(&installed_rb).expect("installed lib/rack.rb"); + let marker = format!( + "\n# SOCKET-PATCH-VENDOR-E2E-MARKER\nmodule Rack\n SOCKET_PATCH_VENDOR_E2E = \"{UUID}\"\nend\n" + ); + let patched: Vec = [orig.as_slice(), marker.as_bytes()].concat(); + let purl = format!("pkg:gem/{DEP}@{version}"); + stage_patch_with_vuln(&proj, &purl, "lib/rack.rb", &orig, &patched); + + let vendor_args = [ + "vendor", + "--json", + "--offline", + "--cwd", + proj.to_str().unwrap(), + ]; + let (code, stdout, stderr) = run_socket(&proj, &vendor_args); + assert_eq!( + code, 0, + "vendor failed.\nstdout:\n{stdout}\nstderr:\n{stderr}" + ); + let env = parse_envelope(&stdout); + assert_eq!(env["summary"]["applied"], 1, "one package vendored: {env}"); + assert_eq!(env["summary"]["failed"], 0, "no failures: {env}"); + + let copy_rel = format!(".socket/vendor/gem/{UUID}/{DEP}-{version}"); + let lock = std::fs::read_to_string(&lock_path).unwrap(); + assert!( + lock.contains(&format!( + "PATH\n remote: {copy_rel}\n specs:\n {DEP} ({version})" + )), + "canonical PATH section missing from Gemfile.lock:\n{lock}" + ); + assert!( + lock.contains(" aaa-internal (1.0.0)"), + "the private source's spec stays in its GEM section:\n{lock}" + ); + + // Fresh checkout: only the committable files, frozen install, and rack + // loads the patched bytes from the vendored path. + let fresh = tmp.path().join("fresh"); + std::fs::create_dir_all(&fresh).unwrap(); + std::fs::copy(&gemfile_path, fresh.join("Gemfile")).unwrap(); + std::fs::copy(&lock_path, fresh.join("Gemfile.lock")).unwrap(); + copy_dir_recursive(&proj.join(".socket"), &fresh.join(".socket")); + copy_dir_recursive(&proj.join(".bundle"), &fresh.join(".bundle")); + let lock_wired = std::fs::read(&lock_path).unwrap(); + let ci = bundle(&fresh, &["install"], true); + assert!( + ci.status.success(), + "fresh-checkout frozen `bundle install` must succeed.\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&ci.stdout), + String::from_utf8_lossy(&ci.stderr), + ); + assert_eq!( + std::fs::read(fresh.join("Gemfile.lock")).unwrap(), + lock_wired, + "frozen install must leave the committed Gemfile.lock byte-identical" + ); + let probe = bundle( + &fresh, + &[ + "exec", + "ruby", + "-e", + "require \"rack\"\n\ + abort \"probe constant missing after require\" unless defined?(Rack::SOCKET_PATCH_VENDOR_E2E)\n\ + puts $LOADED_FEATURES.grep(%r{/rack\\.rb\\z})", + ], + false, + ); + assert!( + probe.status.success(), + "bundle exec runtime probe failed.\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&probe.stdout), + String::from_utf8_lossy(&probe.stderr), + ); + assert!( + String::from_utf8_lossy(&probe.stdout).contains(&format!("{copy_rel}/lib/rack.rb")), + "rack must be loaded from the vendored path:\n{}", + String::from_utf8_lossy(&probe.stdout) + ); + + // Idempotent re-run. + let (code, stdout, stderr) = run_socket(&proj, &vendor_args); + assert_eq!( + code, 0, + "re-vendor failed.\nstdout:\n{stdout}\nstderr:\n{stderr}" + ); + assert_eq!(std::fs::read(&lock_path).unwrap(), lock_wired); + + // Revert: the spec goes back into the second GEM section. + let (code, stdout, stderr) = run_socket( + &proj, + &[ + "vendor", + "--revert", + "--json", + "--cwd", + proj.to_str().unwrap(), + ], + ); + assert_eq!( + code, 0, + "revert failed.\nstdout:\n{stdout}\nstderr:\n{stderr}" + ); + let renv = parse_envelope(&stdout); + assert_eq!(renv["summary"]["removed"], 1, "one entry reverted: {renv}"); + assert_eq!( + std::fs::read(&gemfile_path).unwrap(), + gemfile_before, + "revert must restore the Gemfile byte for byte" + ); + assert_eq!( + String::from_utf8(std::fs::read(&lock_path).unwrap()).unwrap(), + lock_text, + "revert must restore Gemfile.lock byte for byte" + ); + assert!(!proj.join(".socket/vendor").exists()); +} diff --git a/crates/socket-patch-core/src/vendor/gem.rs b/crates/socket-patch-core/src/vendor/gem.rs index 99fc41219..b1b39a47d 100644 --- a/crates/socket-patch-core/src/vendor/gem.rs +++ b/crates/socket-patch-core/src/vendor/gem.rs @@ -92,10 +92,13 @@ const GEMFILE_LOCK: &str = "Gemfile.lock"; /// /// `gemfile_lock_spec`: `original` and `new` are arrays of verbatim lock /// lines. In `original`, lines indented 4+ spaces are the gem's GEM spec -/// block and the single 2-space line (if any) is the pre-vendor DEPENDENCIES -/// entry — its absence means the gem was transitive and revert deletes the -/// added entry. In `new`, the last element is the DEPENDENCIES entry we wrote -/// and the rest is the emitted PATH section. +/// block and the single 2-space line (if any) other than a ` remote: ` line +/// is the pre-vendor DEPENDENCIES entry — its absence means the gem was +/// transitive and revert deletes the added entry. A ` remote: ` line names +/// the GEM section the block came from; it is recorded only when that is not +/// the lock's first GEM section (#779), and revert restores into it. In +/// `new`, the last element is the DEPENDENCIES entry we wrote and the rest +/// is the emitted PATH section. /// /// `gemfile_lock_checksum`: `original`/`new` are the verbatim CHECKSUMS line /// strings (the registry ` () sha256=` form vs the bare @@ -728,32 +731,16 @@ pub async fn vendor_gem<'a>( // fragments: record `original: None` — the true originals live in the // ledger entry being replaced, which the caller carries forward by // wiring identity (`persist_vendor_entry`). - let lock_original = if lock_edit.rewired_ours { - None - } else { - let mut original_lines: Vec = lock_edit - .removed_spec_block - .iter() - .map(|l| Value::String(l.clone())) - .collect(); - if let Some(dep) = &lock_edit.old_dep_line { - original_lines.push(Value::String(dep.clone())); - } - Some(Value::Array(original_lines)) - }; - let mut new_lines: Vec = lock_edit - .path_section - .iter() - .map(|l| Value::String(l.clone())) - .collect(); - new_lines.push(Value::String(lock_edit.new_dep_line.clone())); + let (original_lines, new_lines) = lock_record_lines(&lock_edit); + let to_array = + |lines: Vec| Value::Array(lines.into_iter().map(Value::String).collect()); let lock_record = WiringRecord { file: GEMFILE_LOCK.to_string(), kind: LOCK_WIRING_KIND.to_string(), action: WiringAction::Rewritten, key: Some(name.to_string()), - original: lock_original, - new: Some(Value::Array(new_lines)), + original: (!lock_edit.rewired_ours).then(|| to_array(original_lines)), + new: Some(to_array(new_lines)), }; let mut wiring = vec![gemfile_record, lock_record]; // The CHECKSUMS rewrite (when the lock had a registry entry for the gem) @@ -1654,6 +1641,21 @@ struct LockEdit { /// must ride again or the first run's registry `sha256=` restore line /// drops out of the ledger with the entry being replaced. checksum_bare: Option, + /// The ` remote: ` line of the GEM section the spec block was lifted + /// from, when that is not the lock's first GEM section (#779). Revert + /// puts the block back into that section. + source_remote: Option, +} + +/// The `gemfile_lock_spec` record's `(original, new)` line arrays for +/// `edit` (see [`LOCK_WIRING_KIND`] for the positional grammar). +fn lock_record_lines(edit: &LockEdit) -> (Vec, Vec) { + let mut original = edit.removed_spec_block.clone(); + original.extend(edit.source_remote.iter().cloned()); + original.extend(edit.old_dep_line.iter().cloned()); + let mut new = edit.path_section.clone(); + new.push(edit.new_dep_line.clone()); + (original, new) } /// Produce the pair-edited lock text (see the module doc for the canonical @@ -1665,10 +1667,16 @@ fn edit_lock(text: &str, name: &str, version: &str, rel: &str) -> Result Result = None; let mut rewired_ours = false; - let removed_spec_block: Vec = match (gem_start..gem_end).find(|&i| lines[i] == target) { - Some(block_start) => { + let removed_spec_block: Vec = match hit { + Some((k, gem_start, gem_end, block_start)) => { + if k > 0 { + // Revert must find this section again; without a remote + // line there is nothing to find it by. + source_remote = Some( + lines[gem_start..gem_end] + .iter() + .find(|l| l.starts_with(" remote: ")) + .cloned() + .ok_or_else(|| { + format!("Gemfile.lock GEM section holding `{name} ({version})` has no remote: line") + })?, + ); + } let mut block_end = block_start + 1; while block_end < gem_end && lines[block_end].starts_with(" ") { block_end += 1; @@ -1884,6 +1925,7 @@ fn edit_lock(text: &str, name: &str, version: &str, rel: &str) -> Result usize { end } +/// `[start, end)` of every GEM section, in lock order. +fn gem_section_spans(lines: &[String]) -> Vec<(usize, usize)> { + let mut spans = Vec::new(); + let mut i = 0; + while i < lines.len() { + if lines[i].as_str() == "GEM" { + let end = section_end(lines, i); + spans.push((i, end)); + i = end; + } else { + i += 1; + } + } + spans +} + +/// The GEM section a `gemfile_lock_spec` record's spec block belongs in: +/// the one carrying the recorded `remote:` line, or the first GEM section +/// when none was recorded (the block came from it). +fn record_gem_section(lines: &[String], source_remote: Option<&str>) -> Option<(usize, usize)> { + let spans = gem_section_spans(lines); + match source_remote { + None => spans.first().copied(), + Some(remote) => spans + .into_iter() + .find(|&(gs, ge)| lines[gs..ge].iter().any(|l| l == remote)), + } +} + +/// The recorded source-section `remote:` line in a `gemfile_lock_spec` +/// record's `original` (present only when it was not the first GEM section). +fn record_source_remote(original_lines: &[String]) -> Option<&str> { + original_lines + .iter() + .map(String::as_str) + .find(|l| l.starts_with(" remote: ")) +} + /// Bundler's lock-sort identifier for a path source — `source at ``` /// (`Source::Path#to_s`, aliased as `identifier`); sections order by a /// byte-wise comparison of these, which Rust's `str` ordering matches. @@ -2142,7 +2222,7 @@ fn lock_record_converged(text: &str, original_lines: &[String], new_lines: &[Str if find_path_section(&lines, remote_line).is_some() { return false; } - let Some((gs, ge)) = section_span(&lines, "GEM") else { + let Some((gs, ge)) = record_gem_section(&lines, record_source_remote(original_lines)) else { return false; }; let gem = &lines[gs..ge]; @@ -2233,9 +2313,10 @@ fn revert_lock_text(text: &str, original_lines: &[String], new_lines: &[String]) .iter() .filter(|l| l.starts_with(" ")) .collect(); + let source_remote = record_source_remote(original_lines); let old_dep_line = original_lines .iter() - .find(|l| l.starts_with(" ") && !l[2..].starts_with(' ')); + .find(|l| l.starts_with(" ") && !l[2..].starts_with(' ') && !l.starts_with(" remote: ")); let our_name = spec_entry_name(spec_block.first()?)?.to_string(); let mut lines: Vec = text.split('\n').map(str::to_string).collect(); @@ -2246,7 +2327,7 @@ fn revert_lock_text(text: &str, original_lines: &[String], new_lines: &[String]) return None; } { - let (gs, ge) = section_span(&lines, "GEM")?; + let (gs, ge) = record_gem_section(&lines, source_remote)?; (gs..ge).find(|&i| lines[i] == " specs:")?; } @@ -2254,8 +2335,9 @@ fn revert_lock_text(text: &str, original_lines: &[String], new_lines: &[String]) lines.drain(path_start..path_end); // 2. Spec block back into GEM/specs, sorted by entry name (bundler keeps - // specs alphabetized; the block came out of a sorted list). - let (gs, ge) = section_span(&lines, "GEM")?; + // specs alphabetized; the block came out of a sorted list), in the GEM + // section it was lifted from. + let (gs, ge) = record_gem_section(&lines, source_remote)?; let specs_idx = (gs..ge).find(|&i| lines[i] == " specs:")?; let mut insert_at = specs_idx + 1; let mut i = specs_idx + 1; @@ -7573,4 +7655,198 @@ mod tests { assert_eq!(code, "vendor_prebuilt_required"); assert!(!root.join(".socket").exists(), "nothing written"); } + + // ── #779: a gem outside the lock's first GEM section ────────────────── + + /// Bundler 2+ writes one GEM section per rubygems source, sorted by + /// remote, so a private source can put rubygems.org second. + const GEMFILE_TWO_SOURCES: &str = "source \"https://rubygems.org\"\n\ngem \"puma\"\ngem \"rack\", \"~> 3.1\"\n\nsource \"https://gems.example.com\" do\n gem \"aaa-internal\"\nend\n"; + const LOCK_TWO_SOURCES: &str = "GEM\n remote: https://gems.example.com/\n specs:\n aaa-internal (1.0.0)\n\nGEM\n remote: https://rubygems.org/\n specs:\n puma (6.4.2)\n nio4r (~> 2.0)\n rack (3.2.6)\n base64 (>= 0.1.0)\n\nPLATFORMS\n ruby\n\nDEPENDENCIES\n aaa-internal!\n puma\n rack (~> 3.1)\n\nBUNDLED WITH\n 2.5.22\n"; + + /// Bundler's own re-lock of [`LOCK_TWO_SOURCES`] with rack path-sourced + /// (shape verified against bundler 4.0.17 `bundle lock --local`). + fn expected_lock_two_sources() -> String { + format!( + "PATH\n remote: {rel}\n specs:\n rack (3.2.6)\n base64 (>= 0.1.0)\n\nGEM\n remote: https://gems.example.com/\n specs:\n aaa-internal (1.0.0)\n\nGEM\n remote: https://rubygems.org/\n specs:\n puma (6.4.2)\n nio4r (~> 2.0)\n\nPLATFORMS\n ruby\n\nDEPENDENCIES\n aaa-internal!\n puma\n rack (= 3.2.6)!\n\nBUNDLED WITH\n 2.5.22\n", + rel = copy_rel() + ) + } + + /// #779: the spec sits in the second GEM section. It used to fail with + /// "GEM specs has no entry" although the lock lists it. + #[test] + fn edit_lock_lifts_spec_from_second_gem_section() { + let edit = edit_lock(LOCK_TWO_SOURCES, "rack", "3.2.6", ©_rel()).unwrap(); + assert_eq!(edit.text, expected_lock_two_sources()); + assert!(!edit.rewired_ours); + assert_eq!( + edit.removed_spec_block, + vec![" rack (3.2.6)", " base64 (>= 0.1.0)"] + ); + assert_eq!( + edit.source_remote.as_deref(), + Some(" remote: https://rubygems.org/") + ); + } + + /// The first-section case keeps its ledger shape: no source remote is + /// recorded, so ledgers written before #779 and after it agree. + #[test] + fn edit_lock_first_gem_section_records_no_source_remote() { + let edit = edit_lock(LOCK_DIRECT, "rack", "3.2.6", ©_rel()).unwrap(); + assert_eq!(edit.source_remote, None); + let (original, _new) = lock_record_lines(&edit); + assert!(original.iter().all(|l| !l.starts_with(" remote: "))); + } + + /// SECURITY/fail-closed: the same `name (version)` in two GEM sections + /// means the lock disagrees with bundler's one-spec-per-gem model. + /// Lifting either copy would be a guess. + #[test] + fn edit_lock_spec_in_two_gem_sections_fails_closed() { + let lock = LOCK_TWO_SOURCES.replace( + " aaa-internal (1.0.0)\n", + " aaa-internal (1.0.0)\n rack (3.2.6)\n", + ); + let err = edit_lock(&lock, "rack", "3.2.6", ©_rel()) + .err() + .expect("a spec listed in two GEM sections must fail closed"); + assert!(err.contains("more than one GEM section"), "{err}"); + } + + /// SECURITY/fail-closed: a platform-suffixed sibling in ANOTHER GEM + /// section is still a lock that disagrees with the install. + #[test] + fn edit_lock_platform_sibling_in_other_gem_section_fails_closed() { + let lock = LOCK_TWO_SOURCES.replace( + " aaa-internal (1.0.0)\n", + " aaa-internal (1.0.0)\n rack (3.2.6-x86_64-linux)\n", + ); + let err = edit_lock(&lock, "rack", "3.2.6", ©_rel()) + .err() + .expect("a platform sibling in any GEM section must fail closed"); + assert!(err.contains("platform-suffixed"), "{err}"); + } + + /// #779 end to end: vendor a gem from the second GEM section, then + /// revert. The lock is Bundler's canonical form while vendored, and + /// revert puts the spec back into the section it came from, restoring + /// the original bytes exactly. + #[tokio::test] + async fn second_gem_section_vendor_and_revert_round_trip() { + let (_tmp, root, installed, blobs, record) = + fixture(GEMFILE_TWO_SOURCES, LOCK_TWO_SOURCES).await; + + let (result, entry, _w) = + unwrap_done(run_vendor(&root, &blobs, &installed, &record, false).await); + assert!(result.success, "{:?}", result.error); + let entry = entry.unwrap(); + assert_eq!( + tokio::fs::read_to_string(root.join(GEMFILE_LOCK)) + .await + .unwrap(), + expected_lock_two_sources() + ); + + // Idempotent re-run: nothing to change. + let lock_before = tokio::fs::read(root.join(GEMFILE_LOCK)).await.unwrap(); + let (r2, _e2, _) = unwrap_done(run_vendor(&root, &blobs, &installed, &record, false).await); + assert!(r2.success, "{:?}", r2.error); + assert_eq!( + tokio::fs::read(root.join(GEMFILE_LOCK)).await.unwrap(), + lock_before + ); + + let outcome = revert_gem(&entry, &root, false).await; + assert!(outcome.success, "{:?}", outcome.error); + assert!( + !outcome + .warnings + .iter() + .any(|w| w.code == "vendor_lock_entry_drifted"), + "clean revert must not report drift: {:?}", + outcome.warnings + ); + assert_eq!( + tokio::fs::read_to_string(root.join(GEMFILE)).await.unwrap(), + GEMFILE_TWO_SOURCES + ); + assert_eq!( + tokio::fs::read_to_string(root.join(GEMFILE_LOCK)) + .await + .unwrap(), + LOCK_TWO_SOURCES, + "revert must restore the spec into the second GEM section" + ); + } + + /// Re-vendor to a newer patch uuid (the spec now lives in our PATH + /// section) keeps the source section recorded by the first run, so a + /// later revert still targets the second GEM section. + #[test] + fn revert_lock_text_targets_recorded_gem_section() { + let edit = edit_lock(LOCK_TWO_SOURCES, "rack", "3.2.6", ©_rel()).unwrap(); + let (original, new) = lock_record_lines(&edit); + assert_eq!( + revert_lock_text(&edit.text, &original, &new).as_deref(), + Some(LOCK_TWO_SOURCES) + ); + // A regenerated lock that already put rack back into its section is + // converged, not drifted. + assert!(lock_record_converged(LOCK_TWO_SOURCES, &original, &new)); + // The recorded section vanished (the source was dropped): drift, + // never a guess at another section. + let gone = edit.text.replace( + " remote: https://rubygems.org/\n", + " remote: https://mirror.example/\n", + ); + assert_eq!(revert_lock_text(&gone, &original, &new), None); + assert!(!lock_record_converged( + &LOCK_TWO_SOURCES.replace("https://rubygems.org/", "https://mirror.example/"), + &original, + &new + )); + } + + /// #779 re-vendor: a superseding patch uuid lifts the spec out of our + /// own PATH section, and the carried-forward original still names the + /// second GEM section, so revert restores the pre-vendor bytes. + #[tokio::test] + async fn second_gem_section_revendor_then_revert_restores_original() { + let (_tmp, root, installed, blobs, record) = + fixture(GEMFILE_TWO_SOURCES, LOCK_TWO_SOURCES).await; + let (r1, e1, _) = unwrap_done(run_vendor(&root, &blobs, &installed, &record, false).await); + assert!(r1.success, "{:?}", r1.error); + let entry1 = e1.unwrap(); + + let mut record2 = record.clone(); + record2.uuid = "0e1f2a3b-4c5d-4e6f-8a7b-9c0d1e2f3a4b".to_string(); + let (r2, e2, _) = unwrap_done(run_vendor(&root, &blobs, &installed, &record2, false).await); + assert!(r2.success, "re-vendor must succeed: {:?}", r2.error); + assert_eq!( + tokio::fs::read_to_string(root.join(GEMFILE_LOCK)) + .await + .unwrap(), + expected_lock_two_sources().replace(UUID, &record2.uuid) + ); + + let mut entry2 = e2.unwrap(); + carry_forward_originals(&entry1, &mut entry2); + let outcome = revert_gem(&entry2, &root, false).await; + assert!(outcome.success, "{:?}", outcome.error); + assert!( + !outcome + .warnings + .iter() + .any(|w| w.code == "vendor_lock_entry_drifted"), + "{:?}", + outcome.warnings + ); + assert_eq!( + tokio::fs::read_to_string(root.join(GEMFILE_LOCK)) + .await + .unwrap(), + LOCK_TWO_SOURCES + ); + } } From 30d3a70add5e8c79428b41f0ca22051e63572a87 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 20:54:27 +0000 Subject: [PATCH 3/3] Gate the two-GEM-section premise on Bundler 2.2 Bundler 2.1 and older merge every rubygems remote into one GEM section, so the e2e's "rack is in the second section" premise only holds from Bundler 2.2 on. The vendor and revert round trip is still checked on the merged layout. Refs #779 Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/tests/e2e_vendor_gem_build.rs | 9 +++++---- crates/socket-patch-core/src/vendor/gem.rs | 4 ++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/crates/socket-patch-cli/tests/e2e_vendor_gem_build.rs b/crates/socket-patch-cli/tests/e2e_vendor_gem_build.rs index 2376ba61c..325601a81 100644 --- a/crates/socket-patch-cli/tests/e2e_vendor_gem_build.rs +++ b/crates/socket-patch-cli/tests/e2e_vendor_gem_build.rs @@ -1675,7 +1675,7 @@ File.binwrite("quick/Marshal.4.8/#{spec.full_name}.gemspec.rz", Zlib::Deflate.de ); } -/// #779: a Gemfile with a second rubygems source. Bundler 2+ writes one +/// #779: a Gemfile with a second rubygems source. Bundler 2.2+ writes one /// GEM section per source, sorted by remote, so the `file://` source comes /// first and rack sits in the second GEM section. Vendor used to fail with /// "GEM specs has no entry". It must wire rack so a fresh frozen install @@ -1722,9 +1722,10 @@ fn gem_vendor_second_gem_section_fresh_checkout_and_revert() { let gemfile_before = std::fs::read(&gemfile_path).unwrap(); let lock_text = String::from_utf8_lossy(&lock_before).into_owned(); let version = locked_gem_version(&lock_text, DEP).expect("resolved rack version"); - if bundler.at_least(2, 0) { - // The premise: rack is NOT in the first GEM section. (Bundler 1.x - // merges every rubygems remote into one GEM section.) + if bundler.at_least(2, 2) { + // The premise: rack is NOT in the first GEM section. (Bundler + // <= 2.1 merges every rubygems remote into one GEM section; the + // round trip below must hold on that layout too.) let sections: Vec<&str> = lock_text .split("\n\n") .filter(|s| s.starts_with("GEM\n")) diff --git a/crates/socket-patch-core/src/vendor/gem.rs b/crates/socket-patch-core/src/vendor/gem.rs index b1b39a47d..39ac9fb11 100644 --- a/crates/socket-patch-core/src/vendor/gem.rs +++ b/crates/socket-patch-core/src/vendor/gem.rs @@ -1667,7 +1667,7 @@ fn edit_lock(text: &str, name: &str, version: &str, rel: &str) -> Result 3.1\"\n\nsource \"https://gems.example.com\" do\n gem \"aaa-internal\"\nend\n"; const LOCK_TWO_SOURCES: &str = "GEM\n remote: https://gems.example.com/\n specs:\n aaa-internal (1.0.0)\n\nGEM\n remote: https://rubygems.org/\n specs:\n puma (6.4.2)\n nio4r (~> 2.0)\n rack (3.2.6)\n base64 (>= 0.1.0)\n\nPLATFORMS\n ruby\n\nDEPENDENCIES\n aaa-internal!\n puma\n rack (~> 3.1)\n\nBUNDLED WITH\n 2.5.22\n";