Skip to content

Commit fb7e518

Browse files
fix(gem): crawl bundle-path roots in bundler precedence order, patch every coexisting copy, contain config-sourced roots (#222)
Post-merge audit follow-up on #218 (gem crawler flat-BUNDLE_PATH discovery), mirroring the #216 npm multi-copy precedent (0433bcb). 1. MULTI-COPY (most severe): bundler's scoped `<engine>/<abi>/gems` and flat `gems/` stores coexist under one root, each holding a REAL physical copy of the same gem@version — exactly the state #218's tests create. First-wins merging resolved the purl to ONE path, so apply patched one store and reported success while the other bundler loaded pristine (vulnerable) bytes. Fix mirrors #216: `find_all_packages_for_purls` now routes the release-variant ecosystems through an accumulating `merge_variant_copies` (reusing `push_path`, base-PURL keyed, precedence order kept) and apply's variant branch fans out per copy for gem — per-copy Applied events, `summary.applied` counts each copy, a copy matching no variant fails loudly. Rollback already carried every copy via `merge_qualified` and its per-path grouping; the new in-process suite pins both directions. PyPI/Maven deliberately keep the one-representative contract (the nuget `already_patched` double-patch regression from #216's second commit), as do all collapsing consumers (vendor/vex/setup/get/repair-vendor). scan --sync patches every copy too (it runs the real nested apply); scan's inventory stays purl-level, byte-identical to npm's crawl_all purl-dedup precedent. 2. PRECEDENCE: roots probed local-config > env > default vendor/bundle (bundler's real precedence; the old order was inverted), so first-representative consumers pick the copy bundler actually loads. 3. REGRESSION vs pre-#218: env/config roots no longer trip the `gem env` fallback early-return — only the historic project-local vendor/bundle probe keeps it. Default gems (rexml/json) live only in the DEFAULT/system gem homes, so an env-BUNDLE_PATH project gets those homes appended (deduped) again. 4. SECURITY: a config-sourced BUNDLE_PATH (committed .bundle/config = attacker-authored input, and a scan/apply WRITE-target root) must now, after ~ expansion and lexical normalization, stay contained in the project root — otherwise the root is skipped with a `gem_bundle_config_path_ignored` stderr warning naming the value. Windows rooted forms (`\evil`, `C:evil`) take the strict branch. Env-sourced BUNDLE_PATH stays trusted (user's own environment) but is normalized for dedup. `normalize_lexically` is hoisted from the composer crawler into utils::fs and shared. 5. `~` EXPANSION: a leading `~`/`~/...` in BUNDLE_PATH expands against home (bundler File.expand_path), env-injectable via the _with_env seams for hermetic tests. 6. BUNDLE_PATH__SYSTEM: `"true"` makes bundler ignore the recorded path — the config entry now parses as unset and the fallback finds the system gem homes. 7. TEST HERMETICITY: the six crawler_ruby_e2e tests that read ambient BUNDLE_PATH/BUNDLE_APP_CONFIG now route through the new `get_gem_paths_with_env` seam; a new e2e pins env-root + gem-env fallback coexistence (finding 3). 8. CLI_CONTRACT.md: the stale "gem inspects only <cwd>/vendor/bundle" claim replaced with the real root model, the containment policy, and the multi-copy behavior. TDD evidence (red -> green): dispatch-level `find_all_packages_for_purls_carries_every_gem_store_copy`, crawler `bundle_roots_probe_in_bundler_precedence_order`, and the new `in_process_gem_multicopy.rs` (real binary apply/rollback over a coexisting two-store tree) all failed on base — the flat copy stayed byte-for-byte VULNERABLE while apply reported success — and pass now. Live era-image proof (docker run --rm, socket-patch-test-gem-b1:gemx, bundler 1.17.3): the baked pre-fix binary on a coexist fixture reports status=success/applied=1 while the flat store's copy — loaded via real GEM_HOME resolution in the container — still evaluates VULNERABLE; the fixed binary reports applied=2 and both stores load FIXED. Gates: touched files rustfmt-clean; cargo clippy --workspace --all-features -D warnings clean; core --lib 2413 passed; cli --lib 430 passed; crawler_ruby_e2e 25, crawler_composer_e2e 32, crawlers_empty_paths_e2e 13; e2e_gem hermetic 8; in-process gem+npm multicopy suites; cli_gem_variant_mismatch_policy 6; docker_e2e_gem, docker_e2e_vendor_gem, docker_e2e_pypi, docker_e2e_maven all green. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent a2f07e0 commit fb7e518

12 files changed

Lines changed: 2267 additions & 287 deletions

File tree

crates/socket-patch-cli/CLI_CONTRACT.md

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,12 +278,44 @@ the model is **not uniform** today:
278278
One repo-root invocation discovers and configures every member. *Single level only* — see property
279279
9's nested-workspace gap.
280280
- **cwd-only (single project):** gem, pypi, composer. The crawler inspects only the project
281-
rooted at `--cwd` (e.g. gem looks at `<cwd>/vendor/bundle/...`; pypi at `<cwd>/.venv`); it does **not**
281+
rooted at `--cwd` (pypi looks at `<cwd>/.venv`; composer at the vendor tree); it does **not**
282282
descend into sibling subprojects. A monorepo with several independent lockfiles in subdirectories
283283
(`backend/Gemfile.lock` + `frontend/Gemfile.lock`, multiple `.venv`, multiple `go.mod` /
284284
`composer.json`) is handled by invoking the tool **once per subproject** (`--cwd` each), as a
285285
per-directory install hook would.
286286

287+
*Gem install roots (a refinement of "cwd-only", not an exception to the one-project model):* the
288+
crawler probes the project's Bundler install roots in **bundler's own precedence order** — the app
289+
config file's `BUNDLE_PATH:` (`$BUNDLE_APP_CONFIG/config`, else `<cwd>/.bundle/config` — what
290+
`bundle config set --local path` records), then the **`BUNDLE_PATH` environment variable**, then the
291+
default `<cwd>/vendor/bundle` — each in both store layouts bundler produces (scoped
292+
`<root>/<engine>/<abi>/gems/` and flat `<root>/gems/`). The env variable is the user's own machine
293+
state, so it is honored verbatim (it may point outside `--cwd`; a leading `~` expands against home);
294+
the **config file is typically committed — untrusted input — so a config-sourced root that resolves
295+
outside the project root is skipped** (`BUNDLE_PATH__SYSTEM: "true"` likewise drops the recorded
296+
path, as bundler itself ignores it). The skip is surfaced per the run-warning conventions: a
297+
`gem_bundle_config_path_ignored` entry in the run-level `warnings[]` of `scan`/`apply` `--json`
298+
envelopes (detail names the config value and the env-`BUNDLE_PATH` remedy), and one stderr
299+
`Warning (gem_bundle_config_path_ignored): …` line on the human path, gated on `!--silent`
300+
(`--silent` = errors only). Explicit env/config roots only count when `--cwd` holds a Bundler
301+
manifest/lockfile. When the default `vendor/bundle` root holds no store, the gem homes `gem env`
302+
reports are appended (default gems like rexml/json only ever live there). When several roots hold
303+
**coexisting physical copies of one `gem@version`** (bundler-2's scoped store beside bundler-1's
304+
flat store), `apply`/`rollback` patch/restore **every copy** — one summary event per copy,
305+
mirroring npm's multi-copy fan-out — while single-representative consumers (`get`, `vendor`,
306+
`setup`, `vex`) use the highest-precedence copy.
307+
308+
*Copy classes (additive to the multi-copy vocabulary):* a copy under a **bundle-path store**
309+
(config/env/default root) is PRIMARY — a variant mismatch or write failure there fails the run,
310+
as always. A copy in a **`gem env` fallback home** (rvm `@global`, `--user-install`, system gem
311+
dirs — shared, often root-owned) is patched too when it matches and is writable, but becomes
312+
BEST-EFFORT once at least one bundle-store copy applied: its mismatch/write failure surfaces as a
313+
non-fatal `skipped` event (`errorCode: gem_fallback_home_skipped`, detail names the copy's path
314+
and reason; gated stderr twin on the human path) instead of failing a run whose loaded copy is
315+
patched. With **no** bundle-store copy (the historic fallback-only layout, and every `--global`
316+
run) the fallback-home copy IS the primary install and keeps loud-fail parity with pre-bundle-path
317+
`apply`.
318+
287319
**Intended (gap):** the cwd-only ecosystems *should* also auto-discover per-subproject lockfiles when
288320
run from the repo root, matching the npm workspace model. The npm-vs-others asymmetry is a known
289321
defect, guarded by the `#[ignore]`d gap pin

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

