diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 1a0c56081..6b2cd9ea4 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -641,8 +641,16 @@ the model is **not uniform** today: is neither crawled nor allowed to hide the `gem env` homes (#915). When several roots hold **coexisting physical copies of one `gem@version`** (bundler-2's scoped store beside bundler-1's flat store), `apply`/`rollback` patch/restore **every copy** — one summary event per copy, - mirroring npm's multi-copy fan-out — while single-representative consumers (`get`, `vendor`, - `vex`) use the highest-precedence copy. PyPI follows the same rule: when the crawler resolves + mirroring npm's multi-copy fan-out, and `vex` / `apply --check` verify every copy — while + single-representative consumers (`get`, `vendor`) use the highest-precedence copy. One gem home + holding **several builds** of a version (the ruby-platform `x-1.0/` beside + `x-1.0-x86_64-linux-gnu/`, after `gem install --platform ruby` or a `force_ruby_platform` + toggle) counts as several copies too: the plain dir first, then each platform dir, so `apply`, + `rollback`, `apply --check` and `vex` cover the platform build RubyGems loads (#1092). Each build + is matched to its own release variant. A build is then not the only candidate, so the default + mismatch policy never overwrites it: a build whose bytes match no manifest variant is left + untouched, fails `apply` (`no matching variant found`) and withholds the `vex` statement, and + `rollback` skips it. PyPI follows the same rule: when the crawler resolves one release in several site-packages dirs (a Pipenv WORKON_HOME venv beside an auto-detected `./.venv`, or the user site beside a system dir in global scope), agent `apply` patches **every** copy, one summary event per copy, because any of them may be the one the interpreter imports. diff --git a/crates/socket-patch-cli/src/commands/apply.rs b/crates/socket-patch-cli/src/commands/apply.rs index b2ad7e938..d7e8ed0aa 100644 --- a/crates/socket-patch-cli/src/commands/apply.rs +++ b/crates/socket-patch-cli/src/commands/apply.rs @@ -92,6 +92,15 @@ fn mismatch_event_detail(file: &str, dry_run: bool) -> String { ) } +/// Whether another of a gem's `copies` sits in the same gem home as `copy`: +/// two builds of one gem version (`x-1.0/` and `x-1.0-/`) +/// installed side by side. +pub(crate) fn has_build_sibling(copy: &Path, copies: &[PathBuf]) -> bool { + copies + .iter() + .any(|other| other != copy && other.parent() == copy.parent()) +} + /// One `content_mismatch_overwritten` run warning per mismatch-overwritten /// file across `results`, in the event detail's words prefixed with the /// package purl — what a nested apply hands back to `get` / `scan --mode @@ -2333,8 +2342,18 @@ async fn apply_patches_inner( // let a lone x86_64-linux record silently overwrite a // darwin install in the Bundler plugin's `--silent` // auto-apply, where the warn half of warn-and-apply is - // invisible. - if !args.force && (variants.len() > 1 || variants[0] != base_purl) { + // invisible. The same holds for a copy with a BUILD + // SIBLING (another copy in the same gem home: the + // ruby-platform `x-1.0/` beside `x-1.0-x86_64-linux/`, + // #1092): it is no longer the only candidate, so a + // mismatch there means another distribution, and the + // copy stays gated rather than overwritten. + if !args.force + && (variants.len() > 1 + || variants[0] != base_purl + || (Ecosystem::from_purl(purl) == Some(Ecosystem::Gem) + && has_build_sibling(pkg_path, copy_paths))) + { let first_status = match representative_file(&patch.files) { Some((file_name, file_info)) => Some( verify_file_patch(pkg_path, file_name, file_info) diff --git a/crates/socket-patch-cli/src/commands/rollback.rs b/crates/socket-patch-cli/src/commands/rollback.rs index 783d7b63f..c4eb19534 100644 --- a/crates/socket-patch-cli/src/commands/rollback.rs +++ b/crates/socket-patch-cli/src/commands/rollback.rs @@ -2572,47 +2572,82 @@ pub(crate) async fn rollback_patches_inner( // (remove's identifier flow drops the whole group the same way). let mut narrowed_out: Vec = Vec::new(); for (_base, entries) in groups { - let to_rollback: Vec<(&String, &PathBuf)> = if entries.len() == 1 { - entries - } else { - // All variants in a group resolve to the same installed path. - let pkg_path = entries[0].1; - let candidates: Vec<(&str, &HashMap)> = entries - .iter() - .filter_map(|(purl, _)| { - filtered_manifest - .patches - .get(*purl) - .map(|p| (purl.as_str(), &p.files)) - }) - .collect(); - let matched = select_installed_variants(pkg_path, &candidates).await; - if matched.is_empty() { - // No variant matches the installed distribution (e.g. a - // locally-modified file). Fall back to attempting every - // variant so the per-file verification surfaces the - // mismatch rather than silently skipping the package. - entries - } else { - let winners: HashSet = matched - .iter() - .map(|&i| candidates[i].0.to_string()) - .collect(); - narrowed_out.extend( - entries - .iter() - .filter(|(p, _)| !winners.contains(*p)) - .map(|(p, _)| (*p).clone()), - ); + if entries.len() == 1 { + rollback_targets.extend( entries .into_iter() - .filter(|(p, _)| winners.contains(*p)) - .collect() + .map(|(purl, path)| CopyTarget::plain(purl, path)), + ); + continue; + } + // Narrow PER COPY: one gem home can hold several builds of the + // version (`x-1.0/` beside `x-1.0-x86_64-linux/`, #1092), each + // patched with its own variant, so the first copy's variant says + // nothing about the others. + let mut purls: Vec<&String> = Vec::new(); + let mut paths: Vec<&PathBuf> = Vec::new(); + for (purl, path) in &entries { + if !purls.contains(purl) { + purls.push(purl); } - }; + if !paths.contains(path) { + paths.push(path); + } + } + let candidates: Vec<(&str, &HashMap)> = purls + .iter() + .filter_map(|purl| { + filtered_manifest + .patches + .get(*purl) + .map(|p| (purl.as_str(), &p.files)) + }) + .collect(); + let mut chosen: Vec<(&PathBuf, Vec<&str>)> = Vec::new(); + let mut unmatched: Vec<&PathBuf> = Vec::new(); + for path in &paths { + let matched = select_installed_variants(path, &candidates).await; + if matched.is_empty() { + unmatched.push(path); + } else { + chosen.push((path, matched.iter().map(|&i| candidates[i].0).collect())); + } + } + for path in unmatched { + // A gem build matching no variant beside a sibling build that + // does holds another distribution: nothing of this patch was + // ever written there. + let sibling_matched = chosen + .iter() + .any(|(other, _)| *other != path && other.parent() == path.parent()); + if sibling_matched && purls.iter().all(|p| p.starts_with("pkg:gem/")) { + continue; + } + // No variant matches the installed distribution (e.g. a + // locally-modified file). Fall back to attempting every + // variant so the per-file verification surfaces the mismatch + // rather than silently skipping the package. + chosen.push((path, candidates.iter().map(|(purl, _)| *purl).collect())); + } + let attempted: HashSet<&str> = chosen + .iter() + .flat_map(|(_, purls)| purls.iter().copied()) + .collect(); + // Narrowed-away variants: their distribution is on no copy. + narrowed_out.extend( + purls + .iter() + .filter(|p| !attempted.contains(p.as_str())) + .map(|p| (*p).clone()), + ); rollback_targets.extend( - to_rollback + entries .into_iter() + .filter(|(purl, path)| { + chosen + .iter() + .any(|(p, purls)| p == path && purls.contains(&purl.as_str())) + }) .map(|(purl, path)| CopyTarget::plain(purl, path)), ); } diff --git a/crates/socket-patch-cli/src/commands/vex.rs b/crates/socket-patch-cli/src/commands/vex.rs index 5de8c3542..907127d67 100644 --- a/crates/socket-patch-cli/src/commands/vex.rs +++ b/crates/socket-patch-cli/src/commands/vex.rs @@ -998,7 +998,7 @@ pub(crate) async fn vex_copy_sets( } else { None }; - copies + let mut sets: HashMap> = copies .iter() .map(|(purl, paths)| { let paths = match scope.as_ref().filter(|_| purl.starts_with("pkg:maven/")) { @@ -1015,7 +1015,67 @@ pub(crate) async fn vex_copy_sets( }; (purl.clone(), paths) }) - .collect() + .collect(); + narrow_gem_variant_copies(manifest, &mut sets).await; + sets +} + +/// RubyGems: give each qualified variant key (`?platform=`…) only the +/// copies holding its distribution. One gem home can hold several builds of +/// a version (`x-1.0/` beside `x-1.0-x86_64-linux/`, #1092), each patched +/// with its own variant, but the dispatch hands every key every copy, so +/// each key would be judged against the other build's bytes and never +/// attested. A copy holding no variant stays on every key, so the +/// statement is withheld while that build is unaccounted for (`apply +/// --check` reports it as `no_matching_variant`). An unqualified singleton +/// names no distribution and keeps every copy. +async fn narrow_gem_variant_copies( + manifest: &PatchManifest, + sets: &mut HashMap>, +) { + use socket_patch_core::manifest::schema::PatchFileInfo; + use socket_patch_core::patch::apply::select_installed_variants; + use socket_patch_core::utils::purl::strip_purl_qualifiers; + let mut groups: std::collections::BTreeMap<&str, Vec<&String>> = Default::default(); + for purl in sets.keys() { + if purl.starts_with("pkg:gem/") { + groups + .entry(strip_purl_qualifiers(purl)) + .or_default() + .push(purl); + } + } + let mut narrowed: Vec<(String, Vec)> = Vec::new(); + for (base, mut keys) in groups { + if keys.len() == 1 && keys[0] == base { + continue; + } + keys.sort(); + let variants: Vec<(&str, &HashMap)> = keys + .iter() + .filter_map(|k| manifest.patches.get(*k).map(|r| (k.as_str(), &r.files))) + .collect(); + let mut paths: Vec<&PathBuf> = keys.iter().flat_map(|k| &sets[*k]).collect(); + paths.sort(); + paths.dedup(); + let mut kept: HashMap<&str, Vec> = HashMap::new(); + for path in paths { + let matched = select_installed_variants(path, &variants).await; + if matched.is_empty() { + for (key, _) in &variants { + kept.entry(*key).or_default().push(path.clone()); + } + } + for idx in matched { + kept.entry(variants[idx].0).or_default().push(path.clone()); + } + } + for (key, _) in &variants { + let paths = kept.remove(*key).unwrap_or_default(); + narrowed.push((key.to_string(), paths)); + } + } + sets.extend(narrowed); } /// Maven / Gradle: withhold the statement of every purl a build may still diff --git a/crates/socket-patch-cli/src/ecosystem_dispatch.rs b/crates/socket-patch-cli/src/ecosystem_dispatch.rs index a733b3179..66967d12f 100644 --- a/crates/socket-patch-cli/src/ecosystem_dispatch.rs +++ b/crates/socket-patch-cli/src/ecosystem_dispatch.rs @@ -283,14 +283,10 @@ fn merge_variant_copies( /// after the first. fn merge_npm_copies( out: &mut HashMap>, - _purls: &[String], + purls: &[String], packages: HashMap>, ) { - for (purl, pkgs) in packages { - for pkg in pkgs { - push_path(out, purl.clone(), pkg.path); - } - } + merge_each_copy(merge_variant_copies, out, purls, packages); } /// `paths` in order with every path that resolves to an already-listed @@ -439,7 +435,7 @@ async fn dispatch_find( eco = Ecosystem::Gem, options = options, silent = silent, - crawler = RubyCrawler, + crawler = GemCopiesCrawler, get_paths = get_gem_paths, using_label = "ruby gem paths", err_label = "Ruby gems", @@ -447,7 +443,13 @@ async fn dispatch_find( // crawler emits the base PURL and the platform is resolved by // hashing the installed files, same as PyPI. purls_override = dedup_qualified_purls, - on_match = variant_merge, + // One gem home can hold a version's ruby-platform AND platform + // builds; fold every copy in (#1092). + on_match = |out: &mut HashMap>, + purls: &[String], + packages: HashMap>| { + merge_each_copy(variant_merge, out, purls, packages) + }, ); scan_ecosystem!( @@ -669,6 +671,43 @@ impl NpmCrawlSnapshot { } } +/// [`RubyCrawler`] for [`dispatch_find`], returning every installed copy of +/// each gem in a gem home (the ruby-platform dir and each platform dir). +struct GemCopiesCrawler; + +impl GemCopiesCrawler { + async fn get_gem_paths( + &self, + options: &CrawlerOptions, + ) -> Result, std::io::Error> { + RubyCrawler.get_gem_paths(options).await + } + + async fn find_by_purls( + &self, + gem_path: &std::path::Path, + purls: &[String], + ) -> Result>, std::io::Error> { + RubyCrawler.find_all_by_purls(gem_path, purls).await + } +} + +/// Feed every copy in `packages` through `merge`, one copy at a time and in +/// order, so a crawler that returns several copies per PURL reaches the +/// one-copy [`MergeFn`]s unchanged. +fn merge_each_copy( + merge: MergeFn, + out: &mut HashMap>, + purls: &[String], + packages: HashMap>, +) { + for (purl, copies) in packages { + for pkg in copies { + merge(out, purls, HashMap::from([(purl.clone(), pkg)])); + } + } +} + /// [`NpmCrawler`] for [`dispatch_find`], answering its root discovery from /// an earlier crawl's roots when it has them. struct NpmRootsCrawler<'a> { @@ -831,8 +870,8 @@ pub async fn find_manifest_package_copies_reusing( .await; let bases = dedup_qualified_purls(gem_purls); for store in &stores { - if let Ok(found) = RubyCrawler.find_by_purls(store, &bases).await { - merge_qualified(&mut copies, gem_purls, found); + if let Ok(found) = RubyCrawler.find_all_by_purls(store, &bases).await { + merge_each_copy(merge_qualified, &mut copies, gem_purls, found); } } } diff --git a/crates/socket-patch-cli/tests/apply/in_process_gem_multicopy.rs b/crates/socket-patch-cli/tests/apply/in_process_gem_multicopy.rs index 0f0e45e15..b3a8629cc 100644 --- a/crates/socket-patch-cli/tests/apply/in_process_gem_multicopy.rs +++ b/crates/socket-patch-cli/tests/apply/in_process_gem_multicopy.rs @@ -217,3 +217,329 @@ fn rollback_restores_every_coexisting_gem_store_copy() { } assert_ne!(before_hash, after_hash); } + +/// Lay down a project whose one scoped store holds BOTH the ruby-platform +/// build `rack-3.1.0/` and the platform build `rack-3.1.0-x86_64-linux/` of +/// `rack@3.1.0` (#1092: `gem install --platform ruby` beside a platform +/// install, or a project toggling `force_ruby_platform`). RubyGems loads +/// the platform build. Returns +/// `(root, plain_file, platform_file, original, patched)`. +fn build_platform_variant_project(tmp: &Path) -> (PathBuf, PathBuf, PathBuf, Vec, Vec) { + let original = b"module Rack\n VERSION = 'VULNERABLE'\nend\n".to_vec(); + let mut patched = original.clone(); + patched.extend_from_slice(b"# SOCKET-PATCHED-GEM-PLATFORM\n"); + let before_hash = git_sha256(&original); + let after_hash = git_sha256(&patched); + + let gems = tmp + .join("vendor") + .join("bundle") + .join("ruby") + .join("3.2.0") + .join("gems"); + let plain_file = write_copy(&gems.join("rack-3.1.0"), &original); + let platform_file = write_copy(&gems.join("rack-3.1.0-x86_64-linux"), &original); + + stage_manifest_and_blob(tmp, &before_hash, &after_hash, &patched); + // A vulnerability, so `vex` has a statement to attest. + let manifest = tmp.join(".socket").join("manifest.json"); + let text = std::fs::read_to_string(&manifest).unwrap().replace( + r#""vulnerabilities": {}"#, + r#""vulnerabilities": { "GHSA-gplt-gplt-gplt": { + "cves": ["CVE-2026-1092"], "summary": "synthetic", + "severity": "high", "description": "synthetic" + }}"#, + ); + assert!(text.contains("GHSA-gplt-gplt-gplt")); + std::fs::write(&manifest, text).unwrap(); + std::fs::write( + tmp.join(".socket").join("blobs").join(&before_hash), + &original, + ) + .unwrap(); + + ( + tmp.to_path_buf(), + plain_file, + platform_file, + original, + patched, + ) +} + +/// `run_json` without `--json`, for `vex`, whose stdout is the document. +fn run_raw(root: &Path, args: &[&str]) -> (i32, String, String) { + let mut cmd = Command::new(binary()); + cmd.args(args) + .args(["--offline", "--ecosystems", "gem", "--cwd"]) + .arg(root); + for (key, _) in std::env::vars_os() { + let k = key.to_string_lossy(); + if (k.starts_with("SOCKET_") && k != "SOCKET_NO_CONFIG") || k.starts_with("BUNDLE_") { + cmd.env_remove(&key); + } + } + cmd.env("SOCKET_TELEMETRY_DISABLED", "1"); + let out = cmd.output().expect("run socket-patch"); + ( + out.status.code().unwrap_or(-1), + String::from_utf8_lossy(&out.stdout).to_string(), + String::from_utf8_lossy(&out.stderr).to_string(), + ) +} + +/// #1092: `apply` patches the platform build beside the ruby-platform one, +/// and `rollback` restores both. +#[test] +fn apply_and_rollback_cover_every_platform_variant_copy() { + let tmp = tempfile::tempdir().unwrap(); + let (root, plain_file, platform_file, original, patched) = + build_platform_variant_project(tmp.path()); + + let (code, v) = run_json(&root, &["apply"]); + assert_eq!(code, 0, "apply must succeed; envelope={v}"); + for (label, f) in [("plain", &plain_file), ("platform", &platform_file)] { + assert_eq!( + std::fs::read(f).unwrap(), + patched, + "{label} copy at {f:?} was NOT patched; envelope={v}" + ); + } + assert_eq!( + v["summary"]["applied"], 2, + "summary must count both copies; envelope={v}" + ); + + let (code, v) = run_json(&root, &["rollback", "--yes"]); + assert_eq!(code, 0, "rollback must succeed; envelope={v}"); + for (label, f) in [("plain", &plain_file), ("platform", &platform_file)] { + assert_eq!( + std::fs::read(f).unwrap(), + original, + "{label} copy at {f:?} was NOT restored; envelope={v}" + ); + } +} + +/// #1092: with only the ruby-platform copy patched, `vex` must not attest +/// `not_affected` and `apply --check` must not report the patch in sync, +/// because the platform copy RubyGems loads is still vulnerable. +#[test] +fn vex_and_check_refuse_an_unpatched_platform_variant_copy() { + let tmp = tempfile::tempdir().unwrap(); + let (root, plain_file, platform_file, original, patched) = + build_platform_variant_project(tmp.path()); + std::fs::write(&plain_file, &patched).unwrap(); + assert_eq!(std::fs::read(&platform_file).unwrap(), original); + + let (code, stdout, stderr) = run_raw(&root, &["vex", "--product", "pkg:gem/app@1.0.0"]); + assert!( + !stdout.contains("not_affected"), + "vex must not attest while the platform copy is unpatched (exit {code})\ + \nstdout:\n{stdout}\nstderr:\n{stderr}" + ); + + let (code, stdout, stderr) = run_raw(&root, &["apply", "--check"]); + assert_ne!( + code, 0, + "apply --check must flag the unpatched platform copy\ + \nstdout:\n{stdout}\nstderr:\n{stderr}" + ); + + // Once both copies are patched, both gates pass. + std::fs::write(&platform_file, &patched).unwrap(); + let (code, stdout, stderr) = run_raw(&root, &["vex", "--product", "pkg:gem/app@1.0.0"]); + assert_eq!(code, 0, "vex\nstdout:\n{stdout}\nstderr:\n{stderr}"); + assert!(stdout.contains("not_affected"), "vex doc:\n{stdout}"); + let (code, stdout, stderr) = run_raw(&root, &["apply", "--check"]); + assert_eq!( + code, 0, + "apply --check\nstdout:\n{stdout}\nstderr:\n{stderr}" + ); +} + +/// #1092, global mode: `apply -g --global-prefix /gems` patches +/// the platform build too (plain RubyGems activates it, no Bundler needed). +#[test] +fn global_apply_covers_every_platform_variant_copy() { + let tmp = tempfile::tempdir().unwrap(); + let (root, plain_file, platform_file, _original, patched) = + build_platform_variant_project(tmp.path()); + let gems = plain_file + .parent() + .unwrap() + .parent() + .unwrap() + .parent() + .unwrap(); + + let (code, v) = run_json( + &root, + &["apply", "-g", "--global-prefix", gems.to_str().unwrap()], + ); + assert_eq!(code, 0, "apply -g must succeed; envelope={v}"); + for (label, f) in [("plain", &plain_file), ("platform", &platform_file)] { + assert_eq!( + std::fs::read(f).unwrap(), + patched, + "{label} copy at {f:?} was NOT patched; envelope={v}" + ); + } +} + +/// A platform build whose bytes are not the patch's distribution (a native +/// build that ships a different file) is another distribution, not a +/// locally modified copy: apply patches the plain copy, never overwrites +/// the platform one with the plain build's bytes, and fails loudly because +/// the copy RubyGems loads stays unpatched. +#[test] +fn apply_refuses_a_platform_copy_holding_another_distribution() { + let tmp = tempfile::tempdir().unwrap(); + let (root, plain_file, platform_file, _original, patched) = + build_platform_variant_project(tmp.path()); + let native = b"module Rack\n VERSION = 'NATIVE BUILD'\nend\n".to_vec(); + std::fs::write(&platform_file, &native).unwrap(); + + let (code, v) = run_json(&root, &["apply"]); + assert_ne!(code, 0, "apply must flag the unpatched copy; envelope={v}"); + assert_eq!(std::fs::read(&plain_file).unwrap(), patched, "envelope={v}"); + assert_eq!( + std::fs::read(&platform_file).unwrap(), + native, + "envelope={v}" + ); +} + +/// One gem home holding BOTH builds, each a different distribution with its +/// own qualified manifest record (`?platform=ruby` for `rack-3.1.0/`, +/// `?platform=x86_64-linux` for `rack-3.1.0-x86_64-linux/`). Returns +/// `(root, [(plain_file, original, patched), (platform_file, original, patched)])`. +#[allow(clippy::type_complexity)] +fn build_two_variant_project(tmp: &Path) -> (PathBuf, [(PathBuf, Vec, Vec); 2]) { + let gems = tmp + .join("vendor") + .join("bundle") + .join("ruby") + .join("3.2.0") + .join("gems"); + let socket = tmp.join(".socket"); + std::fs::create_dir_all(socket.join("blobs")).unwrap(); + let mut records = Vec::new(); + let mut copies = Vec::new(); + for (dir, platform, uuid) in [ + ("rack-3.1.0", "ruby", "67656d6d-756c-4469-8370-72756279302e"), + ( + "rack-3.1.0-x86_64-linux", + "x86_64-linux", + "67656d6d-756c-4469-8370-6c696e7578ff", + ), + ] { + let original = format!("module Rack\n BUILD = '{platform}'\nend\n").into_bytes(); + let mut patched = original.clone(); + patched.extend_from_slice(format!("# PATCHED-{platform}\n").as_bytes()); + let (before, after) = (git_sha256(&original), git_sha256(&patched)); + std::fs::write(socket.join("blobs").join(&before), &original).unwrap(); + std::fs::write(socket.join("blobs").join(&after), &patched).unwrap(); + records.push(format!( + r#""{PURL}?platform={platform}": {{ + "uuid": "{uuid}", + "exportedAt": "2024-01-01T00:00:00Z", + "files": {{ "lib/rack.rb": {{ + "beforeHash": "{before}", "afterHash": "{after}" + }}}}, + "vulnerabilities": {{ "GHSA-gplt-gplt-gplt": {{ + "cves": ["CVE-2026-1092"], "summary": "synthetic", + "severity": "high", "description": "synthetic" + }}}}, + "description": "gem platform fixture", "license": "MIT", "tier": "free" + }}"# + )); + copies.push((write_copy(&gems.join(dir), &original), original, patched)); + } + std::fs::write( + socket.join("manifest.json"), + format!(r#"{{ "patches": {{ {} }} }}"#, records.join(",")), + ) + .unwrap(); + let [a, b]: [(PathBuf, Vec, Vec); 2] = copies.try_into().unwrap(); + (tmp.to_path_buf(), [a, b]) +} + +/// Each build is patched, attested and rolled back with ITS OWN variant, +/// never judged against the other build's bytes. +#[test] +fn each_platform_build_uses_its_own_variant() { + let tmp = tempfile::tempdir().unwrap(); + let (root, copies) = build_two_variant_project(tmp.path()); + + let (code, v) = run_json(&root, &["apply"]); + assert_eq!(code, 0, "apply must succeed; envelope={v}"); + for (f, _, patched) in &copies { + assert_eq!(&std::fs::read(f).unwrap(), patched, "{f:?}; envelope={v}"); + } + + let (code, stdout, stderr) = run_raw(&root, &["apply", "--check"]); + assert_eq!( + code, 0, + "apply --check\nstdout:\n{stdout}\nstderr:\n{stderr}" + ); + + let (code, stdout, stderr) = run_raw(&root, &["vex", "--product", "pkg:gem/app@1.0.0"]); + assert_eq!(code, 0, "vex\nstdout:\n{stdout}\nstderr:\n{stderr}"); + let doc: serde_json::Value = serde_json::from_str(&stdout).unwrap(); + let products: Vec = doc["statements"] + .as_array() + .unwrap() + .iter() + .filter(|s| s["status"] == "not_affected") + .flat_map(|s| s["products"].as_array().unwrap().clone()) + .flat_map(|p| { + p["subcomponents"] + .as_array() + .cloned() + .unwrap_or_default() + .into_iter() + .map(|c| c["@id"].as_str().unwrap_or_default().to_string()) + }) + .collect(); + for platform in ["ruby", "x86_64-linux"] { + assert!( + products + .iter() + .any(|p| p.ends_with(&format!("platform={platform}"))), + "the {platform} build must be attested; vex doc:\n{stdout}" + ); + } + + let (code, v) = run_json(&root, &["rollback", "--yes"]); + assert_eq!(code, 0, "rollback must succeed; envelope={v}"); + for (f, original, _) in &copies { + assert_eq!(&std::fs::read(f).unwrap(), original, "{f:?}; envelope={v}"); + } +} + +/// After apply refused a build holding another distribution (left +/// untouched), rollback restores the patched build and skips that one. +#[test] +fn rollback_skips_a_platform_copy_holding_another_distribution() { + let tmp = tempfile::tempdir().unwrap(); + let (root, plain_file, platform_file, original, _patched) = + build_platform_variant_project(tmp.path()); + let native = b"module Rack\n VERSION = 'NATIVE BUILD'\nend\n".to_vec(); + std::fs::write(&platform_file, &native).unwrap(); + let (code, v) = run_json(&root, &["apply"]); + assert_ne!(code, 0, "precondition: apply flags the native build; {v}"); + + let (code, v) = run_json(&root, &["rollback", "--yes"]); + assert_eq!(code, 0, "rollback must succeed; envelope={v}"); + assert_eq!( + std::fs::read(&plain_file).unwrap(), + original, + "envelope={v}" + ); + assert_eq!( + std::fs::read(&platform_file).unwrap(), + native, + "envelope={v}" + ); +} diff --git a/crates/socket-patch-core/src/crawlers/ruby_crawler.rs b/crates/socket-patch-core/src/crawlers/ruby_crawler.rs index f6e3af752..ec70c30f0 100644 --- a/crates/socket-patch-core/src/crawlers/ruby_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/ruby_crawler.rs @@ -181,16 +181,45 @@ impl RubyCrawler { /// Find specific packages by PURL inside a single gem directory. /// - /// Gem directories follow the `-` pattern. + /// Gem directories follow the `-` pattern. Each PURL + /// maps to its highest-precedence copy (see `locate_gem_dirs_sync`); + /// [`Self::find_all_by_purls`] returns every copy. pub async fn find_by_purls( &self, gem_path: &Path, purls: &[String], ) -> Result, std::io::Error> { - let mut result: HashMap = HashMap::new(); - - for purl in purls { - if let Some((name, version)) = crate::utils::purl::parse_gem_purl(purl) { + Ok(self + .find_all_by_purls(gem_path, purls) + .await? + .into_iter() + .filter_map(|(purl, copies)| copies.into_iter().next().map(|pkg| (purl, pkg))) + .collect()) + } + + /// [`Self::find_by_purls`], keeping EVERY installed copy of each PURL in + /// `gem_path`: the ruby-platform `-/` dir first, then + /// each `--/` sibling. One gem home can hold + /// both builds of a version (`gem install --platform ruby`, or a + /// project toggling `force_ruby_platform`), and RubyGems loads the + /// platform one, so a writer or checker that sees only the first copy + /// patches or attests the copy nothing loads (#1092). + pub async fn find_all_by_purls( + &self, + gem_path: &Path, + purls: &[String], + ) -> Result>, std::io::Error> { + let gem_path = gem_path.to_path_buf(); + let purls = purls.to_vec(); + Ok(crate::utils::fs::run_blocking(move || { + // `gem_path`'s entry names, listed at most once per call (on the + // first PURL whose platform dirs are looked for). + let mut names: Option> = None; + let mut result: HashMap> = HashMap::new(); + for purl in &purls { + let Some((name, version)) = crate::utils::purl::parse_gem_purl(purl) else { + continue; + }; let (name, version) = (name.as_ref(), version.as_ref()); // SECURITY: name/version come straight from the (untrusted) // manifest PURL and are formatted into a `-` @@ -204,24 +233,26 @@ impl RubyCrawler { continue; } // The purl is the base PURL (qualifiers stripped upstream). - // Resolve it to the installed gem dir, which may carry a - // `-` suffix for platform gems. - if let Some(gem_dir) = self.locate_gem_dir(gem_path, name, version).await { - result.insert( - purl.clone(), - CrawledPackage { + // Resolve it to every installed gem dir, plain or carrying + // a `-` suffix. + let copies: Vec = + locate_gem_dirs_sync(&gem_path, name, version, &mut names) + .into_iter() + .map(|gem_dir| CrawledPackage { name: name.to_string(), version: version.to_string(), namespace: None, purl: purl.clone(), path: gem_dir, - }, - ); + }) + .collect(); + if !copies.is_empty() { + result.insert(purl.clone(), copies); } } - } - - Ok(result) + result + }) + .await) } // ------------------------------------------------------------------ @@ -1057,32 +1088,21 @@ impl RubyCrawler { } Some((name.to_string(), version.to_string())) } +} - /// Locate an installed gem directory for a base `name`/`version`. - /// - /// Plain (ruby-platform) gems live in `-/`; platform - /// gems append a `-` suffix - /// (`--x86_64-linux/`). Only one platform is installed - /// per environment, so we return the exact dir when present, otherwise - /// the first verifying `--*` directory. - async fn locate_gem_dir(&self, gem_path: &Path, name: &str, version: &str) -> Option { - let exact = gem_path.join(format!("{name}-{version}")); - if self.verify_gem_at_path(&exact).await { - return Some(exact); - } - let prefix = format!("{name}-{version}-"); - for entry in list_dir_entries(gem_path).await { - let file_name = entry.file_name(); - let dir_name = file_name.to_string_lossy(); - if dir_name.starts_with(&prefix) { - let dir = gem_path.join(&*dir_name); - if self.verify_gem_at_path(&dir).await { - return Some(dir); - } - } - } - None - } +/// Whether `dir_name` is a `--` gem dir of exactly +/// `name`/`version`. A RubyGems platform starts with a letter (`x86_64-…`, +/// `arm64-…`, `java`, `universal-java-1.8`), so a digit after the prefix +/// means another gem: `foo-1.0-2.0/` is the gem `foo-1.0` at `2.0`, not a +/// build of `foo` `1.0`. +fn is_platform_dir_of(dir_name: &str, name: &str, version: &str) -> bool { + dir_name + .strip_prefix(name) + .and_then(|rest| rest.strip_prefix('-')) + .and_then(|rest| rest.strip_prefix(version)) + .and_then(|rest| rest.strip_prefix('-')) + .and_then(|platform| platform.chars().next()) + .is_some_and(|c| c.is_ascii_alphabetic()) } impl Default for RubyCrawler { @@ -1117,7 +1137,9 @@ impl RubyCrawler { if !path_safety::is_safe_name_version(name, version) { return None; } - let gem_dir = locate_gem_dir_sync(&gem_path, name, version, &mut names)?; + let gem_dir = locate_gem_dirs_sync(&gem_path, name, version, &mut names) + .into_iter() + .next()?; Some(CrawledPackage { name: name.to_string(), version: version.to_string(), @@ -1132,29 +1154,37 @@ impl RubyCrawler { } } -/// Blocking twin of `RubyCrawler::locate_gem_dir` over a lazily listed, -/// reused `gem_path` listing. -fn locate_gem_dir_sync( +/// Every installed gem directory for a base `name`/`version` in +/// `gem_path`, over a lazily listed, reused `gem_path` listing. +/// +/// Plain (ruby-platform) gems live in `-/`; platform gems +/// append a `-` suffix (`--x86_64-linux/`). A gem +/// home can hold several of them at once, so every verifying one is +/// returned: the plain dir first, then the platform dirs in name order. +fn locate_gem_dirs_sync( gem_path: &Path, name: &str, version: &str, names: &mut Option>, -) -> Option { +) -> Vec { + let mut dirs = Vec::new(); let exact = gem_path.join(format!("{name}-{version}")); if verify_gem_at_path_sync(&exact) { - return Some(exact); + dirs.push(exact); } - let prefix = format!("{name}-{version}-"); let names = super::listing::names_memoized(gem_path, names); - for dir_name in names.iter() { - if dir_name.starts_with(&prefix) { - let dir = gem_path.join(dir_name); - if verify_gem_at_path_sync(&dir) { - return Some(dir); - } - } - } - None + let mut siblings: Vec<&String> = names + .iter() + .filter(|dir_name| is_platform_dir_of(dir_name, name, version)) + .collect(); + siblings.sort(); + dirs.extend( + siblings + .into_iter() + .map(|dir_name| gem_path.join(dir_name)) + .filter(|dir| verify_gem_at_path_sync(dir)), + ); + dirs } /// Blocking twin of `RubyCrawler::verify_gem_at_path`: a directory holding @@ -5064,7 +5094,7 @@ mod tests { /// `find_by_purls` must resolve a base PURL to a platform gem dir /// that carries a `-` suffix on disk. Exercises the - /// `locate_gem_dir` prefix-scan fallback, which the original + /// `locate_gem_dirs_sync` platform-dir scan, which the original /// suite only covered for the exact (plain-platform) case. #[tokio::test] async fn find_by_purls_resolves_platform_suffixed_dir() { @@ -5127,10 +5157,10 @@ mod tests { assert_eq!(packages[0].path, plat_dir); } - /// A plain `-` dir must win over any platform - /// sibling when both are present (exact match short-circuits). + /// A plain `-` dir comes before any platform sibling + /// when both are present, so single-copy consumers keep the plain dir. #[tokio::test] - async fn locate_gem_dir_prefers_exact_over_platform() { + async fn locate_gem_dirs_puts_exact_before_platform() { let dir = tempfile::tempdir().unwrap(); let exact = dir.path().join("rails-7.1.0"); let plat = dir.path().join("rails-7.1.0-x86_64-linux"); @@ -5143,6 +5173,66 @@ mod tests { assert_eq!(result.get("pkg:gem/rails@7.1.0").unwrap().path, exact); } + /// #1092: a gem home holding the ruby-platform build AND platform + /// builds of one version yields every copy, plain dir first, so apply + /// patches (and vex checks) the platform copy RubyGems loads too. + #[tokio::test] + async fn find_all_by_purls_returns_every_platform_copy() { + let dir = tempfile::tempdir().unwrap(); + let exact = dir.path().join("ffi-1.17.2"); + let gnu = dir.path().join("ffi-1.17.2-x86_64-linux-gnu"); + let arm = dir.path().join("ffi-1.17.2-aarch64-linux-gnu"); + for d in [&exact, &gnu, &arm] { + tokio::fs::create_dir_all(d.join("lib")).await.unwrap(); + } + // Not copies of ffi@1.17.2: a hollow platform dir, another + // version, and the gem `ffi-1.17.2` at version 3.0. + tokio::fs::create_dir_all(dir.path().join("ffi-1.17.2-java")) + .await + .unwrap(); + tokio::fs::create_dir_all(dir.path().join("ffi-1.17.20-x86_64-linux/lib")) + .await + .unwrap(); + tokio::fs::create_dir_all(dir.path().join("ffi-1.17.2-3.0/lib")) + .await + .unwrap(); + + let purl = "pkg:gem/ffi@1.17.2".to_string(); + let crawler = RubyCrawler::new(); + let result = crawler + .find_all_by_purls(dir.path(), std::slice::from_ref(&purl)) + .await + .unwrap(); + let paths: Vec<&PathBuf> = result[&purl].iter().map(|p| &p.path).collect(); + assert_eq!(paths, vec![&exact, &arm, &gnu]); + + // The single-copy lookup keeps the plain dir. + let one = crawler + .find_by_purls(dir.path(), std::slice::from_ref(&purl)) + .await + .unwrap(); + assert_eq!(one[&purl].path, exact); + } + + /// A dir that merely starts with `--` is not a platform + /// copy when it parses as another gem (`foo-1.0` at `2.0`). + #[test] + fn is_platform_dir_of_rejects_another_gem_sharing_the_prefix() { + assert!(is_platform_dir_of("foo-1.0-x86_64-linux", "foo", "1.0")); + assert!(is_platform_dir_of("foo-1.0-x86_64-darwin-21", "foo", "1.0")); + assert!(is_platform_dir_of("foo-1.0-java", "foo", "1.0")); + assert!(is_platform_dir_of( + "json-1.8.6-universal-java-1.8", + "json", + "1.8.6" + )); + assert!(!is_platform_dir_of("foo-1.0-2.0", "foo", "1.0")); + assert!(!is_platform_dir_of("foo-1.0", "foo", "1.0")); + assert!(!is_platform_dir_of("foo-1.0-", "foo", "1.0")); + assert!(!is_platform_dir_of("foo-1.0.1-x86_64-linux", "foo", "1.0")); + assert!(!is_platform_dir_of("foobar-1.0-x86_64-linux", "foo", "1.0")); + } + // ── gem env memo ────────────────────────────────────────────── /// Every caller gets the first answer, however many ask at once, and @@ -5363,7 +5453,7 @@ mod tests { ); } - /// The exact-match arm of `locate_gem_dir` must *verify gem content*, + /// The exact-match arm of `locate_gem_dirs_sync` must *verify gem content*, /// not merely accept that `-/` exists on disk. When the /// exact dir is present but empty (no `lib/`, no `.gemspec` — a /// malformed/partial install), resolution must fall through to a valid @@ -5430,7 +5520,7 @@ mod tests { // ── PURL coordinate traversal (untrusted manifest input) ────── /// A tampered manifest PURL whose name carries `..` must not resolve - /// to a directory outside the gem root. `locate_gem_dir` joins + /// to a directory outside the gem root. `locate_gem_dirs_sync` joins /// `-` straight onto `gem_path`, and /// `verify_gem_at_path` only checks for `lib/`/`.gemspec`, so without /// a coordinate gate `pkg:gem/../outside@1.0.0` escapes the gem store @@ -5649,7 +5739,7 @@ mod tests { assert!(!crawler.verify_gem_at_path(&gem_dir).await); } - /// `locate_gem_dir`'s prefix scan must skip a `--*` dir + /// `locate_gem_dirs_sync`' platform-dir scan must skip a `--*` dir /// that matches the prefix but fails gem verification (a hollow /// platform dir — no `lib/`, no `.gemspec`): alone it resolves /// nothing, and beside a valid platform sibling the sibling wins.