From e3eeefb9b306c1deb0ab9c9aeb0c1a2fafb682d6 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 23:22:21 +0000 Subject: [PATCH 1/3] Start fix for #1186 Assisted-by: Claude Code:claude-opus-5-5 From 62f363838a2de6951956eb6e2a3972dce9458faa Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 23:38:26 +0000 Subject: [PATCH 2/3] Keep gem lock sections sorted after a re-scan A hosted re-scan that gives an already-redirected gem a new index URL (a superseding patch, or a rotated grant) rewrote that GEM section's remote: line where it stood. Bundler writes GEM sections sorted by their remote URL, so with two or more patched gems the new URL could land out of order, and every frozen or deployment install on Bundler 4.0.19+ then failed with exit 16 ("Your lockfile needs to be updated"). The refreshed section now moves to Bundler's sorted position, the same rule a fresh insert already follows. A lock an earlier run left out of order is healed on the next scan. Fixes #1186 Assisted-by: Claude Code:claude-opus-5-5 --- .../src/formats/gem/hosted.rs | 91 ++++++++-- .../src/patch/redirect/mod.rs | 156 ++++++++++++++++++ 2 files changed, 235 insertions(+), 12 deletions(-) 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 — From d4ccd68e99f99dec5682f5975246a07170855ca7 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 23:54:52 +0000 Subject: [PATCH 3/3] Test superseded gem patches with real Bundler Adds a hosted gem e2e that patches two gems into their own registry sections, then supersedes one with a patch whose URL sorts after its sibling. The committed lock must keep Bundler's section order: bundle lock leaves it byte-identical and a cold frozen install accepts it. Refs #1186 Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/e2e_redirect_gem_build.rs | 288 ++++++++++++++++++ 1 file changed, 288 insertions(+) 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