Lines changed: 355 additions & 122 deletions
Large diffs are not rendered by default.

crates/socket-patch-cli/src/commands/scan/gc.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -624,11 +624,9 @@ mod tests {
624624
let (manifest_path, socket_dir, blob_path) =
625625
seed_manifest_with_blob(tmp.path(), "pkg:npm/gone@1.0.0", &after_hash);
626626

627-
let _holder = socket_patch_core::patch::apply_lock::acquire(
628-
&socket_dir,
629-
std::time::Duration::ZERO,
630-
)
631-
.expect("test holder must win the fresh lock");
627+
let _holder =
628+
socket_patch_core::patch::apply_lock::acquire(&socket_dir, std::time::Duration::ZERO)
629+
.expect("test holder must win the fresh lock");
632630

633631
let scanned: HashSet<String> = HashSet::new();
634632
let gc = run_apply_gc(

crates/socket-patch-cli/src/commands/scan/mod.rs

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@ use socket_patch_core::api::client::{
1111
build_proxy_fallback_client, get_api_client_with_overrides, is_fallback_candidate,
1212
};
1313
use socket_patch_core::api::types::{BatchPackagePatches, PatchSearchResult};
14-
use socket_patch_core::crawlers::{CrawlerOptions, Ecosystem};
14+
use socket_patch_core::crawlers::ruby_crawler::config_path_ignored_warning;
15+
use socket_patch_core::crawlers::{CrawlerOptions, Ecosystem, RubyCrawler};
1516
use socket_patch_core::manifest::operations::read_manifest;
1617
use socket_patch_core::manifest::schema::PatchManifest;
1718
use socket_patch_core::telemetry::{track_patch_scan_failed, track_patch_scanned};
@@ -1451,7 +1452,33 @@ pub async fn run(mut args: ScanArgs) -> i32 {
14511452
// empty) and a stderr line on the human path; exit code and `status`
14521453
// stay deliberately unchanged (same posture as hosted refusals, which
14531454
// exit 0 with `redirected: 0`).
1454-
let layout_refusals = unsupported_layout_warnings(&lockfile_only);
1455+
let mut layout_refusals = unsupported_layout_warnings(&lockfile_only);
1456+
// Config-sourced gem bundle root refused by the crawler's containment
1457+
// guard (a committed `.bundle/config` whose BUNDLE_PATH resolves
1458+
// outside the project — untrusted input that would otherwise become a
1459+
// scan/apply WRITE-target root). The crawl above consulted and
1460+
// silently skipped it; surface the skip on the SAME run-level channel
1461+
// as the layout refusals (JSON `warnings[]` on both the zero-package
1462+
// and ≥1-package envelopes; a gated stderr line on the human path).
1463+
// Scoped like the crawl that hit it: local mode, with gem not filtered
1464+
// out by `--ecosystems`. Cheap re-probe: filesystem only, no `gem env`
1465+
// shell-out.
1466+
if !crawler_options.global
1467+
&& crawler_options.global_prefix.is_none()
1468+
&& args
1469+
.common
1470+
.ecosystems
1471+
.as_ref()
1472+
.is_none_or(|list| list.iter().any(|e| e == Ecosystem::Gem.cli_name()))
1473+
{
1474+
if let Some(value) = RubyCrawler::discover_bundle_stores(&args.common.cwd)
1475+
.await
1476+
.skipped_config_path
1477+
{
1478+
let (code, detail) = config_path_ignored_warning(&value);
1479+
layout_refusals.push((code.to_string(), detail));
1480+
}
1481+
}
14551482
if !lockfile_only.packages.is_empty() {
14561483
for pkg in &lockfile_only.packages {
14571484
if let Some(eco) = Ecosystem::from_purl(&pkg.purl) {

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

Lines changed: 142 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,30 @@ fn merge_first_wins(
153153
}
154154
}
155155

156+
/// Release-variant merge for the APPLY path: keyed by the crawler-returned
157+
/// base PURL (apply's variant loop groups by base), accumulating EVERY
158+
/// distinct path discovered across the ecosystem's source roots in
159+
/// discovery (precedence) order. The gem crawler legitimately discovers
160+
/// several coexisting stores holding REAL physical copies of one
161+
/// `gem@version` (bundler's scoped `<engine>/<abi>/gems` beside the flat
162+
/// `gems/` layout, or an env `BUNDLE_PATH` store) — first-wins here
163+
/// dropped the second copy, so apply patched one store and reported
164+
/// success while the other bundler loaded pristine bytes (the gem sibling
165+
/// of the npm multi-copy P0). Collapsing consumers still take the first
166+
/// (highest-precedence) path, so this changes nothing for
167+
/// vendor/vex/setup/get/repair-vendor; apply fans out per-copy for gem
168+
/// only (PyPI/Maven keep their one-install-dir contract — see the apply
169+
/// variant loop).
170+
fn merge_variant_copies(
171+
out: &mut HashMap<String, Vec<PathBuf>>,
172+
_purls: &[String],
173+
packages: HashMap<String, CrawledPackage>,
174+
) {
175+
for (purl, pkg) in packages {
176+
push_path(out, purl, pkg.path);
177+
}
178+
}
179+
156180
/// npm merge: the npm crawler returns EVERY physical copy of each PURL
157181
/// (nested duplicates, diamonds, `file:` dups), so fold every path in.
158182
/// This is the type shape that carries the second copy the old
@@ -382,7 +406,14 @@ pub async fn find_all_packages_for_purls(
382406
options: &CrawlerOptions,
383407
silent: bool,
384408
) -> HashMap<String, Vec<PathBuf>> {
385-
dispatch_find(partitioned, options, silent, merge_first_wins).await
409+
// Release-variant ecosystems accumulate every distinct discovered copy
410+
// (base-PURL keyed) instead of first-wins: the gem crawler surfaces
411+
// coexisting bundler stores whose copies apply must ALL patch. The
412+
// rollback variant below gets the same multi-copy carry from
413+
// `merge_qualified`'s `push_path`. Single-copy ecosystems keep true
414+
// first-wins via their own `merge_first_wins` wiring in
415+
// `dispatch_find`.
416+
dispatch_find(partitioned, options, silent, merge_variant_copies).await
386417
}
387418

388419
/// Multi-copy variant of `find_packages_for_rollback` (qualified-aware
@@ -535,7 +566,10 @@ mod tests {
535566
let mut out: HashMap<String, Vec<PathBuf>> = HashMap::new();
536567
merge_first_wins(&mut out, &[], packages(&[("pkg:cargo/foo@1.0", "/same")]));
537568
merge_first_wins(&mut out, &[], packages(&[("pkg:cargo/foo@1.0", "/same")]));
538-
assert_eq!(out.get("pkg:cargo/foo@1.0"), Some(&vec![PathBuf::from("/same")]));
569+
assert_eq!(
570+
out.get("pkg:cargo/foo@1.0"),
571+
Some(&vec![PathBuf::from("/same")])
572+
);
539573
}
540574

541575
#[test]
@@ -545,9 +579,20 @@ mod tests {
545579
// the first is kept, so apply does not double-patch (the regression
546580
// that broke docker_e2e_nuget with a spurious `already_patched` skip).
547581
let mut out: HashMap<String, Vec<PathBuf>> = HashMap::new();
548-
merge_first_wins(&mut out, &[], packages(&[("pkg:nuget/foo@1.0", "/global/foo")]));
549-
merge_first_wins(&mut out, &[], packages(&[("pkg:nuget/foo@1.0", "/local/foo")]));
550-
assert_eq!(out.get("pkg:nuget/foo@1.0"), Some(&vec![PathBuf::from("/global/foo")]));
582+
merge_first_wins(
583+
&mut out,
584+
&[],
585+
packages(&[("pkg:nuget/foo@1.0", "/global/foo")]),
586+
);
587+
merge_first_wins(
588+
&mut out,
589+
&[],
590+
packages(&[("pkg:nuget/foo@1.0", "/local/foo")]),
591+
);
592+
assert_eq!(
593+
out.get("pkg:nuget/foo@1.0"),
594+
Some(&vec![PathBuf::from("/global/foo")])
595+
);
551596
}
552597

553598
#[test]
@@ -588,6 +633,40 @@ mod tests {
588633
);
589634
}
590635

636+
// ---- merge_variant_copies ----------------------------------------------
637+
638+
#[test]
639+
fn merge_variant_copies_accumulates_distinct_store_copies() {
640+
// The gem crawler resolves the same base PURL from two coexisting
641+
// stores (scoped + flat) across the macro's per-source-path calls;
642+
// both copies must be carried, discovery (precedence) order kept,
643+
// and an identical re-observed path deduped.
644+
let mut out: HashMap<String, Vec<PathBuf>> = HashMap::new();
645+
merge_variant_copies(
646+
&mut out,
647+
&[],
648+
packages(&[("pkg:gem/rack@3.1.0", "/scoped/rack-3.1.0")]),
649+
);
650+
merge_variant_copies(
651+
&mut out,
652+
&[],
653+
packages(&[("pkg:gem/rack@3.1.0", "/flat/rack-3.1.0")]),
654+
);
655+
merge_variant_copies(
656+
&mut out,
657+
&[],
658+
packages(&[("pkg:gem/rack@3.1.0", "/flat/rack-3.1.0")]),
659+
);
660+
assert_eq!(
661+
out.get("pkg:gem/rack@3.1.0"),
662+
Some(&vec![
663+
PathBuf::from("/scoped/rack-3.1.0"),
664+
PathBuf::from("/flat/rack-3.1.0"),
665+
]),
666+
"every distinct copy carried, precedence order kept, dup deduped"
667+
);
668+
}
669+
591670
// ---- merge_qualified --------------------------------------------------
592671

593672
#[test]
@@ -758,7 +837,10 @@ mod tests {
758837
merge_first_wins(&mut out, &[], packages(&[("pkg:gem/baz@3.0", "/c")]));
759838
assert_eq!(out.len(), 3);
760839
assert_eq!(out.get("pkg:npm/foo@1.0"), Some(&vec![PathBuf::from("/a")]));
761-
assert_eq!(out.get("pkg:cargo/bar@2.0"), Some(&vec![PathBuf::from("/b")]));
840+
assert_eq!(
841+
out.get("pkg:cargo/bar@2.0"),
842+
Some(&vec![PathBuf::from("/b")])
843+
);
762844
assert_eq!(out.get("pkg:gem/baz@3.0"), Some(&vec![PathBuf::from("/c")]));
763845
}
764846

@@ -995,21 +1077,68 @@ mod tests {
9951077
.await;
9961078

9971079
let copies = out.get("pkg:npm/dup@1.0.0").expect("dup resolves");
998-
assert_eq!(copies.len(), 2, "both copies must be carried; got {copies:?}");
1080+
assert_eq!(
1081+
copies.len(),
1082+
2,
1083+
"both copies must be carried; got {copies:?}"
1084+
);
9991085
assert_eq!(copies[0], root_copy, "root copy first");
10001086
assert!(copies.contains(&nested_copy), "nested copy must be present");
10011087

10021088
// The collapsing wrapper (used by vendor/vex/setup/get) keeps the
10031089
// old one-path contract: exactly the root-preferred representative.
1004-
let single = find_packages_for_purls(
1005-
&partitioned,
1006-
&local_options(tmp.path().to_path_buf()),
1007-
true,
1008-
)
1009-
.await;
1090+
let single =
1091+
find_packages_for_purls(&partitioned, &local_options(tmp.path().to_path_buf()), true)
1092+
.await;
10101093
assert_eq!(single.get("pkg:npm/dup@1.0.0"), Some(&root_copy));
10111094
}
10121095

1096+
/// Multi-copy P0 for gem (mirrors the npm test above): bundler's scoped
1097+
/// (`<engine>/<abi>/gems`) and flat (`gems/`) store layouts coexist under
1098+
/// one `vendor/bundle` root — a bundler-2 `--path` install beside a
1099+
/// bundler-1 env install — each holding a REAL physical copy of the same
1100+
/// `gem@version`. `find_all_packages_for_purls` (apply's resolver) must
1101+
/// carry BOTH copies, highest-precedence store first. First-wins merging
1102+
/// resolved ONE copy, apply patched it and reported success while the
1103+
/// other bundler loaded the pristine (vulnerable) sibling.
1104+
#[tokio::test]
1105+
async fn find_all_packages_for_purls_carries_every_gem_store_copy() {
1106+
let tmp = tempfile::tempdir().unwrap();
1107+
// No Gemfile on purpose: env/config bundle roots are manifest-gated,
1108+
// so an ambient BUNDLE_PATH on the dev machine cannot perturb this
1109+
// test; the implicit vendor/bundle probe is ungated.
1110+
let bundle = tmp.path().join("vendor").join("bundle");
1111+
let scoped_copy = bundle
1112+
.join("ruby")
1113+
.join("3.2.0")
1114+
.join("gems")
1115+
.join("rack-3.1.0");
1116+
let flat_copy = bundle.join("gems").join("rack-3.1.0");
1117+
std::fs::create_dir_all(scoped_copy.join("lib")).unwrap();
1118+
std::fs::create_dir_all(flat_copy.join("lib")).unwrap();
1119+
// The specifications/ sibling marks the flat layout as a real gem home.
1120+
std::fs::create_dir_all(bundle.join("specifications")).unwrap();
1121+
1122+
let purl = "pkg:gem/rack@3.1.0".to_string();
1123+
let partitioned = partition_purls(std::slice::from_ref(&purl), None);
1124+
let opts = local_options(tmp.path().to_path_buf());
1125+
1126+
let out = find_all_packages_for_purls(&partitioned, &opts, true).await;
1127+
let copies = out.get(&purl).expect("gem resolves");
1128+
assert_eq!(
1129+
copies.len(),
1130+
2,
1131+
"both coexisting store copies must be carried; got {copies:?}"
1132+
);
1133+
assert_eq!(copies[0], scoped_copy, "scoped store copy first");
1134+
assert!(copies.contains(&flat_copy), "flat store copy present");
1135+
1136+
// The collapsing wrapper (vendor/vex/setup/get/repair-vendor) keeps
1137+
// the one-representative contract: the first store's copy.
1138+
let single = find_packages_for_purls(&partitioned, &opts, true).await;
1139+
assert_eq!(single.get(&purl), Some(&scoped_copy));
1140+
}
1141+
10131142
#[tokio::test]
10141143
async fn find_packages_for_purls_skips_version_mismatch() {
10151144
// The crawler only matches an installed dir whose version equals the

0 commit comments

Comments
 (0)