diff --git a/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs b/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs index be684867e..07fe7fe7a 100644 --- a/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs +++ b/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs @@ -1912,6 +1912,294 @@ async fn gem_get_uuid_hosted_fresh_checkout_bundle_install() { manifestless_vex_matrix(&fx, &fresh).await; } +/// One patched gem a later patch generation serves: its uuid, the patched +/// `.gem`, and the lib file's before/after bytes for the view record. +struct GenerationGem { + name: &'static str, + uuid: &'static str, + deps: Vec, + lib_file: &'static str, + orig: String, + patched: String, + gem: Vec, +} + +/// Layer a new patch generation over the fixture's API mocks (wiremock +/// serves the lowest `priority` first): the batch, by-package, reference and +/// view routes answer with `gems`, and each uuid gets its own patch-registry +/// compact index under the fixture's grant token. +async fn mount_patch_generation(server: &MockServer, priority: u8, gems: &[GenerationGem]) { + let purl = |g: &GenerationGem| format!("pkg:gem/{}@{DEP_VERSION}", g.name); + for g in gems { + mount_compact_index( + server, + &format!("/patch-registry/gem/{TOKEN}/{}", g.uuid), + &[IndexGem { + name: g.name, + version: DEP_VERSION, + deps: g.deps.clone(), + gem: g.gem.clone(), + }], + ) + .await; + Mock::given(method("GET")) + .and(path(format!("/v0/orgs/{ORG}/patches/view/{}", g.uuid))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "uuid": g.uuid, + "purl": purl(g), + "publishedAt": "2026-02-01T00:00:00Z", + "files": { + format!("lib/{}", g.lib_file): { + "beforeHash": compute_git_sha256_from_bytes(g.orig.as_bytes()), + "afterHash": compute_git_sha256_from_bytes(g.patched.as_bytes()), + } + }, + "vulnerabilities": { + GHSA: { + "cves": ["CVE-2026-3333"], + "summary": "gem redirect capstone vuln", + "severity": "high", + "description": "d" + } + }, + "description": "x", "license": "MIT", "tier": "free" + }))) + .with_priority(priority) + .mount(server) + .await; + Mock::given(method("GET")) + .and(path_regex(format!( + "^/v0/orgs/{ORG}/patches/by-package/.*{}.*$", + g.name + ))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "patches": [{ + "uuid": g.uuid, "purl": purl(g), + "publishedAt": "2026-02-01T00:00:00Z", + "description": "x", "license": "MIT", "tier": "free", + "vulnerabilities": {} + }], + "canAccessPaidPatches": false, + }))) + .with_priority(priority) + .mount(server) + .await; + } + let packages: Vec = gems + .iter() + .map(|g| { + serde_json::json!({ + "purl": purl(g), + "patches": [{ + "uuid": g.uuid, "purl": purl(g), "tier": "free", + "cveIds": [], "ghsaIds": [], "severity": "high", + "title": "gem redirect capstone fixture" + }] + }) + }) + .collect(); + Mock::given(method("POST")) + .and(path(format!("/v0/orgs/{ORG}/patches/batch"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "packages": packages, + "canAccessPaidPatches": false, + }))) + .with_priority(priority) + .mount(server) + .await; + let results: serde_json::Map = gems + .iter() + .map(|g| { + let sha = sha256_hex(&g.gem); + let hosted_url = format!( + "{}/patch/gem/{}/{DEP_VERSION}/{TOKEN}/{}/{}-{DEP_VERSION}.gem", + server.uri(), + g.name, + g.uuid, + g.name + ); + let index_url = format!("{}/patch-registry/gem/{TOKEN}/{}/", server.uri(), g.uuid); + ( + g.uuid.to_string(), + serde_json::json!({ + "status": "granted", + "url": hosted_url, + "purl": purl(g), + "artifacts": [{ + "kind": "tarball", + "url": hosted_url, + "integrity": { "sha256": sha } + }], + "registryOverride": { + "kind": "rubygems-compact-index", + "indexUrl": index_url, + "identifiers": { + "name": g.name, + "version": DEP_VERSION, + "gemChecksumSha256": sha, + } + } + }), + ) + }) + .collect(); + Mock::given(method("POST")) + .and(path(format!("/v0/orgs/{ORG}/patches/package"))) + .respond_with( + ResponseTemplate::new(200).set_body_json(serde_json::json!({ "results": results })), + ) + .with_priority(priority) + .mount(server) + .await; +} + +/// The `remote:` URLs of a lock's `GEM` sections, in file order. +fn gem_remotes(lock: &str) -> Vec<&str> { + let mut out = Vec::new(); + let mut in_gem = false; + for line in lock.lines() { + if !line.starts_with(' ') && !line.is_empty() { + in_gem = line == "GEM"; + } else if let Some(url) = line.strip_prefix(" remote: ").filter(|_| in_gem) { + out.push(url); + } + } + out +} + +/// #1186: two gems hosted in their own patch-registry `GEM` sections, then a +/// superseding patch for one of them whose new uuid sorts AFTER its +/// sibling. The re-scan must move the refreshed section to where bundler +/// writes it (sections sorted by remote URL): a converged CHECKSUMS lock +/// that `bundle lock` leaves byte-identical and a cold frozen install +/// accepts. An in-place refresh left the sections out of order, and every +/// frozen install on bundler 4.0.19+ exited 16 ("Your lockfile needs to be +/// updated, but it can't be because frozen mode is set"). +#[tokio::test(flavor = "multi_thread")] +#[ignore = "host capstone: shells out to a real ruby/gem/bundler (>= 2.6 for the CHECKSUMS lock); \ + the unpinned `test` job skips it, an e2e job with a pinned toolchain runs it via --ignored"] +async fn gem_hosted_superseding_patch_keeps_gem_sections_in_bundler_order() { + let Some(fx) = redirect_scanned_project( + "supersede-section-order", + Spelling::Gemfile, + true, + true, + None, + Driver::ScanVex, + ) + .await + else { + return; + }; + const VULN_GEN2: &str = "10000000-1a2b-4a1b-8c2d-3e4f5a6b7c8d"; + const TINY_GEN2: &str = "80000000-1a2b-4a1b-8c2d-3e4f5a6b7c8d"; + const VULN_GEN3: &str = "9a9a9a9a-1a2b-4a1b-8c2d-3e4f5a6b7c8d"; + let stage = fx.tmp.path().join("generation-stage"); + let vuln = |uuid: &'static str, lib: String| { + let gem = build_gem( + &stage.join(uuid), + DEP, + DEP_VERSION, + "vuln_gem.rb", + &lib, + &[TRANSITIVE], + ); + GenerationGem { + name: DEP, + uuid, + deps: vec![format!("{TRANSITIVE}:>= 0")], + lib_file: "vuln_gem.rb", + orig: orig_lib(), + patched: lib, + gem, + } + }; + let tiny_patched = TINY_LIB.replace("tiny-ok", "tiny-patched"); + let tiny = GenerationGem { + name: TRANSITIVE, + uuid: TINY_GEN2, + deps: vec![], + lib_file: "tiny_dep.rb", + orig: TINY_LIB.to_string(), + patched: tiny_patched.clone(), + gem: build_gem( + &stage.join(TINY_GEN2), + TRANSITIVE, + "1.0.0", + "tiny_dep.rb", + &tiny_patched, + &[], + ), + }; + let server = fx._server.uri(); + let registry = |uuid: &str| format!("{server}/patch-registry/gem/{TOKEN}/{uuid}/"); + let upstream = format!("{server}/upstream/"); + let lock_path = fx.proj.join(fx.lock_name); + let scan = |what: &str| { + let (code, stdout, stderr) = run_hosted_scan(&fx.proj, &server); + assert_eq!( + code, 0, + "{what} failed.\nstdout:\n{stdout}\nstderr:\n{stderr}" + ); + std::fs::read_to_string(&lock_path).unwrap() + }; + + // Generation 2: both gems hosted, each in its own section, inserted + // sorted (the path that already worked). + let tiny_gem = tiny.gem.clone(); + mount_patch_generation(&fx._server, 2, &[vuln(VULN_GEN2, patched_lib()), tiny]).await; + let lock = scan("generation-2 scan"); + assert_eq!( + gem_remotes(&lock), + [registry(VULN_GEN2), registry(TINY_GEN2), upstream.clone()], + "generation 2 sections:\n{lock}" + ); + + // Generation 3: a superseding patch for vuln-gem only (same version, + // new bytes) whose uuid sorts after tiny-dep's section. + let gen3_lib = patched_lib().replace("PATCHED", "PATCHED-GEN3"); + assert_ne!(gen3_lib, patched_lib(), "generation 3 must change bytes"); + let tiny = GenerationGem { + name: TRANSITIVE, + uuid: TINY_GEN2, + deps: vec![], + lib_file: "tiny_dep.rb", + orig: TINY_LIB.to_string(), + patched: tiny_patched, + gem: tiny_gem, + }; + mount_patch_generation(&fx._server, 1, &[vuln(VULN_GEN3, gen3_lib), tiny]).await; + let lock = scan("generation-3 scan"); + assert_eq!( + gem_remotes(&lock), + [registry(TINY_GEN2), registry(VULN_GEN3), upstream.clone()], + "the superseded section must move to bundler's sorted position:\n{lock}" + ); + + // Bundler agrees: `bundle lock` re-renders the committed lock + // byte-identically, and a cold frozen install accepts it. + let relock = stage_fresh_checkout(&fx, "fresh-relock"); + let out = bundle(&relock, &["lock"]); + assert!( + out.status.success(), + "bundle lock failed:\n{}", + String::from_utf8_lossy(&out.stderr) + ); + assert_eq!( + std::fs::read_to_string(relock.join(fx.lock_name)).unwrap(), + lock, + "bundle lock must leave the converged lock byte-identical" + ); + let fresh = stage_fresh_checkout(&fx, "fresh-frozen"); + let install = bundle_env(&fresh, &["install"], &[("BUNDLE_FROZEN", "true")]); + let stderr = String::from_utf8_lossy(&install.stderr); + assert!( + install.status.success() && !stderr.contains("Cannot write a changed lockfile"), + "cold frozen install must accept the lock unchanged.\nstdout:\n{}\nstderr:\n{stderr}", + String::from_utf8_lossy(&install.stdout), + ); +} + /// 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 diff --git a/crates/socket-patch-core/src/formats/gem/hosted.rs b/crates/socket-patch-core/src/formats/gem/hosted.rs index 26e4808c0..db978a79a 100644 --- a/crates/socket-patch-core/src/formats/gem/hosted.rs +++ b/crates/socket-patch-core/src/formats/gem/hosted.rs @@ -32,6 +32,62 @@ struct GemLockSection { end: usize, } +impl GemLockSection { + /// Bundler's source identifier for the section (its remote URLs) — the + /// key `SourceList#lock_rubygems_sources` sorts `GEM` sections by. + fn identifier(&self) -> String { + self.remotes + .iter() + .map(|(_, url)| url.as_str()) + .collect::>() + .join(", ") + } +} + +/// Move `GEM` section `sec_idx` (whose remote now reads `index_url`) to +/// where bundler writes it: before the first other section whose +/// identifier sorts after `index_url`, else after the last one. The section +/// moves whole, its lines keeping their own endings. Returns false (nothing +/// touched) when it already sits there. +fn place_gem_section_sorted( + lines: &mut Vec, + sections: &[GemLockSection], + sec_idx: usize, + index_url: &str, + eol: &str, +) -> bool { + let others = || (0..sections.len()).filter(|&k| k != sec_idx); + let before = others().find(|&k| sections[k].identifier().as_str() > index_url); + if before == others().find(|&k| k > sec_idx) { + return false; + } + let (start, end) = (sections[sec_idx].start, sections[sec_idx].end); + let mut block: Vec = lines.drain(start..end).collect(); + let n = block.len(); + // The moved section needs its own blank separator (and a final newline + // if it was the file's last line). + if let Some(last) = block.last_mut() { + if !last.ends_with('\n') { + last.push_str(eol); + } + if !gem_lock_line_content(last).is_empty() { + block.push(eol.to_string()); + } + } + let shifted = |at: usize| if at > start { at - n } else { at }; + let at = match before { + Some(k) => shifted(sections[k].start), + None => { + let last = others() + .next_back() + .expect("a section sorts before this one"); + shifted(sections[last].end) + } + }; + lines.splice(at..at, block); + true +} + /// Converge the lock's source attribution for one redirected dep so the /// Gemfile + lock pair is what bundler itself would write after an install /// from the redirected Gemfile (verified frozen-installable on bundler 4): @@ -46,8 +102,10 @@ struct GemLockSection { /// /// Idempotent and rotation-aware: a section whose remote matches the /// token-wildcard pattern is recognized as ours (never duplicated) and its -/// remote is refreshed in place under a rotated grant -/// (`redirect_gemfile_lock_source_url`, mirroring the Gemfile refresh). +/// remote is refreshed under a rotated grant or superseding patch +/// (`redirect_gemfile_lock_source_url`, mirroring the Gemfile refresh); the +/// section then moves to bundler's sorted position if the new URL sorts +/// elsewhere (`redirect_gemfile_lock_section_order`). /// /// Returns true when the lock ends converged (already, or via edits recorded /// into `result`); false when the dep cannot be attributed safely — spec @@ -181,7 +239,8 @@ pub(crate) fn converge_gem_lock_source( } if socket_remote_re.is_match(&remote_url) { - // Already ours. Rotated grant: refresh the remote in place. + // Already ours. Rotated grant or superseding patch: refresh the + // remote. if remote_url != index_url { let ending = lines[remote_idx][gem_lock_line_content(&lines[remote_idx]).len()..].to_string(); @@ -196,6 +255,22 @@ pub(crate) fn converge_gem_lock_source( }); changed = true; } + // The refreshed URL (a superseding patch uuid, a rotated grant) can + // sort past a sibling `GEM` section, and bundler writes the sections + // sorted by identifier — the same rule as the fresh insert below. + // Re-place the whole section; a lock an earlier run left out of + // order is healed the same way. + if place_gem_section_sorted(&mut lines, §ions, sec_idx, index_url, eol) { + result.edits.push(FileEdit { + path: lock_name.into(), + kind: "redirect_gemfile_lock_section_order".into(), + action: "moved".into(), + key: Some(dep.name.clone()), + original: None, + new: Some(Value::String(index_url.to_string())), + }); + changed = true; + } } else { // Move the spec (+ sublines) into a patch-registry section of its // own, inserted where bundler itself writes it: bundler emits the @@ -229,16 +304,8 @@ pub(crate) fn converge_gem_lock_source( std::cmp::Ordering::Greater => (s.start - n, s.end - n), } }; - let identifier = |k: usize| -> String { - sections[k] - .remotes - .iter() - .map(|(_, url)| url.as_str()) - .collect::>() - .join(", ") - }; let insert_at = (0..sections.len()) - .find(|&k| identifier(k).as_str() > index_url) + .find(|&k| sections[k].identifier().as_str() > index_url) .map(|k| bounds(k).0) .unwrap_or_else(|| bounds(sections.len() - 1).1); let mut block: Vec = Vec::with_capacity(moved.len() + 4); diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index cf21a0c54..13f11c3c6 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -15601,6 +15601,162 @@ mod tests { ); } + /// A superseding patch (new uuid) refreshes an existing patch-registry + /// section's remote — and must then move that section to where bundler + /// writes it. Bundler sorts the rubygems `GEM` sections by identifier, + /// so with a sibling patch-registry section in between, an in-place + /// refresh leaves the lock out of order and every frozen install on + /// bundler 4.0.19+ exits 16 (#1186). + #[test] + fn gem_superseded_remote_moves_section_to_sorted_position() { + fn ov(name: &str, uuid: &str) -> DepOverride { + let mut o = gem_override(name, "1.0.0"); + o.patch_uuid = uuid.into(); + if let Some(r) = o.registry_override.as_mut() { + r.index_url = format!("https://patch.test/gem/tok/{uuid}/"); + } + o + } + let gemfile = "source \"https://rubygems.org\"\n\ngem \"vuln-gem\", \"1.0.0\"\n"; + for eol in ["\n", "\r\n"] { + let mut files = BTreeMap::new(); + files.insert("Gemfile".to_string(), gemfile.replace('\n', eol)); + files.insert( + "Gemfile.lock".to_string(), + format!( + "GEM\n remote: https://rubygems.org/\n specs:\n tiny-dep (1.0.0)\n \ + vuln-gem (1.0.0)\n tiny-dep\n\nPLATFORMS\n ruby\n\nDEPENDENCIES\n \ + vuln-gem (= 1.0.0)\n\nCHECKSUMS\n tiny-dep (1.0.0) sha256={a}\n \ + vuln-gem (1.0.0) sha256={a}\n\nBUNDLED WITH\n 4.0.22\n", + a = "2".repeat(64) + ) + .replace('\n', eol), + ); + let gen1 = [ov("vuln-gem", "10000000"), ov("tiny-dep", "80000000")]; + let first = rewrite_registry_redirect(&files, &gen1); + for (name, content) in first.files { + files.insert(name, content); + } + let remotes = |lock: &str| -> Vec { + lock.lines() + .filter_map(|l| l.trim_end_matches('\r').strip_prefix(" remote: ")) + .map(str::to_string) + .collect() + }; + assert_eq!( + remotes(&files["Gemfile.lock"]), + [ + "https://patch.test/gem/tok/10000000/", + "https://patch.test/gem/tok/80000000/", + "https://rubygems.org/", + ], + "generation 1 inserts sorted: {}", + files["Gemfile.lock"] + ); + + let gen2 = [ov("vuln-gem", "9a9a9a9a"), ov("tiny-dep", "80000000")]; + let second = rewrite_registry_redirect(&files, &gen2); + let lock = second.files.get("Gemfile.lock").expect("lock refreshed"); + assert_eq!( + remotes(lock), + [ + "https://patch.test/gem/tok/80000000/", + "https://patch.test/gem/tok/9a9a9a9a/", + "https://rubygems.org/", + ], + "refreshed section re-placed in bundler's order ({eol:?}): {lock}" + ); + let expected_sections = format!( + "GEM\n remote: https://patch.test/gem/tok/80000000/\n specs:\n \ + tiny-dep (1.0.0)\n\nGEM\n remote: https://patch.test/gem/tok/9a9a9a9a/\n \ + specs:\n vuln-gem (1.0.0)\n tiny-dep\n\nGEM\n remote: \ + https://rubygems.org/\n specs:\n\nPLATFORMS\n" + ) + .replace('\n', eol); + assert!( + lock.starts_with(&expected_sections), + "sections move whole, endings kept ({eol:?}): {lock}" + ); + assert!( + second + .edits + .iter() + .any(|e| e.kind == "redirect_gemfile_lock_source_url" + && e.original + == Some(Value::String("https://patch.test/gem/tok/10000000/".into()))), + "refresh still recorded: {:?}", + second.edits + ); + + // Converged: a re-run is a no-op. + for (name, content) in second.files { + files.insert(name, content); + } + let third = rewrite_registry_redirect(&files, &gen2); + assert!( + third.files.is_empty(), + "re-run after the move must be a no-op: {:?}", + third.files + ); + } + } + + /// A converged lock an earlier run left out of bundler's section order + /// (#1186) is healed by the next run even though the remote is + /// unchanged — here by moving the patch section after a sibling that + /// sorts first, to the end of the `GEM` sections. + #[test] + fn gem_out_of_order_patch_section_is_healed_on_rerun() { + let index = "https://z.test/gem/tok/uuid/"; + let mut o = gem_override("rails", "7.0.0"); + if let Some(r) = o.registry_override.as_mut() { + r.index_url = index.into(); + } + let mut files = BTreeMap::new(); + files.insert( + "Gemfile".to_string(), + "source \"https://rubygems.org\"\n\ngem \"rails\", \"7.0.0\"\n".to_string(), + ); + files.insert( + "Gemfile.lock".to_string(), + gem_lock(&format!(" rails (7.0.0) sha256={}", "2".repeat(64))), + ); + let first = rewrite_registry_redirect(&files, std::slice::from_ref(&o)); + for (name, content) in first.files { + files.insert(name, content); + } + let sorted = files["Gemfile.lock"].clone(); + let rails = format!("GEM\n remote: {index}\n specs:\n rails (7.0.0)\n\n"); + let upstream = "GEM\n remote: https://rubygems.org/\n specs:\n\n"; + assert!( + sorted.starts_with(&format!("{upstream}{rails}PLATFORMS\n")), + "fresh insert sorts the patch section last: {sorted}" + ); + // An out-of-order lock as an earlier run could leave it. + files.insert( + "Gemfile.lock".to_string(), + sorted.replacen( + &format!("{upstream}{rails}"), + &format!("{rails}{upstream}"), + 1, + ), + ); + let second = rewrite_registry_redirect(&files, std::slice::from_ref(&o)); + assert_eq!( + second.files.get("Gemfile.lock"), + Some(&sorted), + "re-run restores bundler's order" + ); + assert!( + second + .edits + .iter() + .any(|e| e.kind == "redirect_gemfile_lock_section_order"), + "the move is recorded: {:?}", + second.edits + ); + } + /// A TRANSITIVE redirected dep (undeclared in the Gemfile, appended as a /// source block) becomes a direct source-pinned dependency, so the /// converged lock must gain its ` (= )!` DEPENDENCIES entry —