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 d7f382808..291dabc66 100644 --- a/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs +++ b/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs @@ -2033,6 +2033,31 @@ struct GenerationGem { gem: Vec, } +/// The `sha512-` SRI string of `bytes`. +fn sri(bytes: &[u8]) -> String { + use base64::Engine as _; + format!( + "sha512-{}", + base64::engine::general_purpose::STANDARD.encode(sha2::Sha512::digest(bytes)) + ) +} + +/// The stub gemspec `gem spec --ruby` derives from a built `.gem`. +fn stub_gemspec(gem: &[u8]) -> Vec { + let file = tempfile::Builder::new().suffix(".gem").tempfile().unwrap(); + std::fs::write(file.path(), gem).unwrap(); + let mut spec = Command::new("gem"); + spec.args(["spec", file.path().to_str().unwrap(), "--ruby"]); + cache_env::isolate(&mut spec); + let stub = spec.output().expect("failed to run gem spec"); + assert!( + stub.status.success(), + "gem spec --ruby:\n{}", + String::from_utf8_lossy(&stub.stderr) + ); + stub.stdout +} + /// 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 @@ -2116,29 +2141,35 @@ async fn mount_patch_generation(server: &MockServer, priority: u8, gems: &[Gener .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 } - }], + // The vendored half of each grant: the patched `.gem` and its stub + // gemspec, so a hosted → vendored eject can fetch both. + let mut results = serde_json::Map::new(); + for g in gems { + 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); + let stub = stub_gemspec(&g.gem); + let base = format!("/artifacts/{}", g.uuid); + let gem_url = format!("{}{base}/{}-{DEP_VERSION}.gem", server.uri(), g.name); + let stub_url = format!("{}{base}/{}-{DEP_VERSION}.gemspec", server.uri(), g.name); + results.insert( + g.uuid.to_string(), + serde_json::json!({ + "status": "granted", + "url": hosted_url, + "purl": purl(g), + "artifacts": [ + { "kind": "tarball", "url": gem_url, + "integrity": { "sha512": sri(&g.gem), "sha256": sha } }, + { "kind": "gem-stub-gemspec", "url": stub_url, + "integrity": { "sha512": sri(&stub) } } + ], "registryOverride": { "kind": "rubygems-compact-index", "indexUrl": index_url, @@ -2148,10 +2179,19 @@ async fn mount_patch_generation(server: &MockServer, priority: u8, gems: &[Gener "gemChecksumSha256": sha, } } - }), - ) - }) - .collect(); + }), + ); + for (leaf, bytes) in [ + (format!("{}-{DEP_VERSION}.gem", g.name), g.gem.clone()), + (format!("{}-{DEP_VERSION}.gemspec", g.name), stub), + ] { + Mock::given(method("GET")) + .and(path(format!("{base}/{leaf}"))) + .respond_with(ResponseTemplate::new(200).set_body_bytes(bytes)) + .mount(server) + .await; + } + } Mock::given(method("POST")) .and(path(format!("/v0/orgs/{ORG}/patches/package"))) .respond_with( @@ -2309,6 +2349,270 @@ async fn gem_hosted_superseding_patch_keeps_gem_sections_in_bundler_order() { ); } +/// #1749: two gems hosted at once. On bundler < 2.2 the converged lock +/// holds ONE merged `GEM` section with a Socket remote per patch; lockfile +/// discovery gave up on that section, so a re-scan skipped both gems as +/// `redirect_unattributable` (exit 0), `vex` found no patch, and a +/// superseding patch never landed. Every bundler must re-scan clean, attest +/// both gems, and take the superseding patch. +#[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_two_gems_rescan_supersede_and_attest() { + let Some(fx) = redirect_scanned_project( + "two-hosted-gems", + Spelling::Gemfile, + false, + 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| 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 tiny_patched = TINY_LIB.replace("tiny-ok", "tiny-patched"); + let tiny_gem = build_gem( + &stage.join(TINY_GEN2), + TRANSITIVE, + "1.0.0", + "tiny_dep.rb", + &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: tiny_gem.clone(), + }; + let api = fx._server.uri(); + let gemfile_path = fx.proj.join(fx.gemfile_name); + let lock_path = fx.proj.join(fx.lock_name); + let scan = |what: &str| { + let (code, stdout, stderr) = run_hosted_scan(&fx.proj, &api); + assert_eq!(code, 0, "{what}: {stdout}\nstderr:\n{stderr}"); + assert!( + !stdout.contains("redirect_unattributable"), + "{what}: a hosted gem was skipped as unattributable:\n{stdout}" + ); + stdout + }; + let converge = |what: &str| { + let install = bundle(&fx.proj, &["install"]); + assert!( + install.status.success(), + "{what}: bundle install:\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&install.stdout), + String::from_utf8_lossy(&install.stderr) + ); + std::fs::read_to_string(&lock_path).unwrap() + }; + + // Both gems hosted, then the prescribed unfrozen install. + mount_patch_generation(&fx._server, 2, &[vuln(VULN_GEN2, patched_lib()), tiny()]).await; + scan("generation-2 scan"); + let lock = converge("generation 2"); + for uuid in [VULN_GEN2, TINY_GEN2] { + assert!(lock.contains(uuid), "generation 2 lock:\n{lock}"); + } + if !fx.bundler.at_least(2, 2) { + assert_eq!( + lock.lines().filter(|l| *l == "GEM").count(), + 1, + "bundler < 2.2 merges every remote into one GEM section:\n{lock}" + ); + } + + // An in-sync re-scan attributes both gems and changes nothing. + let gemfile = std::fs::read_to_string(&gemfile_path).unwrap(); + scan("in-sync re-scan"); + assert_eq!(std::fs::read_to_string(&gemfile_path).unwrap(), gemfile); + assert_eq!(std::fs::read_to_string(&lock_path).unwrap(), lock); + + // Manifest-less vex attests both patches from the lock. + let doc_path = fx.tmp.path().join("two-gems-vex.json"); + let (code, stdout, stderr) = run_socket( + &fx.proj, + &[ + "vex", + "--output", + doc_path.to_str().unwrap(), + "--product", + PRODUCT, + "--cwd", + fx.proj.to_str().unwrap(), + "--patch-server-url", + &api, + "--api-url", + &api, + "--org", + ORG, + "--api-token", + "fake", + ], + ); + assert_eq!(code, 0, "vex: {stdout}\nstderr:\n{stderr}"); + let doc: serde_json::Value = + serde_json::from_slice(&std::fs::read(&doc_path).unwrap()).unwrap(); + // Both patches fix the same advisory, so they share one statement. + let stmts = doc["statements"].as_array().unwrap(); + assert_eq!(stmts.len(), 1, "{doc}"); + assert_eq!(stmts[0]["status"], "not_affected", "{doc}"); + let mut subjects: Vec<&str> = stmts[0]["products"][0]["subcomponents"] + .as_array() + .unwrap() + .iter() + .map(|c| c["@id"].as_str().unwrap()) + .collect(); + subjects.sort_unstable(); + assert_eq!( + subjects, + [format!("pkg:gem/{TRANSITIVE}@1.0.0").as_str(), PURL], + "{doc}" + ); + let impact = stmts[0]["impact_statement"].as_str().unwrap(); + for uuid in [VULN_GEN2, TINY_GEN2] { + assert!( + impact.contains(&format!("Socket patch {uuid} (redirected)")), + "{doc}" + ); + } + + // A superseding patch for vuln-gem lands; tiny-dep keeps its patch. + mount_patch_generation( + &fx._server, + 1, + &[ + vuln(VULN_GEN3, patched_lib().replace("PATCHED", "PATCHED-GEN3")), + tiny(), + ], + ) + .await; + scan("generation-3 scan"); + let gemfile = std::fs::read_to_string(&gemfile_path).unwrap(); + assert!( + gemfile.contains(VULN_GEN3) && !gemfile.contains(VULN_GEN2) && gemfile.contains(TINY_GEN2), + "generation 3 Gemfile:\n{gemfile}" + ); + let lock = converge("generation 3"); + assert!( + lock.contains(VULN_GEN3) && !lock.contains(VULN_GEN2) && lock.contains(TINY_GEN2), + "generation 3 lock:\n{lock}" + ); + // A cold checkout of the converged pair installs frozen. + let fresh = stage_fresh_checkout(&fx, "two-gems-fresh-frozen"); + let frozen = bundle_env(&fresh, &["install"], &[("BUNDLE_FROZEN", "true")]); + assert!( + frozen.status.success(), + "frozen install of the generation 3 pair:\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&frozen.stdout), + String::from_utf8_lossy(&frozen.stderr) + ); + + // The hosted → vendored eject (#1648) reads the same wiring: it takes + // both gems instead of refusing the merged section as contested. + let upstream = format!("{api}/upstream"); + let eject = |extra: &[&str]| { + let mut argv = vec!["vendor", "--json"]; + argv.extend_from_slice(extra); + argv.extend([ + "--cwd", + fx.proj.to_str().unwrap(), + "--api-url", + &api, + "--patch-server-url", + &api, + "--org", + ORG, + "--api-token", + "fake", + ]); + let (code, stdout, stderr) = + run_socket_env(&fx.proj, &argv, &[("SOCKET_RUBYGEMS_URL", &upstream)]); + assert_eq!(code, 0, "vendor {extra:?}: {stdout}\nstderr:\n{stderr}"); + let env: serde_json::Value = serde_json::from_str(&stdout).unwrap(); + for purl in [PURL.to_string(), format!("pkg:gem/{TRANSITIVE}@1.0.0")] { + assert!( + env["events"] + .as_array() + .is_some_and(|e| e.iter().any(|e| e["purl"] == purl.as_str())), + "vendor {extra:?} takes {purl}: {env}" + ); + } + }; + eject(&["--dry-run"]); + assert_eq!(std::fs::read_to_string(&gemfile_path).unwrap(), gemfile); + assert_eq!(std::fs::read_to_string(&lock_path).unwrap(), lock); + eject(&["--yes"]); + 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/") && !lock.contains("/patch-registry/"), + "the eject leaves no hosted remote:\n{gemfile}\n{lock}" + ); + // A cold checkout of the vendored pair installs frozen and loads both + // patched gems. + let fresh = stage_fresh_checkout(&fx, "two-gems-ejected-frozen"); + let frozen = bundle_env(&fresh, &["install"], &[("BUNDLE_FROZEN", "true")]); + assert!( + frozen.status.success(), + "frozen install of the ejected pair:\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&frozen.stdout), + String::from_utf8_lossy(&frozen.stderr) + ); + let gen3_lib = patched_lib().replace("PATCHED", "PATCHED-GEN3"); + for (name, lib, want) in [ + (DEP, "vuln_gem.rb", &gen3_lib), + (TRANSITIVE, "tiny_dep.rb", &tiny_patched), + ] { + let loaded = bundle( + &fresh, + &[ + "exec", + "ruby", + "-e", + "print File.read(File.join(Gem.loaded_specs[ARGV[0]].full_gem_path, 'lib', ARGV[1]))", + name, + lib, + ], + ); + assert!( + loaded.status.success(), + "bundle exec loads {name}:\n{}", + String::from_utf8_lossy(&loaded.stderr) + ); + assert_eq!( + String::from_utf8_lossy(&loaded.stdout), + *want, + "{name} loads its patched bytes" + ); + } +} + /// #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 @@ -3965,8 +4269,6 @@ async fn gem_hosted_unwind_names_a_patched_archive_in_vendor_cache() { /// hosted registry override. Priority 1, so it answers every later /// `patches/package` call in place of the hosted-only grant. async fn mount_vendored_grant(fx: &RedirectFixture) { - use base64::Engine as _; - use sha2::Sha512; let server = &fx._server; let gem_path = fx .tmp @@ -3975,22 +4277,7 @@ async fn mount_vendored_grant(fx: &RedirectFixture) { .join(format!("{DEP}-src")) .join(format!("{DEP}-{DEP_VERSION}.gem")); let gem = std::fs::read(&gem_path).expect("the patched .gem from build_gem"); - let mut spec = Command::new("gem"); - spec.args(["spec", gem_path.to_str().unwrap(), "--ruby"]); - cache_env::isolate(&mut spec); - let stub = spec.output().expect("failed to run gem spec"); - assert!( - stub.status.success(), - "gem spec --ruby:\n{}", - String::from_utf8_lossy(&stub.stderr) - ); - let stub = stub.stdout; - let sri = |bytes: &[u8]| { - format!( - "sha512-{}", - base64::engine::general_purpose::STANDARD.encode(Sha512::digest(bytes)) - ) - }; + let stub = stub_gemspec(&gem); let base = format!("/artifacts/{UUID}"); let gem_url = format!("{}{base}/{DEP}-{DEP_VERSION}.gem", server.uri()); let stub_url = format!("{}{base}/{DEP}-{DEP_VERSION}.gemspec", server.uri()); diff --git a/crates/socket-patch-core/src/vex/discover/gem.rs b/crates/socket-patch-core/src/vex/discover/gem.rs index cf1b4c464..77bd8d140 100644 --- a/crates/socket-patch-core/src/vex/discover/gem.rs +++ b/crates/socket-patch-core/src/vex/discover/gem.rs @@ -92,20 +92,23 @@ //! The section alone cannot say which remote served which spec, but bundler //! resolves a gem declared inside a `source "" do` block ONLY from that //! source, and marks it source-pinned (`!`) in `DEPENDENCIES`. So a spec in -//! a merged section with exactly one Socket patch-registry remote is a -//! hosted ref when BOTH hold: `DEPENDENCIES` pins it (`name …!`) and the +//! a merged section is a hosted ref of one of its Socket patch-registry +//! remotes when BOTH hold: `DEPENDENCIES` pins it (`name …!`) and the //! sibling manifest (`Gemfile` for `Gemfile.lock`, `gems.rb` for //! `gems.locked`) declares it inside a `source` block whose URL is that -//! remote. That manifest is read ONLY for this cross-check — i.e. only when -//! the lock has a merged section, whose own Socket remote already names the -//! uuid — so a Gemfile-only wiring (lock not merged) stays with the ledger -//! fallback. +//! remote. With several hosted gems the section lists one Socket remote +//! per patch, and each remote is attributed on its own. That manifest is +//! read ONLY for this cross-check — i.e. only when the lock has a merged +//! section, whose own Socket remotes already name the uuids — so a +//! Gemfile-only wiring (lock not merged) stays with the ledger fallback. //! The manifest is read as text, not run as Ruby, so the cross-check fails //! closed on what text cannot settle: `=begin` … `=end` block comments are //! skipped, and a gem the manifest ALSO declares in a block for another //! source (dead code, a conditional branch) is not attributed. -//! The section's other specs (upstream gems) are silent; a merged Socket -//! section with no such pinned gem is [`DIAG_REF_UNATTRIBUTABLE`]. +//! The section's other specs (upstream gems) are silent; a Socket remote of +//! a merged section with no such pinned gem, or whose pinned gems the +//! section does not lock, is [`DIAG_REF_UNATTRIBUTABLE`] (the section's +//! other Socket remotes are still read). //! //! ## Vendored (`vendor::gem` module docs) //! @@ -406,40 +409,67 @@ fn extract_lock( } } for sec in &lock.sections { - let wiring = match sec.header { - "GEM" => hosted_wiring(ctx, file, lock, sec, blocks, out), - "PATH" => vendored_wiring(file, sec, out), - _ => None, + let wirings = match sec.header { + "GEM" => hosted_wirings(ctx, file, lock, sec, blocks, out), + "PATH" => vendored_wiring(file, sec, out).into_iter().collect(), + _ => Vec::new(), }; - let Some(wiring) = wiring else { - continue; - }; - if sec.specs.is_empty() { - out.diag( - DIAG_REF_UNATTRIBUTABLE, - file, - format!( - "{file}: line {}: {} section for Socket patch {} locks no gems", - sec.line_no, - sec.header, - wiring.uuid() - ), - ); - continue; + for wiring in &wirings { + section_refs(file, lock, &owners, sec, wiring, out); } - for line in &sec.specs { - if let Wiring::Hosted { - only: Some(only), .. - } = &wiring - { - // Merged section: only the gems source-pinned to the Socket - // remote are its; the rest came from the other remotes. - if !line.parsed.is_some_and(|s| only.contains(s.name)) { - continue; - } - } - spec_ref(file, lock, &owners, line, &wiring, out); + } +} + +/// Every ref one Socket-wired section yields under `wiring`. +fn section_refs( + file: &str, + lock: &GemfileLock<'_>, + owners: &HashMap<&str, BTreeSet>, + sec: &Section<'_>, + wiring: &Wiring<'_>, + out: &mut Discovery, +) { + if sec.specs.is_empty() { + out.diag( + DIAG_REF_UNATTRIBUTABLE, + file, + format!( + "{file}: line {}: {} section for Socket patch {} locks no gems", + sec.line_no, + sec.header, + wiring.uuid() + ), + ); + return; + } + let only = match wiring { + Wiring::Hosted { + only: Some(only), .. + } => Some(only), + _ => None, + }; + let mut matched = false; + for line in &sec.specs { + // Merged section: only the gems source-pinned to the Socket remote + // are its; the rest came from the other remotes. + if only.is_some_and(|only| !line.parsed.is_some_and(|s| only.contains(s.name))) { + continue; } + matched = true; + spec_ref(file, lock, owners, line, wiring, out); + } + if let Some(only) = only.filter(|_| !matched) { + out.diag( + DIAG_REF_UNATTRIBUTABLE, + file, + format!( + "{file}: line {}: the gems source-pinned to Socket patch {} ({}) are not \ + locked in this merged GEM section", + sec.line_no, + wiring.uuid(), + only.iter().cloned().collect::>().join(", ") + ), + ); } } @@ -464,16 +494,18 @@ impl Wiring<'_> { } } -/// A `GEM` section's Socket-hosted wiring, or `None` (diagnosed when it is -/// Socket-shaped but unusable). -fn hosted_wiring<'t>( +/// A `GEM` section's Socket-hosted wirings: one per Socket remote, so a +/// merged section (module docs) with several hosted gems yields each of +/// them. Empty when the section has no usable Socket remote (diagnosed when +/// it is Socket-shaped but unusable). +fn hosted_wirings<'t>( ctx: &DiscoverCtx<'_>, file: &str, lock: &GemfileLock<'t>, sec: &Section<'t>, blocks: &[(String, String)], out: &mut Discovery, -) -> Option> { +) -> Vec> { for remote in sec.remotes.iter().filter(|r| names_vendor_dir(r)) { out.diag( DIAG_REF_INVALID, @@ -490,62 +522,60 @@ fn hosted_wiring<'t>( .iter() .filter_map(|r| ctx.hosted_uuid(r).map(|uuid| (*r, uuid))) .collect(); - let socket_count = socket.len(); - let (remote, uuid) = socket.into_iter().next()?; - let mut only = None; - if sec.remotes.len() != 1 { - // A merged section (module docs): attributable only per gem, through - // the source pin, and only with a single Socket remote in it. A gem - // the manifest ALSO declares in a block for another source is - // ambiguous (the reader sees text, not which branch Ruby runs), so it - // is not attributed to the Socket remote. - let pinned: BTreeSet = - if socket_count == 1 && is_patch_registry_index(remote, &uuid) { - blocks - .iter() - .filter(|(url, name)| { - same_remote(url, remote) - && lock.pinned.contains(name.as_str()) - && blocks - .iter() - .all(|(u, n)| n != name || same_remote(u, remote)) - }) - .map(|(_, name)| name.clone()) - .collect() - } else { - BTreeSet::new() - }; - if pinned.is_empty() { + let merged = sec.remotes.len() != 1; + let mut wirings = Vec::new(); + for (remote, uuid) in socket { + if !is_patch_registry_index(remote, &uuid) { out.diag( - DIAG_REF_UNATTRIBUTABLE, + DIAG_REF_INVALID, file, format!( - "{file}: line {}: GEM section lists {} remotes (a merged multi-source \ - lock), and no gem in it is source-pinned to Socket patch {uuid} ({remote}) \ - by both DEPENDENCIES (`!`) and a {} `source … do` block, so its gems cannot \ - be tied to the patch", - sec.line_no, - sec.remotes.len(), - bundler_manifest_for(file) + "{file}: line {}: GEM remote {remote:?} is on the Socket patch server but \ + is not a patch-registry/gem/// index", + sec.line_no ), ); - return None; + continue; } - only = Some(pinned); - } - if !is_patch_registry_index(remote, &uuid) { - out.diag( - DIAG_REF_INVALID, - file, - format!( - "{file}: line {}: GEM remote {remote:?} is on the Socket patch server but is not \ - a patch-registry/gem/// index", - sec.line_no - ), - ); - return None; + let mut only = None; + if merged { + // A merged section (module docs): attributable only per gem, + // through the source pin to THIS Socket remote. A gem the + // manifest ALSO declares in a block for another source is + // ambiguous (the reader sees text, not which branch Ruby runs), + // so it is not attributed to any Socket remote. + let pinned: BTreeSet = blocks + .iter() + .filter(|(url, name)| { + same_remote(url, remote) + && lock.pinned.contains(name.as_str()) + && blocks + .iter() + .all(|(u, n)| n != name || same_remote(u, remote)) + }) + .map(|(_, name)| name.clone()) + .collect(); + if pinned.is_empty() { + out.diag( + DIAG_REF_UNATTRIBUTABLE, + file, + format!( + "{file}: line {}: GEM section lists {} remotes (a merged multi-source \ + lock), and no gem in it is source-pinned to Socket patch {uuid} \ + ({remote}) by both DEPENDENCIES (`!`) and a {} `source … do` block, \ + so its gems cannot be tied to the patch", + sec.line_no, + sec.remotes.len(), + bundler_manifest_for(file) + ), + ); + continue; + } + only = Some(pinned); + } + wirings.push(Wiring::Hosted { remote, uuid, only }); } - Some(Wiring::Hosted { remote, uuid, only }) + wirings } /// A `PATH` section's vendored wiring, or `None` (diagnosed when it points @@ -1237,8 +1267,8 @@ mod tests { assert_eq!(run(&p).await.refs.len(), 1); } - /// Each half of the source pin is required, the block must name THIS - /// remote, and a merged section with two Socket remotes stays ambiguous. + /// Each half of the source pin is required, and the block must name + /// THIS remote. #[tokio::test] async fn merged_section_without_a_full_source_pin_is_unattributable() { let other = index(UUID_B); @@ -1270,12 +1300,6 @@ mod tests { .replace("source \"https://patch", "# source \"https://patch"), ), ), - ( - "two Socket remotes", - merged_lock(&index(UUID_A), "!") - .replace(" specs:\n", &format!(" remote: {other}\n specs:\n")), - Some(block_gemfile(&index(UUID_A))), - ), ]; for (case, lock, gemfile) in cases { let p = Project::new(); @@ -1294,6 +1318,169 @@ mod tests { } } + /// The bundler 1.17-2.1 lock after the prescribed unfrozen install over + /// TWO rewritten gems (#1749, shape from real bundler 2.1.4): one merged + /// GEM section with one Socket remote per patch, both gems pinned. + fn two_patch_merged_lock(a: &str, b: &str) -> String { + format!( + "GEM\n remote: https://rubygems.org/\n remote: {a}\n remote: {b}\n specs:\n \ + tiny-dep (1.0.0)\n vuln-gem (1.0.0)\n tiny-dep\n\nPLATFORMS\n ruby\n\n\ + DEPENDENCIES\n tiny-dep (= 1.0.0)!\n vuln-gem (= 1.0.0)!\n\nBUNDLED WITH\n \ + 2.1.4\n" + ) + } + + fn two_block_gemfile(a: &str, b: &str) -> String { + format!( + "source \"https://rubygems.org\"\n\nsource \"{a}\" do\n gem \"vuln-gem\", \"1.0.0\"\n\ + end\n\nsource \"{b}\" do\n gem \"tiny-dep\", \"1.0.0\"\nend\n" + ) + } + + /// #1749: a merged section with one Socket remote PER hosted gem + /// attributes each gem to its own remote, so `vex` and the re-scan see + /// both patches instead of giving up on the whole section. + #[tokio::test] + async fn merged_section_attributes_each_socket_remote() { + let uuid_c = "80000000-0000-4000-8000-000000000000"; + let p = Project::new(); + p.write( + "Gemfile.lock", + two_patch_merged_lock(&index(UUID_A), &index(uuid_c)), + ); + p.write("Gemfile", two_block_gemfile(&index(UUID_A), &index(uuid_c))); + let out = run(&p).await; + assert_refs( + &out, + &[ + ("pkg:gem/tiny-dep@1.0.0", uuid_c, WiringMode::Hosted), + ("pkg:gem/vuln-gem@1.0.0", UUID_A, WiringMode::Hosted), + ], + ); + assert!(out.diagnostics.is_empty(), "{:?}", out.diagnostics); + for r in &out.refs { + let want = if r.uuid == UUID_A { + index(UUID_A) + } else { + index(uuid_c) + }; + assert_eq!(r.url.as_deref(), Some(want.as_str()), "{r:?}"); + } + assert_eq!( + out.hosted_claim("pkg:gem/vuln-gem@1.0.0", UUID_A), + Some(true) + ); + assert_eq!( + out.hosted_claim("pkg:gem/tiny-dep@1.0.0", uuid_c), + Some(true) + ); + // Neither gem is attributed to the other's remote. + assert_eq!( + out.hosted_claim("pkg:gem/vuln-gem@1.0.0", uuid_c), + Some(false) + ); + } + + /// A Socket remote no gem is source-pinned to is diagnosed on its own; + /// it no longer hides the other Socket remote's pinned gem. A gem + /// declared in blocks for BOTH Socket remotes is attributed to neither. + #[tokio::test] + async fn merged_section_unpinned_socket_remote_is_diagnosed_alone() { + let lock = two_patch_merged_lock(&index(UUID_A), &index(UUID_B)) + .replace(" tiny-dep (= 1.0.0)!\n", " tiny-dep (= 1.0.0)\n"); + let p = Project::new(); + p.write("Gemfile.lock", &lock); + p.write("Gemfile", block_gemfile(&index(UUID_A))); + let out = run(&p).await; + assert_refs( + &out, + &[("pkg:gem/vuln-gem@1.0.0", UUID_A, WiringMode::Hosted)], + ); + assert_eq!( + diag_codes(&out), + vec![DIAG_REF_UNATTRIBUTABLE], + "{:?}", + out.diagnostics + ); + assert!( + out.diagnostics[0].detail.contains(UUID_B), + "{:?}", + out.diagnostics + ); + + let both = format!( + "{}source \"{}\" do\n gem \"vuln-gem\", \"1.0.0\"\nend\n", + block_gemfile(&index(UUID_A)), + index(UUID_B) + ); + let p = Project::new(); + p.write("Gemfile.lock", &lock); + p.write("Gemfile", both); + let out = run(&p).await; + assert_refs(&out, &[]); + assert_eq!( + diag_codes(&out), + vec![DIAG_REF_UNATTRIBUTABLE; 2], + "{:?}", + out.diagnostics + ); + for uuid in [UUID_A, UUID_B] { + assert_eq!( + out.diagnostics + .iter() + .filter(|d| d.detail.contains(uuid)) + .count(), + 1, + "{uuid}: {:?}", + out.diagnostics + ); + } + + // A Socket remote that is not a patch-registry index is an invalid + // ref, as in a single-remote section; the other remote still pins. + let bad = format!("{}?x=1", index(UUID_B)); + let p = Project::new(); + p.write("Gemfile.lock", two_patch_merged_lock(&index(UUID_A), &bad)); + p.write("Gemfile", block_gemfile(&index(UUID_A))); + let out = run(&p).await; + assert_refs( + &out, + &[("pkg:gem/vuln-gem@1.0.0", UUID_A, WiringMode::Hosted)], + ); + assert_eq!( + diag_codes(&out), + vec![DIAG_REF_INVALID], + "{:?}", + out.diagnostics + ); + + // A gem pinned to a Socket remote but not locked in its merged + // section yields no ref, and says so. + let lock = two_patch_merged_lock(&index(UUID_A), &index(UUID_B)) + .replace(" tiny-dep (1.0.0)\n", "") + .replace(" tiny-dep\n", ""); + let p = Project::new(); + p.write("Gemfile.lock", lock); + p.write("Gemfile", two_block_gemfile(&index(UUID_A), &index(UUID_B))); + let out = run(&p).await; + assert_refs( + &out, + &[("pkg:gem/vuln-gem@1.0.0", UUID_A, WiringMode::Hosted)], + ); + assert_eq!( + diag_codes(&out), + vec![DIAG_REF_UNATTRIBUTABLE], + "{:?}", + out.diagnostics + ); + assert!( + out.diagnostics[0].detail.contains(UUID_B) + && out.diagnostics[0].detail.contains("tiny-dep"), + "{:?}", + out.diagnostics + ); + } + /// The Gemfile is read only to cross-check a merged Socket section, so a /// Gemfile-only wiring (the pre-2.6 mixed pair, lock not merged) is never /// recognized: it stays a ledger-fallback decision, not a discovery