Skip to content

Commit ff4ced8

Browse files
Fix gem apply/vex skipping platform gem copies (#1092) (#1395)
* Start fix for #1092 Assisted-by: Claude Code:claude-opus-5-5 * Patch and check every installed build of a gem A gem home can hold both builds of one gem version, such as ffi-1.17.2/ and ffi-1.17.2-x86_64-linux-gnu/. RubyGems and Bundler load the platform build, but agent-mode apply, rollback, apply --check and vex only ever looked at the plain dir. So apply patched a copy nothing loads, and vex attested not_affected while the loaded copy stayed vulnerable. Global mode (-g) had the same gap. The gem crawler now returns every verifying build of a version in a gem home (plain dir first, then each platform dir in name order), and the dispatch maps feed all of them to the commands that already act on every copy. A platform dir must parse back to the same gem name and version, so foo-1.0-2.0/ (the gem foo-1.0 at 2.0) is never taken as a build of foo 1.0. When builds sit side by side, a build whose bytes match none of the patch's variants is a different distribution rather than a locally edited copy. Apply leaves it untouched and fails, instead of overwriting it with another build's bytes. Fixes #1092 Assisted-by: Claude Code:claude-opus-5-5 * Match each gem build to its own patch variant Now that every build of a gem version in one gem home is a copy, a manifest with a separate record per build (?platform=ruby for x-1.0/, ?platform=x86_64-linux for x-1.0-x86_64-linux/) must be judged build by build: - rollback picked a variant from the first copy and rolled it back on every copy, failing on the other build and dropping its record. It now picks variants per copy, and skips a build that holds none of them when a sibling build does (nothing was ever written there). - vex judged each variant against every build, so two correctly patched builds were never attested. Each qualified variant now gets only the builds holding its distribution; a build holding none stays on every key, so the statement is still withheld. Also from review: the build-sibling apply gate is gem-only (PyPI site-packages dirs can share a parent), the gem home is listed once per lookup instead of once per patch, a dotted platform such as universal-java-1.8 is still recognised, and the contract no longer lists vex as a single-copy consumer. Assisted-by: Claude Code:claude-opus-5-5 * Satisfy clippy in per-copy rollback narrowing Assisted-by: Claude Code:claude-opus-5-5 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6f0ea2c commit ff4ced8

7 files changed

Lines changed: 693 additions & 116 deletions

File tree

‎crates/socket-patch-cli/CLI_CONTRACT.md‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -646,8 +646,16 @@ the model is **not uniform** today:
646646
is neither crawled nor allowed to hide the `gem env` homes (#915). When several roots hold
647647
**coexisting physical copies of one `gem@version`** (bundler-2's scoped store beside bundler-1's
648648
flat store), `apply`/`rollback` patch/restore **every copy** — one summary event per copy,
649-
mirroring npm's multi-copy fan-out — while single-representative consumers (`get`, `vendor`,
650-
`vex`) use the highest-precedence copy. PyPI follows the same rule: when the crawler resolves
649+
mirroring npm's multi-copy fan-out, and `vex` / `apply --check` verify every copy — while
650+
single-representative consumers (`get`, `vendor`) use the highest-precedence copy. One gem home
651+
holding **several builds** of a version (the ruby-platform `x-1.0/` beside
652+
`x-1.0-x86_64-linux-gnu/`, after `gem install --platform ruby` or a `force_ruby_platform`
653+
toggle) counts as several copies too: the plain dir first, then each platform dir, so `apply`,
654+
`rollback`, `apply --check` and `vex` cover the platform build RubyGems loads (#1092). Each build
655+
is matched to its own release variant. A build is then not the only candidate, so the default
656+
mismatch policy never overwrites it: a build whose bytes match no manifest variant is left
657+
untouched, fails `apply` (`no matching variant found`) and withholds the `vex` statement, and
658+
`rollback` skips it. PyPI follows the same rule: when the crawler resolves
651659
one release in several site-packages dirs (a Pipenv WORKON_HOME venv beside an auto-detected
652660
`./.venv`, or the user site beside a system dir in global scope), agent `apply` patches **every**
653661
copy, one summary event per copy, because any of them may be the one the interpreter imports.

‎crates/socket-patch-cli/src/commands/apply.rs‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,15 @@ fn mismatch_event_detail(file: &str, dry_run: bool) -> String {
9292
)
9393
}
9494

95+
/// Whether another of a gem's `copies` sits in the same gem home as `copy`:
96+
/// two builds of one gem version (`x-1.0/` and `x-1.0-<platform>/`)
97+
/// installed side by side.
98+
pub(crate) fn has_build_sibling(copy: &Path, copies: &[PathBuf]) -> bool {
99+
copies
100+
.iter()
101+
.any(|other| other != copy && other.parent() == copy.parent())
102+
}
103+
95104
/// One `content_mismatch_overwritten` run warning per mismatch-overwritten
96105
/// file across `results`, in the event detail's words prefixed with the
97106
/// package purl — what a nested apply hands back to `get` / `scan --mode
@@ -2333,8 +2342,18 @@ async fn apply_patches_inner(
23332342
// let a lone x86_64-linux record silently overwrite a
23342343
// darwin install in the Bundler plugin's `--silent`
23352344
// auto-apply, where the warn half of warn-and-apply is
2336-
// invisible.
2337-
if !args.force && (variants.len() > 1 || variants[0] != base_purl) {
2345+
// invisible. The same holds for a copy with a BUILD
2346+
// SIBLING (another copy in the same gem home: the
2347+
// ruby-platform `x-1.0/` beside `x-1.0-x86_64-linux/`,
2348+
// #1092): it is no longer the only candidate, so a
2349+
// mismatch there means another distribution, and the
2350+
// copy stays gated rather than overwritten.
2351+
if !args.force
2352+
&& (variants.len() > 1
2353+
|| variants[0] != base_purl
2354+
|| (Ecosystem::from_purl(purl) == Some(Ecosystem::Gem)
2355+
&& has_build_sibling(pkg_path, copy_paths)))
2356+
{
23382357
let first_status = match representative_file(&patch.files) {
23392358
Some((file_name, file_info)) => Some(
23402359
verify_file_patch(pkg_path, file_name, file_info)

‎crates/socket-patch-cli/src/commands/rollback.rs‎

Lines changed: 71 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -2572,47 +2572,82 @@ pub(crate) async fn rollback_patches_inner(
25722572
// (remove's identifier flow drops the whole group the same way).
25732573
let mut narrowed_out: Vec<String> = Vec::new();
25742574
for (_base, entries) in groups {
2575-
let to_rollback: Vec<(&String, &PathBuf)> = if entries.len() == 1 {
2576-
entries
2577-
} else {
2578-
// All variants in a group resolve to the same installed path.
2579-
let pkg_path = entries[0].1;
2580-
let candidates: Vec<(&str, &HashMap<String, PatchFileInfo>)> = entries
2581-
.iter()
2582-
.filter_map(|(purl, _)| {
2583-
filtered_manifest
2584-
.patches
2585-
.get(*purl)
2586-
.map(|p| (purl.as_str(), &p.files))
2587-
})
2588-
.collect();
2589-
let matched = select_installed_variants(pkg_path, &candidates).await;
2590-
if matched.is_empty() {
2591-
// No variant matches the installed distribution (e.g. a
2592-
// locally-modified file). Fall back to attempting every
2593-
// variant so the per-file verification surfaces the
2594-
// mismatch rather than silently skipping the package.
2595-
entries
2596-
} else {
2597-
let winners: HashSet<String> = matched
2598-
.iter()
2599-
.map(|&i| candidates[i].0.to_string())
2600-
.collect();
2601-
narrowed_out.extend(
2602-
entries
2603-
.iter()
2604-
.filter(|(p, _)| !winners.contains(*p))
2605-
.map(|(p, _)| (*p).clone()),
2606-
);
2575+
if entries.len() == 1 {
2576+
rollback_targets.extend(
26072577
entries
26082578
.into_iter()
2609-
.filter(|(p, _)| winners.contains(*p))
2610-
.collect()
2579+
.map(|(purl, path)| CopyTarget::plain(purl, path)),
2580+
);
2581+
continue;
2582+
}
2583+
// Narrow PER COPY: one gem home can hold several builds of the
2584+
// version (`x-1.0/` beside `x-1.0-x86_64-linux/`, #1092), each
2585+
// patched with its own variant, so the first copy's variant says
2586+
// nothing about the others.
2587+
let mut purls: Vec<&String> = Vec::new();
2588+
let mut paths: Vec<&PathBuf> = Vec::new();
2589+
for (purl, path) in &entries {
2590+
if !purls.contains(purl) {
2591+
purls.push(purl);
26112592
}
2612-
};
2593+
if !paths.contains(path) {
2594+
paths.push(path);
2595+
}
2596+
}
2597+
let candidates: Vec<(&str, &HashMap<String, PatchFileInfo>)> = purls
2598+
.iter()
2599+
.filter_map(|purl| {
2600+
filtered_manifest
2601+
.patches
2602+
.get(*purl)
2603+
.map(|p| (purl.as_str(), &p.files))
2604+
})
2605+
.collect();
2606+
let mut chosen: Vec<(&PathBuf, Vec<&str>)> = Vec::new();
2607+
let mut unmatched: Vec<&PathBuf> = Vec::new();
2608+
for path in &paths {
2609+
let matched = select_installed_variants(path, &candidates).await;
2610+
if matched.is_empty() {
2611+
unmatched.push(path);
2612+
} else {
2613+
chosen.push((path, matched.iter().map(|&i| candidates[i].0).collect()));
2614+
}
2615+
}
2616+
for path in unmatched {
2617+
// A gem build matching no variant beside a sibling build that
2618+
// does holds another distribution: nothing of this patch was
2619+
// ever written there.
2620+
let sibling_matched = chosen
2621+
.iter()
2622+
.any(|(other, _)| *other != path && other.parent() == path.parent());
2623+
if sibling_matched && purls.iter().all(|p| p.starts_with("pkg:gem/")) {
2624+
continue;
2625+
}
2626+
// No variant matches the installed distribution (e.g. a
2627+
// locally-modified file). Fall back to attempting every
2628+
// variant so the per-file verification surfaces the mismatch
2629+
// rather than silently skipping the package.
2630+
chosen.push((path, candidates.iter().map(|(purl, _)| *purl).collect()));
2631+
}
2632+
let attempted: HashSet<&str> = chosen
2633+
.iter()
2634+
.flat_map(|(_, purls)| purls.iter().copied())
2635+
.collect();
2636+
// Narrowed-away variants: their distribution is on no copy.
2637+
narrowed_out.extend(
2638+
purls
2639+
.iter()
2640+
.filter(|p| !attempted.contains(p.as_str()))
2641+
.map(|p| (*p).clone()),
2642+
);
26132643
rollback_targets.extend(
2614-
to_rollback
2644+
entries
26152645
.into_iter()
2646+
.filter(|(purl, path)| {
2647+
chosen
2648+
.iter()
2649+
.any(|(p, purls)| p == path && purls.contains(&purl.as_str()))
2650+
})
26162651
.map(|(purl, path)| CopyTarget::plain(purl, path)),
26172652
);
26182653
}

‎crates/socket-patch-cli/src/commands/vex.rs‎

Lines changed: 62 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1029,7 +1029,7 @@ pub(crate) async fn vex_copy_sets(
10291029
} else {
10301030
None
10311031
};
1032-
copies
1032+
let mut sets: HashMap<String, Vec<PathBuf>> = copies
10331033
.iter()
10341034
.map(|(purl, paths)| {
10351035
let paths = match scope.as_ref().filter(|_| purl.starts_with("pkg:maven/")) {
@@ -1046,7 +1046,67 @@ pub(crate) async fn vex_copy_sets(
10461046
};
10471047
(purl.clone(), paths)
10481048
})
1049-
.collect()
1049+
.collect();
1050+
narrow_gem_variant_copies(manifest, &mut sets).await;
1051+
sets
1052+
}
1053+
1054+
/// RubyGems: give each qualified variant key (`?platform=`…) only the
1055+
/// copies holding its distribution. One gem home can hold several builds of
1056+
/// a version (`x-1.0/` beside `x-1.0-x86_64-linux/`, #1092), each patched
1057+
/// with its own variant, but the dispatch hands every key every copy, so
1058+
/// each key would be judged against the other build's bytes and never
1059+
/// attested. A copy holding no variant stays on every key, so the
1060+
/// statement is withheld while that build is unaccounted for (`apply
1061+
/// --check` reports it as `no_matching_variant`). An unqualified singleton
1062+
/// names no distribution and keeps every copy.
1063+
async fn narrow_gem_variant_copies(
1064+
manifest: &PatchManifest,
1065+
sets: &mut HashMap<String, Vec<PathBuf>>,
1066+
) {
1067+
use socket_patch_core::manifest::schema::PatchFileInfo;
1068+
use socket_patch_core::patch::apply::select_installed_variants;
1069+
use socket_patch_core::utils::purl::strip_purl_qualifiers;
1070+
let mut groups: std::collections::BTreeMap<&str, Vec<&String>> = Default::default();
1071+
for purl in sets.keys() {
1072+
if purl.starts_with("pkg:gem/") {
1073+
groups
1074+
.entry(strip_purl_qualifiers(purl))
1075+
.or_default()
1076+
.push(purl);
1077+
}
1078+
}
1079+
let mut narrowed: Vec<(String, Vec<PathBuf>)> = Vec::new();
1080+
for (base, mut keys) in groups {
1081+
if keys.len() == 1 && keys[0] == base {
1082+
continue;
1083+
}
1084+
keys.sort();
1085+
let variants: Vec<(&str, &HashMap<String, PatchFileInfo>)> = keys
1086+
.iter()
1087+
.filter_map(|k| manifest.patches.get(*k).map(|r| (k.as_str(), &r.files)))
1088+
.collect();
1089+
let mut paths: Vec<&PathBuf> = keys.iter().flat_map(|k| &sets[*k]).collect();
1090+
paths.sort();
1091+
paths.dedup();
1092+
let mut kept: HashMap<&str, Vec<PathBuf>> = HashMap::new();
1093+
for path in paths {
1094+
let matched = select_installed_variants(path, &variants).await;
1095+
if matched.is_empty() {
1096+
for (key, _) in &variants {
1097+
kept.entry(*key).or_default().push(path.clone());
1098+
}
1099+
}
1100+
for idx in matched {
1101+
kept.entry(variants[idx].0).or_default().push(path.clone());
1102+
}
1103+
}
1104+
for (key, _) in &variants {
1105+
let paths = kept.remove(*key).unwrap_or_default();
1106+
narrowed.push((key.to_string(), paths));
1107+
}
1108+
}
1109+
sets.extend(narrowed);
10501110
}
10511111

10521112
/// Maven / Gradle: withhold the statement of every purl a build may still

‎crates/socket-patch-cli/src/ecosystem_dispatch.rs‎

Lines changed: 49 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -329,14 +329,10 @@ fn is_crates_io_copy(crate_dir: &std::path::Path) -> bool {
329329
/// after the first.
330330
fn merge_npm_copies(
331331
out: &mut HashMap<String, Vec<PathBuf>>,
332-
_purls: &[String],
332+
purls: &[String],
333333
packages: HashMap<String, Vec<CrawledPackage>>,
334334
) {
335-
for (purl, pkgs) in packages {
336-
for pkg in pkgs {
337-
push_path(out, purl.clone(), pkg.path);
338-
}
339-
}
335+
merge_each_copy(merge_variant_copies, out, purls, packages);
340336
}
341337

342338
/// `paths` in order with every path that resolves to an already-listed
@@ -486,15 +482,21 @@ async fn dispatch_find(
486482
eco = Ecosystem::Gem,
487483
options = options,
488484
silent = silent,
489-
crawler = RubyCrawler,
485+
crawler = GemCopiesCrawler,
490486
get_paths = get_gem_paths,
491487
using_label = "ruby gem paths",
492488
err_label = "Ruby gems",
493489
// RubyGems has per-platform release variants (`?platform=`); the
494490
// crawler emits the base PURL and the platform is resolved by
495491
// hashing the installed files, same as PyPI.
496492
purls_override = dedup_qualified_purls,
497-
on_match = variant_merge,
493+
// One gem home can hold a version's ruby-platform AND platform
494+
// builds; fold every copy in (#1092).
495+
on_match = |out: &mut HashMap<String, Vec<PathBuf>>,
496+
purls: &[String],
497+
packages: HashMap<String, Vec<CrawledPackage>>| {
498+
merge_each_copy(variant_merge, out, purls, packages)
499+
},
498500
);
499501

500502
scan_ecosystem!(
@@ -717,6 +719,43 @@ impl NpmCrawlSnapshot {
717719
}
718720
}
719721

722+
/// [`RubyCrawler`] for [`dispatch_find`], returning every installed copy of
723+
/// each gem in a gem home (the ruby-platform dir and each platform dir).
724+
struct GemCopiesCrawler;
725+
726+
impl GemCopiesCrawler {
727+
async fn get_gem_paths(
728+
&self,
729+
options: &CrawlerOptions,
730+
) -> Result<Vec<PathBuf>, std::io::Error> {
731+
RubyCrawler.get_gem_paths(options).await
732+
}
733+
734+
async fn find_by_purls(
735+
&self,
736+
gem_path: &std::path::Path,
737+
purls: &[String],
738+
) -> Result<HashMap<String, Vec<CrawledPackage>>, std::io::Error> {
739+
RubyCrawler.find_all_by_purls(gem_path, purls).await
740+
}
741+
}
742+
743+
/// Feed every copy in `packages` through `merge`, one copy at a time and in
744+
/// order, so a crawler that returns several copies per PURL reaches the
745+
/// one-copy [`MergeFn`]s unchanged.
746+
fn merge_each_copy(
747+
merge: MergeFn,
748+
out: &mut HashMap<String, Vec<PathBuf>>,
749+
purls: &[String],
750+
packages: HashMap<String, Vec<CrawledPackage>>,
751+
) {
752+
for (purl, copies) in packages {
753+
for pkg in copies {
754+
merge(out, purls, HashMap::from([(purl.clone(), pkg)]));
755+
}
756+
}
757+
}
758+
720759
/// [`NpmCrawler`] for [`dispatch_find`], answering its root discovery from
721760
/// an earlier crawl's roots when it has them.
722761
struct NpmRootsCrawler<'a> {
@@ -879,8 +918,8 @@ pub async fn find_manifest_package_copies_reusing(
879918
.await;
880919
let bases = dedup_qualified_purls(gem_purls);
881920
for store in &stores {
882-
if let Ok(found) = RubyCrawler.find_by_purls(store, &bases).await {
883-
merge_qualified(&mut copies, gem_purls, found);
921+
if let Ok(found) = RubyCrawler.find_all_by_purls(store, &bases).await {
922+
merge_each_copy(merge_qualified, &mut copies, gem_purls, found);
884923
}
885924
}
886925
}

0 commit comments

Comments
 (0)