From 7e2022704e981b204ab95e9163ab457c44b7c0ea Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 19 Aug 2026 16:14:46 -0400 Subject: [PATCH 1/2] fix(gem): crawl bundle-path roots in bundler precedence order, patch every coexisting copy, contain config-sourced roots MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 `//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 /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 --- crates/socket-patch-cli/CLI_CONTRACT.md | 20 +- crates/socket-patch-cli/src/commands/apply.rs | 259 +++--- .../src/ecosystem_dispatch.rs | 155 +++- .../tests/in_process_gem_multicopy.rs | 232 ++++++ .../src/crawlers/composer_crawler.rs | 38 +- .../src/crawlers/ruby_crawler.rs | 782 +++++++++++++++--- crates/socket-patch-core/src/utils/fs.rs | 37 + .../tests/crawler_ruby_e2e.rs | 104 ++- 8 files changed, 1362 insertions(+), 265 deletions(-) create mode 100644 crates/socket-patch-cli/tests/in_process_gem_multicopy.rs diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 75fef147..d50406bf 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -278,12 +278,30 @@ the model is **not uniform** today: One repo-root invocation discovers and configures every member. *Single level only* — see property 9's nested-workspace gap. - **cwd-only (single project):** gem, pypi, composer. The crawler inspects only the project - rooted at `--cwd` (e.g. gem looks at `/vendor/bundle/...`; pypi at `/.venv`); it does **not** + rooted at `--cwd` (pypi looks at `/.venv`; composer at the vendor tree); it does **not** descend into sibling subprojects. A monorepo with several independent lockfiles in subdirectories (`backend/Gemfile.lock` + `frontend/Gemfile.lock`, multiple `.venv`, multiple `go.mod` / `composer.json`) is handled by invoking the tool **once per subproject** (`--cwd` each), as a per-directory install hook would. + *Gem install roots (a refinement of "cwd-only", not an exception to the one-project model):* the + crawler probes the project's Bundler install roots in **bundler's own precedence order** — the app + config file's `BUNDLE_PATH:` (`$BUNDLE_APP_CONFIG/config`, else `/.bundle/config` — what + `bundle config set --local path` records), then the **`BUNDLE_PATH` environment variable**, then the + default `/vendor/bundle` — each in both store layouts bundler produces (scoped + `///gems/` and flat `/gems/`). The env variable is the user's own machine + state, so it is honored verbatim (it may point outside `--cwd`; a leading `~` expands against home); + the **config file is typically committed — untrusted input — so a config-sourced root that resolves + outside the project root is skipped with a `gem_bundle_config_path_ignored` stderr warning** + (`BUNDLE_PATH__SYSTEM: "true"` likewise drops the recorded path, as bundler itself ignores it). + Explicit env/config roots only count when `--cwd` holds a Bundler manifest/lockfile. When the + default `vendor/bundle` root holds no store, the gem homes `gem env` reports are appended (default + gems like rexml/json only ever live there). 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`, `setup`, `vex`) use the + highest-precedence copy. + **Intended (gap):** the cwd-only ecosystems *should* also auto-discover per-subproject lockfiles when run from the repo root, matching the npm workspace model. The npm-vs-others asymmetry is a known defect, guarded by the `#[ignore]`d gap pin diff --git a/crates/socket-patch-cli/src/commands/apply.rs b/crates/socket-patch-cli/src/commands/apply.rs index dee1d363..b58431cc 100644 --- a/crates/socket-patch-cli/src/commands/apply.rs +++ b/crates/socket-patch-cli/src/commands/apply.rs @@ -1227,10 +1227,13 @@ async fn apply_patches_inner( let mut applied_base_purls: HashSet = HashSet::new(); for (purl, pkg_paths) in &all_packages { - // Release-variant ecosystems install exactly one directory per - // `package@version` (the variants are jars/wheels inside it), so a - // representative path is all the variant branch needs. npm's branch - // below iterates every physical copy. + // The paths carry every resolved physical copy. Release-variant + // ecosystems install one directory per `package@version` (the + // variants are jars/wheels inside it) — EXCEPT gem, where bundler's + // coexisting store layouts (the scoped `//gems` store + // bundler >= 2 loads beside the flat `gems/` store bundler 1 loads) + // hold genuinely distinct physical copies of one `gem@version`. + // npm's branch below iterates every physical copy. let pkg_path = pkg_paths .first() .expect("all_packages only holds PURLs with at least one resolved copy"); @@ -1253,128 +1256,158 @@ async fn apply_patches_inner( { continue; } - let mut applied = false; - // Did at least one variant reach `apply_package_patch`? A - // variant reaches it only after passing the first-file - // installed-distribution check (or under `--force`), so an - // attempted variant *is* the installed distribution — it must - // not be reported as "package_not_installed" even if the patch - // itself then fails. Tracks the "matched but failed" case so the - // failure message is honest and `unmatched` stays accurate. - let mut attempted = false; - - for variant_purl in &variants { - let patch = match manifest.patches.get(variant_purl) { - Some(p) => p, - None => continue, + + // Patch EVERY coexisting gem store copy (the npm multi-copy + // precedent): leaving the other store pristine is a silent + // false "applied" for whichever bundler loads it, and the + // per-copy results below make the JSON summary count each + // patched copy — the signal a second copy exists. PyPI/Maven + // keep the one-representative contract: their crawlers resolve + // one install dir per version, and a second path can only + // alias the same logical install (re-patching it would produce + // the spurious `already_patched` double-patch the nuget + // first-wins restoration fixed). + let copy_paths: &[PathBuf] = + if matches!(Ecosystem::from_purl(purl), Some(Ecosystem::Gem)) { + pkg_paths.as_slice() + } else { + std::slice::from_ref(pkg_path) }; - // Check the representative file's status (skip when - // --force). A mismatch *or* a missing file means this - // variant's distribution isn't the one on disk, so skip it — - // attempting it would only produce a spurious failure. - // Mirrors `select_installed_variants`, used by rollback/get. - // - // Exempt only an UNQUALIFIED singleton (the common bare - // `pkg:gem/name@ver` manifest key): it names no - // distribution, so a mismatch there is locally-modified - // bytes on the only candidate — exactly what the default - // mismatch policy covers (warn + apply the full verified - // patched content; `--strict` refuses). Gating it made that - // documented policy unreachable for gem/pypi/maven, so it - // falls through to `apply_package_patch`, whose - // `MismatchPolicy` handles it like the npm branch below. - // A QUALIFIED singleton (`?platform=`…) stays gated: it - // names one specific distribution, and — because the - // crawler drops the installed dir's platform suffix (see - // ruby_crawler's `parse_dir_name_version`) — this hash - // check is the ONLY thing resolving whether that - // distribution is the one on disk. Falling through would - // 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) { - let first_status = match representative_file(&patch.files) { - Some((file_name, file_info)) => Some( - verify_file_patch(pkg_path, file_name, file_info) - .await - .status, - ), - None => None, + let mut any_copy_applied = false; + for pkg_path in copy_paths { + let mut applied = false; + // Did at least one variant reach `apply_package_patch`? A + // variant reaches it only after passing the first-file + // installed-distribution check (or under `--force`), so an + // attempted variant *is* the installed distribution — it must + // not be reported as "package_not_installed" even if the patch + // itself then fails. Tracks the "matched but failed" case so the + // failure message is honest and `unmatched` stays accurate. + // Both are PER COPY: a copy that matches no variant must fail + // loudly even when a sibling copy applied cleanly — that copy + // is a real on-disk gem some bundler loads. + let mut attempted = false; + + for variant_purl in &variants { + let patch = match manifest.patches.get(variant_purl) { + Some(p) => p, + None => continue, }; - if !variant_matches_installed(first_status.as_ref()) { - continue; + + // Check the representative file's status (skip when + // --force). A mismatch *or* a missing file means this + // variant's distribution isn't the one on disk, so skip it — + // attempting it would only produce a spurious failure. + // Mirrors `select_installed_variants`, used by rollback/get. + // + // Exempt only an UNQUALIFIED singleton (the common bare + // `pkg:gem/name@ver` manifest key): it names no + // distribution, so a mismatch there is locally-modified + // bytes on the only candidate — exactly what the default + // mismatch policy covers (warn + apply the full verified + // patched content; `--strict` refuses). Gating it made that + // documented policy unreachable for gem/pypi/maven, so it + // falls through to `apply_package_patch`, whose + // `MismatchPolicy` handles it like the npm branch below. + // A QUALIFIED singleton (`?platform=`…) stays gated: it + // names one specific distribution, and — because the + // crawler drops the installed dir's platform suffix (see + // ruby_crawler's `parse_dir_name_version`) — this hash + // check is the ONLY thing resolving whether that + // distribution is the one on disk. Falling through would + // 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) { + let first_status = match representative_file(&patch.files) { + Some((file_name, file_info)) => Some( + verify_file_patch(pkg_path, file_name, file_info) + .await + .status, + ), + None => None, + }; + if !variant_matches_installed(first_status.as_ref()) { + continue; + } } - } - attempted = true; - let result = apply_package_patch( - variant_purl, - pkg_path, - &patch.files, - &sources, - Some(&patch.uuid), - args.common.dry_run, - policy, - ) - .await; + attempted = true; + let result = apply_package_patch( + variant_purl, + pkg_path, + &patch.files, + &sources, + Some(&patch.uuid), + args.common.dry_run, + policy, + ) + .await; + + warn_mismatch_overwrites(&result, &args.common); + // A variant that reached apply is the installed distribution + // (it passed the first-file check, or `--force` bypassed it), + // so record it as matched whether or not the patch succeeded. + // Otherwise a variant that matched on disk but failed to patch + // would land in `unmatched` and be misreported by the run + // loop as a `package_not_installed` Skipped event — on top of + // the Failed event it already emits. Mirrors the npm branch + // below, which always marks an attempted PURL matched. + matched_manifest_purls.insert(variant_purl.clone()); + if result.success { + applied = true; + // No `break`: apply *every* matching variant. PyPI/gem + // have exactly one installed distribution (the rest + // hash-mismatch and were skipped above), so this + // applies a single variant for them; Maven's coexisting + // classifier jars each get patched. + } else { + // A variant that reached apply IS the installed + // distribution, so a failure here is a real apply + // failure — flag it even if a *sibling* variant of the + // same base succeeds (Maven's coexisting classifier + // jars, or any base where `--force` attempts every + // variant). Mirrors the npm branch below and the + // rollback loop, which mark `has_errors` on every failed + // result; without this a partial multi-variant failure + // would leave a `failed` event in the envelope while the + // command still reported `success` / exit 0. + has_errors = true; + if !args.common.silent && !args.common.json { + eprintln!( + "Failed to patch {}: {}", + variant_purl, + result.error.as_deref().unwrap_or("unknown error") + ); + } + } + results.push(result); + } - warn_mismatch_overwrites(&result, &args.common); - // A variant that reached apply is the installed distribution - // (it passed the first-file check, or `--force` bypassed it), - // so record it as matched whether or not the patch succeeded. - // Otherwise a variant that matched on disk but failed to patch - // would land in `unmatched` and be misreported by the run - // loop as a `package_not_installed` Skipped event — on top of - // the Failed event it already emits. Mirrors the npm branch - // below, which always marks an attempted PURL matched. - matched_manifest_purls.insert(variant_purl.clone()); - if result.success { - applied = true; - // No `break`: apply *every* matching variant. PyPI/gem - // have exactly one installed distribution (the rest - // hash-mismatch and were skipped above), so this - // applies a single variant for them; Maven's coexisting - // classifier jars each get patched. + if applied { + any_copy_applied = true; } else { - // A variant that reached apply IS the installed - // distribution, so a failure here is a real apply - // failure — flag it even if a *sibling* variant of the - // same base succeeds (Maven's coexisting classifier - // jars, or any base where `--force` attempts every - // variant). Mirrors the npm branch below and the - // rollback loop, which mark `has_errors` on every failed - // result; without this a partial multi-variant failure - // would leave a `failed` event in the envelope while the - // command still reported `success` / exit 0. + // Nothing applied for this copy. `has_errors` was already set + // per-variant above when a variant was attempted-but-failed; + // set it here too for the no-variant-attempted case so both + // paths fail the command — per copy, so a second store copy + // that matches no variant fails loudly instead of silently + // staying vulnerable behind a sibling copy's success. has_errors = true; - if !args.common.silent && !args.common.json { - eprintln!( - "Failed to patch {}: {}", - variant_purl, - result.error.as_deref().unwrap_or("unknown error") - ); + if !attempted && !args.common.silent && !args.common.json { + // No variant matched the installed distribution at all — + // the package on disk isn't any known release variant. + // (Attempted-but-failed variants already printed their own + // per-variant failure line above.) + eprintln!("Failed to patch {base_purl}: no matching variant found"); } } - results.push(result); } - if applied { + if any_copy_applied { applied_base_purls.insert(base_purl.clone()); - } else { - // Nothing applied for this base. `has_errors` was already set - // per-variant above when a variant was attempted-but-failed; - // set it here too for the no-variant-attempted case so both - // paths fail the command. - has_errors = true; - if !attempted && !args.common.silent && !args.common.json { - // No variant matched the installed distribution at all — - // the package on disk isn't any known release variant. - // (Attempted-but-failed variants already printed their own - // per-variant failure line above.) - eprintln!("Failed to patch {base_purl}: no matching variant found"); - } } } else { // Vendor-owned purl: already reported by the synthesized diff --git a/crates/socket-patch-cli/src/ecosystem_dispatch.rs b/crates/socket-patch-cli/src/ecosystem_dispatch.rs index d0cb42df..c4bcd8e9 100644 --- a/crates/socket-patch-cli/src/ecosystem_dispatch.rs +++ b/crates/socket-patch-cli/src/ecosystem_dispatch.rs @@ -153,6 +153,30 @@ fn merge_first_wins( } } +/// Release-variant merge for the APPLY path: keyed by the crawler-returned +/// base PURL (apply's variant loop groups by base), accumulating EVERY +/// distinct path discovered across the ecosystem's source roots in +/// discovery (precedence) order. The gem crawler legitimately discovers +/// several coexisting stores holding REAL physical copies of one +/// `gem@version` (bundler's scoped `//gems` beside the flat +/// `gems/` layout, or an env `BUNDLE_PATH` store) — first-wins here +/// dropped the second copy, so apply patched one store and reported +/// success while the other bundler loaded pristine bytes (the gem sibling +/// of the npm multi-copy P0). Collapsing consumers still take the first +/// (highest-precedence) path, so this changes nothing for +/// vendor/vex/setup/get/repair-vendor; apply fans out per-copy for gem +/// only (PyPI/Maven keep their one-install-dir contract — see the apply +/// variant loop). +fn merge_variant_copies( + out: &mut HashMap>, + _purls: &[String], + packages: HashMap, +) { + for (purl, pkg) in packages { + push_path(out, purl, pkg.path); + } +} + /// npm merge: the npm crawler returns EVERY physical copy of each PURL /// (nested duplicates, diamonds, `file:` dups), so fold every path in. /// This is the type shape that carries the second copy the old @@ -382,7 +406,14 @@ pub async fn find_all_packages_for_purls( options: &CrawlerOptions, silent: bool, ) -> HashMap> { - dispatch_find(partitioned, options, silent, merge_first_wins).await + // Release-variant ecosystems accumulate every distinct discovered copy + // (base-PURL keyed) instead of first-wins: the gem crawler surfaces + // coexisting bundler stores whose copies apply must ALL patch. The + // rollback variant below gets the same multi-copy carry from + // `merge_qualified`'s `push_path`. Single-copy ecosystems keep true + // first-wins via their own `merge_first_wins` wiring in + // `dispatch_find`. + dispatch_find(partitioned, options, silent, merge_variant_copies).await } /// Multi-copy variant of `find_packages_for_rollback` (qualified-aware @@ -535,7 +566,10 @@ mod tests { let mut out: HashMap> = HashMap::new(); merge_first_wins(&mut out, &[], packages(&[("pkg:cargo/foo@1.0", "/same")])); merge_first_wins(&mut out, &[], packages(&[("pkg:cargo/foo@1.0", "/same")])); - assert_eq!(out.get("pkg:cargo/foo@1.0"), Some(&vec![PathBuf::from("/same")])); + assert_eq!( + out.get("pkg:cargo/foo@1.0"), + Some(&vec![PathBuf::from("/same")]) + ); } #[test] @@ -545,9 +579,20 @@ mod tests { // the first is kept, so apply does not double-patch (the regression // that broke docker_e2e_nuget with a spurious `already_patched` skip). let mut out: HashMap> = HashMap::new(); - merge_first_wins(&mut out, &[], packages(&[("pkg:nuget/foo@1.0", "/global/foo")])); - merge_first_wins(&mut out, &[], packages(&[("pkg:nuget/foo@1.0", "/local/foo")])); - assert_eq!(out.get("pkg:nuget/foo@1.0"), Some(&vec![PathBuf::from("/global/foo")])); + merge_first_wins( + &mut out, + &[], + packages(&[("pkg:nuget/foo@1.0", "/global/foo")]), + ); + merge_first_wins( + &mut out, + &[], + packages(&[("pkg:nuget/foo@1.0", "/local/foo")]), + ); + assert_eq!( + out.get("pkg:nuget/foo@1.0"), + Some(&vec![PathBuf::from("/global/foo")]) + ); } #[test] @@ -588,6 +633,40 @@ mod tests { ); } + // ---- merge_variant_copies ---------------------------------------------- + + #[test] + fn merge_variant_copies_accumulates_distinct_store_copies() { + // The gem crawler resolves the same base PURL from two coexisting + // stores (scoped + flat) across the macro's per-source-path calls; + // both copies must be carried, discovery (precedence) order kept, + // and an identical re-observed path deduped. + let mut out: HashMap> = HashMap::new(); + merge_variant_copies( + &mut out, + &[], + packages(&[("pkg:gem/rack@3.1.0", "/scoped/rack-3.1.0")]), + ); + merge_variant_copies( + &mut out, + &[], + packages(&[("pkg:gem/rack@3.1.0", "/flat/rack-3.1.0")]), + ); + merge_variant_copies( + &mut out, + &[], + packages(&[("pkg:gem/rack@3.1.0", "/flat/rack-3.1.0")]), + ); + assert_eq!( + out.get("pkg:gem/rack@3.1.0"), + Some(&vec![ + PathBuf::from("/scoped/rack-3.1.0"), + PathBuf::from("/flat/rack-3.1.0"), + ]), + "every distinct copy carried, precedence order kept, dup deduped" + ); + } + // ---- merge_qualified -------------------------------------------------- #[test] @@ -758,7 +837,10 @@ mod tests { merge_first_wins(&mut out, &[], packages(&[("pkg:gem/baz@3.0", "/c")])); assert_eq!(out.len(), 3); assert_eq!(out.get("pkg:npm/foo@1.0"), Some(&vec![PathBuf::from("/a")])); - assert_eq!(out.get("pkg:cargo/bar@2.0"), Some(&vec![PathBuf::from("/b")])); + assert_eq!( + out.get("pkg:cargo/bar@2.0"), + Some(&vec![PathBuf::from("/b")]) + ); assert_eq!(out.get("pkg:gem/baz@3.0"), Some(&vec![PathBuf::from("/c")])); } @@ -995,21 +1077,68 @@ mod tests { .await; let copies = out.get("pkg:npm/dup@1.0.0").expect("dup resolves"); - assert_eq!(copies.len(), 2, "both copies must be carried; got {copies:?}"); + assert_eq!( + copies.len(), + 2, + "both copies must be carried; got {copies:?}" + ); assert_eq!(copies[0], root_copy, "root copy first"); assert!(copies.contains(&nested_copy), "nested copy must be present"); // The collapsing wrapper (used by vendor/vex/setup/get) keeps the // old one-path contract: exactly the root-preferred representative. - let single = find_packages_for_purls( - &partitioned, - &local_options(tmp.path().to_path_buf()), - true, - ) - .await; + let single = + find_packages_for_purls(&partitioned, &local_options(tmp.path().to_path_buf()), true) + .await; assert_eq!(single.get("pkg:npm/dup@1.0.0"), Some(&root_copy)); } + /// Multi-copy P0 for gem (mirrors the npm test above): bundler's scoped + /// (`//gems`) and flat (`gems/`) store layouts coexist under + /// one `vendor/bundle` root — a bundler-2 `--path` install beside a + /// bundler-1 env install — each holding a REAL physical copy of the same + /// `gem@version`. `find_all_packages_for_purls` (apply's resolver) must + /// carry BOTH copies, highest-precedence store first. First-wins merging + /// resolved ONE copy, apply patched it and reported success while the + /// other bundler loaded the pristine (vulnerable) sibling. + #[tokio::test] + async fn find_all_packages_for_purls_carries_every_gem_store_copy() { + let tmp = tempfile::tempdir().unwrap(); + // No Gemfile on purpose: env/config bundle roots are manifest-gated, + // so an ambient BUNDLE_PATH on the dev machine cannot perturb this + // test; the implicit vendor/bundle probe is ungated. + let bundle = tmp.path().join("vendor").join("bundle"); + let scoped_copy = bundle + .join("ruby") + .join("3.2.0") + .join("gems") + .join("rack-3.1.0"); + let flat_copy = bundle.join("gems").join("rack-3.1.0"); + std::fs::create_dir_all(scoped_copy.join("lib")).unwrap(); + std::fs::create_dir_all(flat_copy.join("lib")).unwrap(); + // The specifications/ sibling marks the flat layout as a real gem home. + std::fs::create_dir_all(bundle.join("specifications")).unwrap(); + + let purl = "pkg:gem/rack@3.1.0".to_string(); + let partitioned = partition_purls(std::slice::from_ref(&purl), None); + let opts = local_options(tmp.path().to_path_buf()); + + let out = find_all_packages_for_purls(&partitioned, &opts, true).await; + let copies = out.get(&purl).expect("gem resolves"); + assert_eq!( + copies.len(), + 2, + "both coexisting store copies must be carried; got {copies:?}" + ); + assert_eq!(copies[0], scoped_copy, "scoped store copy first"); + assert!(copies.contains(&flat_copy), "flat store copy present"); + + // The collapsing wrapper (vendor/vex/setup/get/repair-vendor) keeps + // the one-representative contract: the first store's copy. + let single = find_packages_for_purls(&partitioned, &opts, true).await; + assert_eq!(single.get(&purl), Some(&scoped_copy)); + } + #[tokio::test] async fn find_packages_for_purls_skips_version_mismatch() { // The crawler only matches an installed dir whose version equals the diff --git a/crates/socket-patch-cli/tests/in_process_gem_multicopy.rs b/crates/socket-patch-cli/tests/in_process_gem_multicopy.rs new file mode 100644 index 00000000..9b71ac29 --- /dev/null +++ b/crates/socket-patch-cli/tests/in_process_gem_multicopy.rs @@ -0,0 +1,232 @@ +//! Gem multi-copy apply/rollback regression (silent partial — the gem +//! sibling of the npm multi-copy P0, see `in_process_npm_multicopy.rs`). +//! +//! Bundler produces TWO store layouts under one install root: the scoped +//! `vendor/bundle///gems/` store (bundler >= 2, `--path` / +//! local-config installs) and the flat `vendor/bundle/gems/` store +//! (bundler 1 with an env `BUNDLE_PATH`). Both can coexist — a bundler-2 +//! install beside a bundler-1 install of the SAME project — each holding a +//! REAL physical copy of the same `gem@version`. `apply` used to resolve +//! the purl to ONE store's copy (first-wins merge), patch it, and report +//! `success` while whichever bundler loaded the OTHER store ran pristine +//! (vulnerable) bytes. +//! +//! These tests build the coexisting layout by hand (hermetic, offline, no +//! ruby toolchain), hand-stage a `.socket/` manifest + blobs, run the REAL +//! `apply` / `rollback` flow through the built binary, and assert EVERY +//! physical copy is patched (and later restored) AND that the JSON summary +//! counts every copy. + +use std::path::{Path, PathBuf}; +use std::process::Command; + +use sha2::{Digest, Sha256}; + +const PURL: &str = "pkg:gem/rack@3.1.0"; + +fn binary() -> PathBuf { + env!("CARGO_BIN_EXE_socket-patch").into() +} + +/// Git-SHA256: SHA256("blob \0" ++ content). +fn git_sha256(content: &[u8]) -> String { + let header = format!("blob {}\0", content.len()); + let mut hasher = Sha256::new(); + hasher.update(header.as_bytes()); + hasher.update(content); + hex::encode(hasher.finalize()) +} + +/// Write a gem copy at `gem_dir` with `lib/rack.rb` holding `bytes`, +/// returning the file path. +fn write_copy(gem_dir: &Path, bytes: &[u8]) -> PathBuf { + let file = gem_dir.join("lib").join("rack.rb"); + std::fs::create_dir_all(file.parent().unwrap()).unwrap(); + std::fs::write(&file, bytes).unwrap(); + file +} + +fn stage_manifest_and_blob(root: &Path, before_hash: &str, after_hash: &str, patched: &[u8]) { + let socket = root.join(".socket"); + std::fs::create_dir_all(socket.join("blobs")).unwrap(); + std::fs::write( + socket.join("manifest.json"), + format!( + r#"{{ "patches": {{ + "{PURL}": {{ + "uuid": "67656d6d-756c-4469-8370-79302e302e30", + "exportedAt": "2024-01-01T00:00:00Z", + "files": {{ "lib/rack.rb": {{ + "beforeHash": "{before_hash}", "afterHash": "{after_hash}" + }}}}, + "vulnerabilities": {{}}, "description": "gem multi-copy fixture", + "license": "MIT", "tier": "free" + }} + }}}}"# + ), + ) + .unwrap(); + std::fs::write(socket.join("blobs").join(after_hash), patched).unwrap(); +} + +/// Lay down a project whose `vendor/bundle` holds BOTH bundler store +/// layouts, each with a real copy of `rack@3.1.0`. Returns +/// `(root, scoped_file, flat_file, before_hash, after_hash, patched)`. +fn build_two_store_project(tmp: &Path) -> (PathBuf, PathBuf, PathBuf, String, String, Vec) { + let original = b"module Rack\n VERSION = 'VULNERABLE'\nend\n"; + let mut patched = original.to_vec(); + patched.extend_from_slice(b"# SOCKET-PATCHED-GEM-MULTICOPY\n"); + let before_hash = git_sha256(original); + let after_hash = git_sha256(&patched); + assert_ne!(before_hash, after_hash, "fixture must be non-degenerate"); + + let bundle = tmp.join("vendor").join("bundle"); + // Scoped store (bundler >= 2 layout). + let scoped_file = write_copy( + &bundle + .join("ruby") + .join("3.2.0") + .join("gems") + .join("rack-3.1.0"), + original, + ); + // Flat store (bundler 1 env-BUNDLE_PATH layout) — the `specifications/` + // sibling marks it as a real gem home. + let flat_file = write_copy(&bundle.join("gems").join("rack-3.1.0"), original); + std::fs::create_dir_all(bundle.join("specifications")).unwrap(); + + stage_manifest_and_blob(tmp, &before_hash, &after_hash, &patched); + + ( + tmp.to_path_buf(), + scoped_file, + flat_file, + before_hash, + after_hash, + patched, + ) +} + +/// Run the given subcommand with the ambient `SOCKET_*` and `BUNDLE_*` +/// environment scrubbed (a developer's `BUNDLE_PATH` export or +/// `BUNDLE_APP_CONFIG` must not steer discovery), telemetry disabled. +fn run_json(root: &Path, args: &[&str]) -> (i32, serde_json::Value) { + let mut cmd = Command::new(binary()); + cmd.args(args) + .args(["--json", "--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"); + let code = out.status.code().unwrap_or(-1); + let stdout = String::from_utf8_lossy(&out.stdout).to_string(); + let v: serde_json::Value = serde_json::from_str(stdout.trim()) + .unwrap_or_else(|e| panic!("{args:?} must emit JSON: {e}; stdout={stdout}")); + (code, v) +} + +#[test] +fn apply_patches_every_coexisting_gem_store_copy() { + let tmp = tempfile::tempdir().unwrap(); + let (root, scoped_file, flat_file, before_hash, after_hash, patched) = + build_two_store_project(tmp.path()); + + // Pristine pre-check: neither copy carries the marker yet. + for f in [&scoped_file, &flat_file] { + let bytes = std::fs::read(f).unwrap(); + assert_eq!(git_sha256(&bytes), before_hash, "pre-apply copy at {f:?}"); + } + + let (code, v) = run_json(&root, &["apply"]); + assert_eq!(code, 0, "apply must succeed; envelope={v}"); + assert_eq!(v["status"], "success", "envelope={v}"); + + // THE security guarantee: BOTH physical copies must now be patched + // byte-for-byte. Before the fix, only the scoped copy was written and + // the flat copy (the one bundler 1 loads) stayed VULNERABLE while the + // run still reported success. + for (label, f) in [("scoped", &scoped_file), ("flat", &flat_file)] { + let bytes = std::fs::read(f).unwrap(); + assert_eq!( + bytes, patched, + "{label} store copy at {f:?} was NOT patched (silent partial)" + ); + assert_eq!( + git_sha256(&bytes), + after_hash, + "{label} store copy at {f:?} does not hash to afterHash" + ); + } + + // The summary must COUNT both copies — one Applied event per physical + // copy — so the envelope carries a signal that a second copy existed. + assert_eq!( + v["summary"]["applied"], 2, + "summary must count both patched copies; envelope={v}" + ); + let applied_events = v["events"] + .as_array() + .expect("events array") + .iter() + .filter(|e| e["action"] == "applied" && e["purl"] == PURL) + .count(); + assert_eq!( + applied_events, 2, + "one applied event per physical copy; envelope={v}" + ); +} + +#[test] +fn rollback_restores_every_coexisting_gem_store_copy() { + let tmp = tempfile::tempdir().unwrap(); + let (root, scoped_file, flat_file, before_hash, after_hash, patched) = + build_two_store_project(tmp.path()); + + // Stage the before-blob so rollback can restore in place. + let original: Vec = { + let mut o = patched.clone(); + let marker = b"# SOCKET-PATCHED-GEM-MULTICOPY\n"; + o.truncate(o.len() - marker.len()); + o + }; + assert_eq!(git_sha256(&original), before_hash); + std::fs::write( + root.join(".socket").join("blobs").join(&before_hash), + &original, + ) + .unwrap(); + + // Apply first so both copies are patched. + let (code, v) = run_json(&root, &["apply"]); + assert_eq!(code, 0, "apply precondition; envelope={v}"); + for f in [&scoped_file, &flat_file] { + assert_eq!( + std::fs::read(f).unwrap(), + patched, + "precondition: patched {f:?}" + ); + } + + // Now roll back and assert EVERY copy is restored to pristine bytes. + let (code, v) = run_json(&root, &["rollback", "--yes"]); + assert_eq!(code, 0, "rollback must succeed; envelope={v}"); + + for (label, f) in [("scoped", &scoped_file), ("flat", &flat_file)] { + let bytes = std::fs::read(f).unwrap(); + assert_eq!( + bytes, original, + "{label} store copy at {f:?} was NOT restored (rollback left a patched copy)" + ); + assert_eq!( + git_sha256(&bytes), + before_hash, + "{label} store copy restore hash" + ); + } + assert_ne!(before_hash, after_hash); +} diff --git a/crates/socket-patch-core/src/crawlers/composer_crawler.rs b/crates/socket-patch-core/src/crawlers/composer_crawler.rs index 41bb9a55..da02fa72 100644 --- a/crates/socket-patch-core/src/crawlers/composer_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/composer_crawler.rs @@ -3,7 +3,7 @@ use std::path::{Path, PathBuf}; use super::types::{CrawledPackage, CrawlerOptions}; use crate::patch::path_safety; -use crate::utils::fs::{is_dir, is_file}; +use crate::utils::fs::{is_dir, is_file, normalize_lexically}; use crate::utils::process::{CommandRunner, SystemCommandRunner}; /// PHP/Composer ecosystem crawler for discovering packages in Composer @@ -433,39 +433,9 @@ async fn resolve_project_root(vendor_path: &Path) -> PathBuf { normalize_lexically(&root).unwrap_or(root) } -/// Resolve `.`/`..` without touching the filesystem, so a path can be -/// containment-checked BEFORE it is opened (a canonicalizing check would -/// have to stat the very path being validated, and would fail on -/// not-yet-existing directories). Returns `None` when `..` pops above the -/// path's own root — nothing legitimate does that, so it fails closed. -/// -/// Symlinks are not resolved: a symlink INSIDE the project pointing out -/// of it is a pre-existing trust decision of the project's own tree, the -/// same assumption the rest of the crawler layer makes. -fn normalize_lexically(path: &Path) -> Option { - use std::path::Component; - - let mut out = PathBuf::new(); - let mut depth = 0usize; - for component in path.components() { - match component { - Component::Prefix(_) | Component::RootDir => out.push(component.as_os_str()), - Component::CurDir => {} - Component::ParentDir => { - if depth == 0 { - return None; - } - out.pop(); - depth -= 1; - } - Component::Normal(segment) => { - out.push(segment); - depth += 1; - } - } - } - Some(out) -} +// `normalize_lexically` (resolve `.`/`..` without touching the filesystem) +// lives in `crate::utils::fs` — shared with the ruby crawler's +// config-sourced `BUNDLE_PATH` containment guard. /// Resolve an installed.json `install-path` against the vendor tree. /// diff --git a/crates/socket-patch-core/src/crawlers/ruby_crawler.rs b/crates/socket-patch-core/src/crawlers/ruby_crawler.rs index 63b5fabe..276676d2 100644 --- a/crates/socket-patch-core/src/crawlers/ruby_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/ruby_crawler.rs @@ -4,7 +4,7 @@ use std::path::{Path, PathBuf}; use super::types::{CrawledPackage, CrawlerOptions}; use crate::patch::path_safety; -use crate::utils::fs::{entry_is_dir, home_dir, is_dir, list_dir_entries}; +use crate::utils::fs::{entry_is_dir, home_dir, is_dir, list_dir_entries, normalize_lexically}; use crate::utils::process::{CommandRunner, SystemCommandRunner}; /// Ruby/RubyGems ecosystem crawler for discovering gems in Bundler vendor @@ -23,18 +23,43 @@ impl RubyCrawler { /// Get gem installation paths based on options. /// - /// In local mode, checks the project's Bundler install roots first — - /// `vendor/bundle` plus any explicit `BUNDLE_PATH` (env var or - /// `.bundle/config`), each in both the scoped - /// `///gems/` and flat `/gems/` layouts — then, - /// only if the cwd holds a Bundler manifest or lockfile, falls back to - /// the gem homes `gem env` reports. + /// In local mode, probes the project's Bundler install roots in + /// bundler's own precedence order — the app config file's + /// `BUNDLE_PATH:` (`bundle config set --local path`), then the + /// `BUNDLE_PATH` env var, then the default `vendor/bundle` — each in + /// both the scoped `///gems/` and flat + /// `/gems/` layouts. When the default `vendor/bundle` root holds + /// a store (a deployment-style install), those stores are the whole + /// answer; otherwise, if the cwd holds a Bundler manifest or lockfile, + /// the gem homes `gem env` reports are appended (deduped) — default + /// gems (rexml, json, …) never live in a bundle path, so an + /// env/config-rooted project still needs them. /// /// In global mode, queries `gem env gemdir` and `gem env gempath`, plus /// well-known fallback paths for rbenv, rvm, Homebrew, and system Ruby. pub async fn get_gem_paths( &self, options: &CrawlerOptions, + ) -> Result, std::io::Error> { + self.get_gem_paths_with_env( + options, + std::env::var_os("BUNDLE_PATH").as_deref(), + std::env::var_os("BUNDLE_APP_CONFIG").as_deref(), + ambient_home().as_deref(), + ) + .await + } + + /// [`Self::get_gem_paths`] with the ambient `BUNDLE_PATH` / + /// `BUNDLE_APP_CONFIG` / home environment passed explicitly, so tests + /// stay hermetic on machines where bundler is configured. (`gem env` + /// still shells out; PATH-swapping tests keep covering that seam.) + pub async fn get_gem_paths_with_env( + &self, + options: &CrawlerOptions, + bundle_path_env: Option<&OsStr>, + app_config_env: Option<&OsStr>, + home_env: Option<&OsStr>, ) -> Result, std::io::Error> { if options.global || options.global_prefix.is_some() { if let Some(ref custom) = options.global_prefix { @@ -43,13 +68,31 @@ impl RubyCrawler { return Ok(Self::get_global_gem_paths().await); } - // Local mode: check the Bundler install roots first - let vendor_gems = Self::get_vendor_bundle_paths(&options.cwd).await; - if !vendor_gems.is_empty() { - return Ok(vendor_gems); + // Local mode: probe the Bundler install roots first. + let discovery = Self::discover_bundle_stores_with_env( + &options.cwd, + bundle_path_env, + app_config_env, + home_env, + ) + .await; + + // Historic early-return, kept ONLY for the implicit project-local + // `vendor/bundle` probe: a deployment-style install is the + // project's one gem source, so the ambient gem homes don't apply. + // Stores found via an env/config root do NOT suppress the fallback + // below: default gems (rexml, json, …) never live in a bundle path + // — they ship with ruby in the DEFAULT/system gem homes — so an + // env-`BUNDLE_PATH` project still needs the `gem env` homes to see + // them (the explicit-roots feature briefly suppressed that + // pre-existing fallback). + if discovery.default_root_has_stores { + return Ok(discovery.stores); } - // Only fall back to the installed gem homes if this looks like a Ruby + let mut paths = discovery.stores; + + // Only consult the installed gem homes if this looks like a Ruby // project. A non-deployment `bundle install` puts the project's gems // in the ambient gem homes, so every home `gem env` reports counts — // not just `gemdir`: bundler resolves from all of `Gem.path`, and a @@ -57,14 +100,15 @@ impl RubyCrawler { // keeps shared gems in the `@global` gemset; `--user-install` puts // them under `~/.gem`/`$XDG_DATA_HOME`). if Self::has_bundler_manifest(&options.cwd).await { - let gems_dirs = Self::gem_env_gems_dirs().await; - if !gems_dirs.is_empty() { - return Ok(gems_dirs); + let mut seen: HashSet = paths.iter().cloned().collect(); + for gems_dir in Self::gem_env_gems_dirs().await { + if seen.insert(gems_dir.clone()) { + paths.push(gems_dir); + } } } - // Not a Ruby project — return empty - Ok(Vec::new()) + Ok(paths) } /// Crawl all discovered gem paths and return every package found. @@ -185,25 +229,53 @@ impl RubyCrawler { /// Find installed-gem `gems/` directories under the project's Bundler /// install roots. /// - /// Reads the ambient `BUNDLE_PATH`/`BUNDLE_APP_CONFIG` environment; the - /// `_with_env` variant takes both as parameters so tests stay hermetic. + /// Reads the ambient `BUNDLE_PATH`/`BUNDLE_APP_CONFIG`/home + /// environment; the `_with_env` variant takes them as parameters so + /// tests stay hermetic. Production flows go through + /// [`Self::get_gem_paths`] → [`Self::discover_bundle_stores_with_env`]; + /// these two are the unit-test seam pinning the store list shape. + #[cfg(test)] async fn get_vendor_bundle_paths(cwd: &Path) -> Vec { - Self::get_vendor_bundle_paths_with_env( + Self::discover_bundle_stores_with_env( cwd, std::env::var_os("BUNDLE_PATH").as_deref(), std::env::var_os("BUNDLE_APP_CONFIG").as_deref(), + ambient_home().as_deref(), ) .await + .stores } - /// The bundler install roots probed: + /// [`Self::discover_bundle_stores_with_env`], flattened to just the + /// store list (the historical shape most tests pin). + #[cfg(test)] + async fn get_vendor_bundle_paths_with_env( + cwd: &Path, + bundle_path_env: Option<&OsStr>, + app_config_env: Option<&OsStr>, + ) -> Vec { + Self::discover_bundle_stores_with_env(cwd, bundle_path_env, app_config_env, None) + .await + .stores + } + + /// The bundler install roots, probed in bundler's own precedence order + /// (local config beats env beats default — `Bundler::Settings`): /// - /// - `/vendor/bundle` — the default deployment/`--path` location; - /// - `$BUNDLE_PATH` — bundler's explicit install root (a relative value - /// resolves against the project root, matching `Bundler.bundle_path`); - /// - the `BUNDLE_PATH:` entry of the app config file - /// (`$BUNDLE_APP_CONFIG/config`, else `/.bundle/config`) — what - /// `bundle config set --local path ` records. + /// 1. the `BUNDLE_PATH:` entry of the app config file + /// (`$BUNDLE_APP_CONFIG/config`, else `/.bundle/config`) — what + /// `bundle config set --local path ` records. SECURITY: this + /// file is typically committed, i.e. attacker-authored input, and + /// the root becomes a scan/apply WRITE target — so a value that + /// resolves outside the project root is skipped with a warning (see + /// [`resolve_config_bundle_path`]). `BUNDLE_PATH__SYSTEM: "true"` + /// makes bundler ignore the recorded path, so it is dropped too + /// (see [`parse_bundle_config_path`]). + /// 2. `$BUNDLE_PATH` — bundler's explicit install root (a relative + /// value resolves against the project root, matching + /// `Bundler.bundle_path`; a leading `~` expands against home). + /// Trusted as-is: it is the user's own environment. + /// 3. `/vendor/bundle` — the default deployment/`--path` location. /// /// The explicit roots can point anywhere (a machine-wide `BUNDLE_PATH` /// export must not pull another project's gem store into a non-Ruby @@ -214,38 +286,61 @@ impl RubyCrawler { /// /// Each root is probed in BOTH layouts bundler produces (see /// [`Self::bundle_root_gems_dirs`]), and roots plus discovered `gems/` - /// dirs are deduped so a root reachable two ways (e.g. `BUNDLE_PATH` - /// naming `vendor/bundle`) is not scanned twice. - async fn get_vendor_bundle_paths_with_env( + /// dirs are lexically normalized and deduped so a root reachable two + /// ways (e.g. `BUNDLE_PATH` naming `vendor/bundle`, or spelling it + /// `vendor/x/../bundle`) is not scanned — or patched — twice. + async fn discover_bundle_stores_with_env( cwd: &Path, bundle_path_env: Option<&OsStr>, app_config_env: Option<&OsStr>, - ) -> Vec { - let mut roots = vec![cwd.join("vendor").join("bundle")]; + home_env: Option<&OsStr>, + ) -> BundleStoreDiscovery { + let home = home_env.map(Path::new); + let default_root = cwd.join("vendor").join("bundle"); + let default_root = normalize_lexically(&default_root).unwrap_or(default_root); + let mut roots: Vec = Vec::new(); if Self::has_bundler_manifest(cwd).await { - if let Some(v) = bundle_path_env.filter(|v| !v.is_empty()) { - roots.push(resolve_bundle_path(cwd, Path::new(v))); + if let Some(value) = Self::app_config_bundle_path(cwd, app_config_env).await { + match resolve_config_bundle_path(cwd, &value, home) { + Some(root) => roots.push(root), + None => eprintln!( + "Warning (gem_bundle_config_path_ignored): bundler app config \ + BUNDLE_PATH {value:?} resolves outside the project root; \ + ignoring it as an install root (a committed .bundle/config is \ + untrusted input — set BUNDLE_PATH in the environment to use an \ + out-of-tree bundle path)" + ), + } } - if let Some(v) = Self::app_config_bundle_path(cwd, app_config_env).await { - roots.push(resolve_bundle_path(cwd, Path::new(&v))); + if let Some(v) = bundle_path_env.filter(|v| !v.is_empty()) { + roots.push(resolve_bundle_path(cwd, Path::new(v), home)); } } + roots.push(default_root.clone()); - let mut paths = Vec::new(); + let mut stores = Vec::new(); + let mut default_root_has_stores = false; let mut seen_roots = HashSet::new(); let mut seen = HashSet::new(); for root in roots { if !seen_roots.insert(root.clone()) { continue; } + let is_default = root == default_root; for gems_dir in Self::bundle_root_gems_dirs(&root).await { + if is_default { + default_root_has_stores = true; + } if seen.insert(gems_dir.clone()) { - paths.push(gems_dir); + stores.push(gems_dir); } } } - paths + BundleStoreDiscovery { + stores, + default_root_has_stores, + } } /// The installed-gem `gems/` dirs under one bundler install root, in @@ -537,6 +632,26 @@ impl Default for RubyCrawler { } } +/// Result of probing the Bundler install roots: the discovered +/// installed-gem `gems/` stores in root-precedence order, plus whether any +/// of them sit under the implicit project-local `vendor/bundle` root — +/// which keeps its historic [`RubyCrawler::get_gem_paths`] early-return +/// (a deployment install suppresses the `gem env` fallback; env/config +/// roots must not). +struct BundleStoreDiscovery { + stores: Vec, + default_root_has_stores: bool, +} + +/// The ambient home directory as an env value (`HOME`, else Windows' +/// `USERPROFILE`), `None` when unset or empty — the `~`-expansion base for +/// ambient runs; tests inject theirs through the `_with_env` seams. +fn ambient_home() -> Option { + std::env::var_os("HOME") + .filter(|v| !v.is_empty()) + .or_else(|| std::env::var_os("USERPROFILE").filter(|v| !v.is_empty())) +} + /// Pure parser for `gem env ` stdout. Returns the trimmed path /// string or `None` on empty input. Extracted so the helper logic is /// unit-testable without shelling out to the gem CLI. @@ -561,43 +676,137 @@ fn gem_homes_to_gems_dirs(gempath: &str) -> Vec { .collect() } -/// Resolve a `BUNDLE_PATH` value against the project root. Bundler resolves -/// a relative bundle path against the directory of the Gemfile -/// (`Bundler.root`), not the process cwd — the same rule +/// Expand a leading `~` component against the home directory, as bundler's +/// `File.expand_path` does for `BUNDLE_PATH` values. Only the bare-`~` +/// form (`~`, `~/store`) expands; `~user` needs the passwd lookup bundler +/// itself would do and is left untouched (it then resolves like a relative +/// path, the crawler's previous behavior for every `~` form). With no home +/// available the value is likewise left untouched. +fn expand_tilde(value: &Path, home: Option<&Path>) -> PathBuf { + let mut components = value.components(); + if let (Some(std::path::Component::Normal(first)), Some(home)) = (components.next(), home) { + if first == OsStr::new("~") { + return home.join(components.as_path()); + } + } + value.to_path_buf() +} + +/// Resolve a trusted (ENV-sourced) `BUNDLE_PATH` value against the project +/// root. Bundler `File.expand_path`s the value: a leading `~` expands to +/// the user's home, and a relative path resolves against the directory of +/// the Gemfile (`Bundler.root`), not the process cwd — the same rule /// [`crate::setup::gem::bundler_app_config_dir`] follows for -/// `BUNDLE_APP_CONFIG`. -fn resolve_bundle_path(root: &Path, value: &Path) -> PathBuf { - if value.is_absolute() { - value.to_path_buf() +/// `BUNDLE_APP_CONFIG`. `.`/`..` segments are folded lexically so the same +/// physical root spelled two ways dedups to one probe; a value that pops +/// above its own root keeps its unnormalized spelling (it is only ever +/// probed, and the env value is the user's own machine state — no +/// containment applies, unlike [`resolve_config_bundle_path`]). +fn resolve_bundle_path(root: &Path, value: &Path, home: Option<&Path>) -> PathBuf { + let expanded = expand_tilde(value, home); + let resolved = if expanded.is_absolute() { + expanded } else { - root.join(value) + root.join(expanded) + }; + normalize_lexically(&resolved).unwrap_or(resolved) +} + +/// Resolve a CONFIG-sourced `BUNDLE_PATH` value (bundler's app config file +/// — typically a committed `.bundle/config`) into an install root, or +/// `None` when the containment policy refuses it. +/// +/// SECURITY: unlike the environment variable (the user's own machine +/// state), a repo-committed `.bundle/config` is attacker-authored input — +/// and the resolved root becomes a scan target and, via `apply`, a WRITE +/// target. An absolute value (`/usr/local/…`) or a `..` traversal +/// (`../sibling-checkout`) must not let a malicious clone direct patch +/// writes outside the project. Policy: after `~` expansion and lexical +/// `.`/`..` normalization, the root must stay contained in the project +/// root — the same containment posture as the composer crawler's +/// `install-path` guard and the gem plugin-index cleanup in +/// `setup/gem/mod.rs`. Out-of-tree bundle paths stay reachable via the +/// trusted env `BUNDLE_PATH`. +fn resolve_config_bundle_path( + project_root: &Path, + value: &str, + home: Option<&Path>, +) -> Option { + let expanded = expand_tilde(Path::new(value), home); + // Anything rooted takes the strict prefix check — not just + // `is_absolute()`: on Windows a root-relative `\evil` or drive-relative + // `C:evil` is NOT "absolute" yet `Path::join` substitutes it for (part + // of) the base, so routing it through the relative branch would escape + // containment. + let rooted = expanded.has_root() + || matches!( + expanded.components().next(), + Some(std::path::Component::Prefix(_)) + ); + if rooted { + // Contained iff it normalizes to somewhere under the project root. + // The comparison base must be absolute too: the CLI's default + // `--cwd .` is relative, and `starts_with` against a relative (or + // empty) base would trivially pass. `std::path::absolute` is + // lexical (no symlink resolution), matching the normalization here; + // if it cannot produce a base, fail closed. + let normalized = normalize_lexically(&expanded)?; + let base = std::path::absolute(project_root).ok()?; + let base = normalize_lexically(&base)?; + (!base.as_os_str().is_empty() && normalized.starts_with(&base)).then_some(normalized) + } else { + // A relative value is contained by construction unless its `..` + // segments climb out of the project root — `normalize_lexically` + // fails closed on exactly that. + let contained = normalize_lexically(&expanded)?; + let joined = project_root.join(contained); + Some(normalize_lexically(&joined).unwrap_or(joined)) } } -/// Extract the `BUNDLE_PATH:` value from bundler's app config file contents. -/// The file is flat YAML bundler writes itself +/// Extract the effective `BUNDLE_PATH:` value from bundler's app config +/// file contents. The file is flat YAML bundler writes itself /// (`---\nBUNDLE_PATH: "vendor/bundle"\n`), so a line-based scrape is enough /// — matching the repo convention of line-parsing Cargo.toml rather than /// pulling in a format crate. Quoted values (bundler double-quotes what it /// writes) are unwrapped; an empty value counts as unset. Sibling keys like -/// `BUNDLE_PATH__SYSTEM:` must not match — the prefix requires the colon -/// immediately after `BUNDLE_PATH`. +/// `BUNDLE_PATH__SYSTEM:` must not match the path key — the prefix requires +/// the colon immediately after `BUNDLE_PATH`. +/// +/// `BUNDLE_PATH__SYSTEM: "true"` (bundler's `path.system` setting) makes +/// bundler IGNORE any recorded path and use the system/default gem home, so +/// the whole config entry parses as unset — the caller then falls through +/// to the `gem env` homes, which is exactly where those gems live. Bundler +/// converts only the exact string `true` to a truthy setting; anything else +/// leaves the recorded path in effect. fn parse_bundle_config_path(contents: &str) -> Option { + let mut path: Option = None; + let mut path_system = false; for line in contents.lines() { if let Some(rest) = line.strip_prefix("BUNDLE_PATH:") { - let v = rest.trim(); - let v = v - .strip_prefix('"') - .and_then(|s| s.strip_suffix('"')) - .or_else(|| v.strip_prefix('\'').and_then(|s| s.strip_suffix('\''))) - .unwrap_or(v); - if v.is_empty() { - return None; + let v = unquote_bundle_config_value(rest); + if !v.is_empty() { + path = Some(v.to_string()); } - return Some(v.to_string()); + } else if let Some(rest) = line.strip_prefix("BUNDLE_PATH__SYSTEM:") { + path_system = unquote_bundle_config_value(rest) == "true"; } } - None + if path_system { + None + } else { + path + } +} + +/// Unwrap one bundler app-config scalar: trim, then strip one matching +/// pair of double or single quotes (bundler double-quotes what it writes). +fn unquote_bundle_config_value(rest: &str) -> &str { + let v = rest.trim(); + v.strip_prefix('"') + .and_then(|s| s.strip_suffix('"')) + .or_else(|| v.strip_prefix('\'').and_then(|s| s.strip_suffix('\''))) + .unwrap_or(v) } /// Whether a PURL-derived gem coordinate is safe to join onto the gem root. @@ -1018,42 +1227,6 @@ mod tests { assert_eq!(paths, vec![flat]); } - /// `$BUNDLE_APP_CONFIG` relocates the app config dir (the official - /// ruby Docker images export it) — the `BUNDLE_PATH:` entry must be - /// honored from there, and the default `.bundle/config` (absent - /// here) must not be required. - #[tokio::test] - async fn app_config_env_relocates_config() { - let dir = tempfile::tempdir().unwrap(); - tokio::fs::write(dir.path().join("Gemfile"), b"gem \"foo\"\n") - .await - .unwrap(); - let app_config = dir.path().join("elsewhere-config"); - tokio::fs::create_dir_all(&app_config).await.unwrap(); - let root = dir.path().join("store"); - tokio::fs::write( - app_config.join("config"), - format!("---\nBUNDLE_PATH: \"{}\"\n", root.display()), - ) - .await - .unwrap(); - let flat = root.join("gems"); - tokio::fs::create_dir_all(flat.join("foo-1.0.0").join("lib")) - .await - .unwrap(); - tokio::fs::create_dir_all(root.join("specifications")) - .await - .unwrap(); - - let paths = RubyCrawler::get_vendor_bundle_paths_with_env( - dir.path(), - None, - Some(app_config.as_os_str()), - ) - .await; - assert_eq!(paths, vec![flat]); - } - /// A FIFO planted as `.bundle/config` must not wedge discovery. The /// app-config read used a plain `tokio::fs::read_to_string`, whose /// `open(2)` on a FIFO waits for a writer that never comes — so one @@ -1113,6 +1286,399 @@ mod tests { assert_eq!(paths, vec![flat]); } + /// `$BUNDLE_APP_CONFIG` relocates the app config dir (the official + /// ruby Docker images export it) — the `BUNDLE_PATH:` entry must be + /// honored from there, and the default `.bundle/config` (absent + /// here) must not be required. + #[tokio::test] + async fn app_config_env_relocates_config() { + let dir = tempfile::tempdir().unwrap(); + tokio::fs::write(dir.path().join("Gemfile"), b"gem \"foo\"\n") + .await + .unwrap(); + let app_config = dir.path().join("elsewhere-config"); + tokio::fs::create_dir_all(&app_config).await.unwrap(); + let root = dir.path().join("store"); + tokio::fs::write( + app_config.join("config"), + format!("---\nBUNDLE_PATH: \"{}\"\n", root.display()), + ) + .await + .unwrap(); + let flat = root.join("gems"); + tokio::fs::create_dir_all(flat.join("foo-1.0.0").join("lib")) + .await + .unwrap(); + tokio::fs::create_dir_all(root.join("specifications")) + .await + .unwrap(); + + let paths = RubyCrawler::get_vendor_bundle_paths_with_env( + dir.path(), + None, + Some(app_config.as_os_str()), + ) + .await; + assert_eq!(paths, vec![flat]); + } + + /// Roots must be probed in bundler's own precedence order — local + /// `.bundle/config` `BUNDLE_PATH:` first, then the `BUNDLE_PATH` + /// environment variable, then the implicit `vendor/bundle` default — + /// so the stores come back highest-precedence first and first-wins + /// consumers pick the copy bundler actually loads. The pre-fix order + /// (default → env → config) was bundler's precedence inverted. + #[tokio::test] + async fn bundle_roots_probe_in_bundler_precedence_order() { + let dir = tempfile::tempdir().unwrap(); + tokio::fs::write(dir.path().join("Gemfile"), b"gem \"foo\"\n") + .await + .unwrap(); + + // Flat store under each of the three roots. + let mut flats = Vec::new(); + for root in ["configstore", "envstore"] { + let root = dir.path().join("vendor").join(root); + let flat = root.join("gems"); + tokio::fs::create_dir_all(flat.join("foo-1.0.0").join("lib")) + .await + .unwrap(); + tokio::fs::create_dir_all(root.join("specifications")) + .await + .unwrap(); + flats.push(flat); + } + let default_root = dir.path().join("vendor").join("bundle"); + let default_flat = default_root.join("gems"); + tokio::fs::create_dir_all(default_flat.join("foo-1.0.0").join("lib")) + .await + .unwrap(); + tokio::fs::create_dir_all(default_root.join("specifications")) + .await + .unwrap(); + + tokio::fs::create_dir_all(dir.path().join(".bundle")) + .await + .unwrap(); + tokio::fs::write( + dir.path().join(".bundle").join("config"), + "---\nBUNDLE_PATH: \"vendor/configstore\"\n", + ) + .await + .unwrap(); + + let env_root = dir.path().join("vendor").join("envstore"); + let paths = RubyCrawler::get_vendor_bundle_paths_with_env( + dir.path(), + Some(env_root.as_os_str()), + None, + ) + .await; + assert_eq!( + paths, + vec![flats[0].clone(), flats[1].clone(), default_flat], + "stores must come back config-first, env second, default last (bundler precedence); got {paths:?}" + ); + } + + // ── config-sourced root containment (untrusted .bundle/config) ─ + + /// SECURITY: an ABSOLUTE `BUNDLE_PATH` in the (typically committed, + /// attacker-authored) app config file that points outside the project + /// must be skipped — it would otherwise become a scan/apply WRITE + /// target anywhere on the machine. The store it names must NOT be + /// discovered even though it is real and valid. + #[tokio::test] + async fn config_bundle_path_absolute_outside_is_skipped() { + let dir = tempfile::tempdir().unwrap(); + let outside = tempfile::tempdir().unwrap(); + tokio::fs::write(dir.path().join("Gemfile"), b"gem \"foo\"\n") + .await + .unwrap(); + tokio::fs::create_dir_all(dir.path().join(".bundle")) + .await + .unwrap(); + tokio::fs::write( + dir.path().join(".bundle").join("config"), + format!("---\nBUNDLE_PATH: \"{}\"\n", outside.path().display()), + ) + .await + .unwrap(); + // A real store at the outside root — must stay undiscovered. + tokio::fs::create_dir_all(outside.path().join("gems").join("foo-1.0.0").join("lib")) + .await + .unwrap(); + tokio::fs::create_dir_all(outside.path().join("specifications")) + .await + .unwrap(); + + let paths = RubyCrawler::get_vendor_bundle_paths_with_env(dir.path(), None, None).await; + assert!( + paths.is_empty(), + "absolute out-of-project config BUNDLE_PATH must be skipped: {paths:?}" + ); + } + + /// SECURITY: a `..` traversal in the config value (`../sibling`) must + /// be skipped — a malicious clone must not direct patch writes into a + /// sibling checkout. + #[tokio::test] + async fn config_bundle_path_parent_traversal_is_skipped() { + let dir = tempfile::tempdir().unwrap(); + let project = dir.path().join("project"); + let sibling = dir.path().join("sibling"); + tokio::fs::create_dir_all(&project).await.unwrap(); + tokio::fs::write(project.join("Gemfile"), b"gem \"foo\"\n") + .await + .unwrap(); + tokio::fs::create_dir_all(project.join(".bundle")) + .await + .unwrap(); + tokio::fs::write( + project.join(".bundle").join("config"), + "---\nBUNDLE_PATH: \"../sibling\"\n", + ) + .await + .unwrap(); + tokio::fs::create_dir_all(sibling.join("gems").join("foo-1.0.0").join("lib")) + .await + .unwrap(); + tokio::fs::create_dir_all(sibling.join("specifications")) + .await + .unwrap(); + + let paths = RubyCrawler::get_vendor_bundle_paths_with_env(&project, None, None).await; + assert!( + paths.is_empty(), + "`..`-traversing config BUNDLE_PATH must be skipped: {paths:?}" + ); + } + + /// A contained relative config value is accepted — including one that + /// detours through `.`/`..` segments but normalizes back inside the + /// project (bundler resolves it the same way). + #[tokio::test] + async fn config_bundle_path_contained_relative_accepted() { + let dir = tempfile::tempdir().unwrap(); + tokio::fs::write(dir.path().join("Gemfile"), b"gem \"foo\"\n") + .await + .unwrap(); + tokio::fs::create_dir_all(dir.path().join(".bundle")) + .await + .unwrap(); + tokio::fs::write( + dir.path().join(".bundle").join("config"), + "---\nBUNDLE_PATH: \"vendor/./extra/../mygems\"\n", + ) + .await + .unwrap(); + let root = dir.path().join("vendor").join("mygems"); + let flat = root.join("gems"); + tokio::fs::create_dir_all(flat.join("foo-1.0.0").join("lib")) + .await + .unwrap(); + tokio::fs::create_dir_all(root.join("specifications")) + .await + .unwrap(); + + let paths = RubyCrawler::get_vendor_bundle_paths_with_env(dir.path(), None, None).await; + assert_eq!( + paths, + vec![flat], + "contained (normalized) relative config BUNDLE_PATH must be accepted" + ); + } + + /// Unit contract for the config-root containment policy itself. + /// Real (absolute) tempdir paths keep the assertions valid on Windows, + /// where a `/`-rooted literal is NOT absolute. + #[test] + fn resolve_config_bundle_path_containment_contract() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + let outside = tempfile::tempdir().unwrap(); + + // Contained relative values resolve against the project root. + assert_eq!( + resolve_config_bundle_path(root, "vendor/bundle", None), + Some(root.join("vendor").join("bundle")) + ); + assert_eq!( + resolve_config_bundle_path(root, "vendor/x/../y", None), + Some(root.join("vendor").join("y")) + ); + // Escaping relative values are refused. + assert_eq!(resolve_config_bundle_path(root, "../sibling", None), None); + assert_eq!( + resolve_config_bundle_path(root, "vendor/../../sibling", None), + None + ); + // Absolute values must land under the project root. + let inside = root.join("vendor").join("bundle"); + assert_eq!( + resolve_config_bundle_path(root, inside.to_str().unwrap(), None), + Some(inside) + ); + assert_eq!( + resolve_config_bundle_path(root, outside.path().to_str().unwrap(), None), + None + ); + // `..` smuggled into an absolute value cannot sneak past the + // prefix check — it is normalized BEFORE comparing. + let sneaky = root.join("vendor").join("..").join(".."); + assert_eq!( + resolve_config_bundle_path(root, sneaky.to_str().unwrap(), None), + None + ); + // A root-relative value (`/evil`) is refused on every platform: a + // unix absolute path outside the project, and on Windows a rooted + // path `Path::join` would substitute into the base — either way it + // must take the strict branch and fail the prefix check. + assert_eq!(resolve_config_bundle_path(root, "/evil", None), None); + // Windows drive-relative (`C:evil`) likewise must not reach the + // join-based relative branch. + #[cfg(windows)] + assert_eq!(resolve_config_bundle_path(root, "C:evil", None), None); + // `~` expands against home first; home outside the project → + // refused, home inside → accepted. + assert_eq!( + resolve_config_bundle_path(root, "~/store", Some(outside.path())), + None + ); + let home_in = root.join("home"); + assert_eq!( + resolve_config_bundle_path(root, "~/store", Some(&home_in)), + Some(home_in.join("store")) + ); + } + + // ── env BUNDLE_PATH `~` expansion + normalization ────────────── + + /// A leading `~/` in the env `BUNDLE_PATH` expands against HOME + /// (bundler `File.expand_path`s the value); it used to resolve as a + /// literal `/~/...` relative path and discover nothing. + #[tokio::test] + async fn bundle_path_env_tilde_expands_against_home() { + let dir = tempfile::tempdir().unwrap(); + let home = tempfile::tempdir().unwrap(); + tokio::fs::write(dir.path().join("Gemfile"), b"gem \"foo\"\n") + .await + .unwrap(); + let root = home.path().join("bundle-store"); + let flat = root.join("gems"); + tokio::fs::create_dir_all(flat.join("foo-1.0.0").join("lib")) + .await + .unwrap(); + tokio::fs::create_dir_all(root.join("specifications")) + .await + .unwrap(); + + let discovery = RubyCrawler::discover_bundle_stores_with_env( + dir.path(), + Some(OsStr::new("~/bundle-store")), + None, + Some(home.path().as_os_str()), + ) + .await; + assert_eq!( + discovery.stores, + vec![flat], + "~/ must expand against the provided home" + ); + assert!( + !discovery.default_root_has_stores, + "env root must not count as the default vendor/bundle root" + ); + } + + /// The ENV value stays trusted — an out-of-project absolute root is + /// honored (unlike the config file, it is the user's own machine + /// state) — and `..` segments are normalized so the same physical + /// root spelled two ways dedups against the default probe. + #[tokio::test] + async fn bundle_path_env_outside_project_trusted_and_dotdot_dedups() { + let dir = tempfile::tempdir().unwrap(); + let outside = tempfile::tempdir().unwrap(); + tokio::fs::write(dir.path().join("Gemfile"), b"gem \"foo\"\n") + .await + .unwrap(); + tokio::fs::create_dir_all(outside.path().join("gems").join("foo-1.0.0").join("lib")) + .await + .unwrap(); + tokio::fs::create_dir_all(outside.path().join("specifications")) + .await + .unwrap(); + let paths = RubyCrawler::get_vendor_bundle_paths_with_env( + dir.path(), + Some(outside.path().as_os_str()), + None, + ) + .await; + assert_eq!( + paths, + vec![outside.path().join("gems")], + "env BUNDLE_PATH outside the project stays honored (trusted)" + ); + + // `vendor/x/../bundle` names the default root — must dedup to one + // probe (one store, once). + let bundle = dir.path().join("vendor").join("bundle"); + let flat = bundle.join("gems"); + tokio::fs::create_dir_all(flat.join("foo-1.0.0").join("lib")) + .await + .unwrap(); + tokio::fs::create_dir_all(bundle.join("specifications")) + .await + .unwrap(); + let paths = RubyCrawler::get_vendor_bundle_paths_with_env( + dir.path(), + Some(OsStr::new("vendor/x/../bundle")), + None, + ) + .await; + assert_eq!( + paths.iter().filter(|p| **p == flat).count(), + 1, + "normalized env root must dedup against the default probe: {paths:?}" + ); + } + + // ── BUNDLE_PATH__SYSTEM drops the config-sourced root ────────── + + /// `BUNDLE_PATH__SYSTEM: "true"` makes bundler ignore the recorded + /// path entirely — the config-sourced root must be dropped so the + /// project falls through to the system gem homes (the `gem env` + /// fallback in `get_gem_paths`). + #[tokio::test] + async fn config_bundle_path_system_true_drops_config_root() { + let dir = tempfile::tempdir().unwrap(); + tokio::fs::write(dir.path().join("Gemfile"), b"gem \"foo\"\n") + .await + .unwrap(); + tokio::fs::create_dir_all(dir.path().join(".bundle")) + .await + .unwrap(); + tokio::fs::write( + dir.path().join(".bundle").join("config"), + "---\nBUNDLE_PATH: \"vendor/mygems\"\nBUNDLE_PATH__SYSTEM: \"true\"\n", + ) + .await + .unwrap(); + let root = dir.path().join("vendor").join("mygems"); + tokio::fs::create_dir_all(root.join("gems").join("foo-1.0.0").join("lib")) + .await + .unwrap(); + tokio::fs::create_dir_all(root.join("specifications")) + .await + .unwrap(); + + let paths = RubyCrawler::get_vendor_bundle_paths_with_env(dir.path(), None, None).await; + assert!( + paths.is_empty(), + "path.system=true must drop the config-sourced root: {paths:?}" + ); + } + /// Pure parser contract for the `.bundle/config` scrape: bundler's /// own quoted form, unquoted and single-quoted variants, CRLF, /// empty-value-as-unset, and no match on `BUNDLE_PATH__SYSTEM:` or @@ -1146,6 +1712,28 @@ mod tests { parse_bundle_config_path("---\nBUNDLE_PATH__SYSTEM: \"true\"\n"), None ); + // `path.system` true means "ignore the recorded path, use the + // system gem home" — the recorded path must parse as unset, + // whichever order the keys appear in. + assert_eq!( + parse_bundle_config_path( + "---\nBUNDLE_PATH: \"vendor/bundle\"\nBUNDLE_PATH__SYSTEM: \"true\"\n" + ), + None + ); + assert_eq!( + parse_bundle_config_path( + "---\nBUNDLE_PATH__SYSTEM: \"true\"\nBUNDLE_PATH: \"vendor/bundle\"\n" + ), + None + ); + // Only the exact string "true" is truthy (bundler's own coercion). + assert_eq!( + parse_bundle_config_path( + "---\nBUNDLE_PATH: \"vendor/bundle\"\nBUNDLE_PATH__SYSTEM: \"false\"\n" + ), + Some("vendor/bundle".to_string()) + ); assert_eq!( parse_bundle_config_path("---\nBUNDLE_FROZEN: \"true\"\n"), None diff --git a/crates/socket-patch-core/src/utils/fs.rs b/crates/socket-patch-core/src/utils/fs.rs index 4d696af6..f69b683c 100644 --- a/crates/socket-patch-core/src/utils/fs.rs +++ b/crates/socket-patch-core/src/utils/fs.rs @@ -166,6 +166,43 @@ pub(crate) fn home_dir() -> PathBuf { PathBuf::from(home) } +/// Resolve `.`/`..` without touching the filesystem, so a path can be +/// containment-checked BEFORE it is opened (a canonicalizing check would +/// have to stat the very path being validated, and would fail on +/// not-yet-existing directories). Returns `None` when `..` pops above the +/// path's own root — nothing legitimate does that, so it fails closed. +/// +/// Symlinks are not resolved: a symlink INSIDE the project pointing out +/// of it is a pre-existing trust decision of the project's own tree, the +/// same assumption the rest of the crawler layer makes. +/// +/// Shared by the composer crawler's `install-path` containment guard and +/// the ruby crawler's config-sourced `BUNDLE_PATH` containment guard. +pub(crate) fn normalize_lexically(path: &Path) -> Option { + use std::path::Component; + + let mut out = PathBuf::new(); + let mut depth = 0usize; + for component in path.components() { + match component { + Component::Prefix(_) | Component::RootDir => out.push(component.as_os_str()), + Component::CurDir => {} + Component::ParentDir => { + if depth == 0 { + return None; + } + out.pop(); + depth -= 1; + } + Component::Normal(segment) => { + out.push(segment); + depth += 1; + } + } + } + Some(out) +} + /// Atomically commit `content` to `path` via stage + fsync + rename. /// /// The single shared implementation of the hardened-writer pattern used for diff --git a/crates/socket-patch-core/tests/crawler_ruby_e2e.rs b/crates/socket-patch-core/tests/crawler_ruby_e2e.rs index d611dded..9a2bf068 100644 --- a/crates/socket-patch-core/tests/crawler_ruby_e2e.rs +++ b/crates/socket-patch-core/tests/crawler_ruby_e2e.rs @@ -370,7 +370,11 @@ async fn get_gem_paths_with_gemfile_no_vendor_returns_gemdir() { std::env::set_var("PATH", bin.path()); let crawler = RubyCrawler; - let result = crawler.get_gem_paths(&options_at(tmp.path())).await; + // Hermetic seam: a developer's ambient BUNDLE_PATH/BUNDLE_APP_CONFIG + // must not add install roots to this assertion. + let result = crawler + .get_gem_paths_with_env(&options_at(tmp.path()), None, None, None) + .await; if let Some(v) = prev { std::env::set_var("PATH", v); @@ -408,7 +412,11 @@ async fn get_gem_paths_with_gemfile_lock_only_returns_gemdir() { std::env::set_var("PATH", bin.path()); let crawler = RubyCrawler; - let result = crawler.get_gem_paths(&options_at(tmp.path())).await; + // Hermetic seam: a developer's ambient BUNDLE_PATH/BUNDLE_APP_CONFIG + // must not add install roots to this assertion. + let result = crawler + .get_gem_paths_with_env(&options_at(tmp.path()), None, None, None) + .await; if let Some(v) = prev { std::env::set_var("PATH", v); @@ -452,8 +460,12 @@ async fn get_gem_paths_with_gems_rb_manifest_returns_gemdir() { install_fake_gem(bin.path(), gemdir.path()); let crawler = RubyCrawler; + // Hermetic seam: ambient BUNDLE_PATH/BUNDLE_APP_CONFIG must not add + // install roots to this assertion. let paths = with_path(bin.path(), || async { - crawler.get_gem_paths(&options_at(tmp.path())).await + crawler + .get_gem_paths_with_env(&options_at(tmp.path()), None, None, None) + .await }) .await .unwrap(); @@ -484,8 +496,12 @@ async fn get_gem_paths_with_gems_locked_only_returns_gemdir() { install_fake_gem(bin.path(), gemdir.path()); let crawler = RubyCrawler; + // Hermetic seam: ambient BUNDLE_PATH/BUNDLE_APP_CONFIG must not add + // install roots to this assertion. let paths = with_path(bin.path(), || async { - crawler.get_gem_paths(&options_at(tmp.path())).await + crawler + .get_gem_paths_with_env(&options_at(tmp.path()), None, None, None) + .await }) .await .unwrap(); @@ -537,15 +553,17 @@ async fn get_gem_paths_local_includes_every_gempath_home() { let crawler = RubyCrawler; let (paths, decoy, crawled) = with_path(bin.path(), || async { + // Hermetic seam: ambient BUNDLE_PATH/BUNDLE_APP_CONFIG must not + // add install roots to this assertion. let paths = crawler - .get_gem_paths(&options_at(tmp.path())) + .get_gem_paths_with_env(&options_at(tmp.path()), None, None, None) .await .unwrap(); // Control: the gate still holds with `gem env` answerable — a // non-Ruby cwd must not pull in the ambient gem homes. let non_ruby = tempfile::tempdir().unwrap(); let decoy = crawler - .get_gem_paths(&options_at(non_ruby.path())) + .get_gem_paths_with_env(&options_at(non_ruby.path()), None, None, None) .await .unwrap(); let crawled = crawler.crawl_all(&options_at(tmp.path())).await; @@ -574,6 +592,76 @@ async fn get_gem_paths_local_includes_every_gempath_home() { ); } +/// An explicit env `BUNDLE_PATH` root must NOT suppress the `gem env` +/// fallback homes: default gems (rexml, json, …) ship with ruby in the +/// DEFAULT/system gem homes and never live in a bundle path. Before the +/// explicit-roots feature, an env-`BUNDLE_PATH` project with no +/// `vendor/bundle` reached the fallback (the vendor probe came back +/// empty); afterwards the env root tripped the same early-return and the +/// fallback vanished — a regression this pins closed. Only the implicit +/// project-local `vendor/bundle` probe keeps the early-return. +/// +/// Order is bundler's precedence: the env store first, then the gem-env +/// homes (gemdir's, then every other gempath home's), deduped. +#[cfg(unix)] +#[tokio::test] +#[serial] +async fn get_gem_paths_env_root_still_includes_gempath_homes() { + let tmp = tempfile::tempdir().unwrap(); + tokio::fs::write( + tmp.path().join("Gemfile"), + b"source 'https://rubygems.org'\n", + ) + .await + .unwrap(); + + // The env-designated bundle root, flat (bundler-1) layout. + let env_root = tmp.path().join("bundle-store"); + let env_flat = env_root.join("gems"); + tokio::fs::create_dir_all(env_flat.join("rack-3.1.0").join("lib")) + .await + .unwrap(); + tokio::fs::create_dir_all(env_root.join("specifications")) + .await + .unwrap(); + + // Two gem homes the fake `gem env` reports: gemdir + a gempath-only one. + let home_a = tempfile::tempdir().unwrap(); + let home_b = tempfile::tempdir().unwrap(); + let gems_a = home_a.path().join("gems"); + let gems_b = home_b.path().join("gems"); + tokio::fs::create_dir_all(gems_a.join("rexml-3.2.6").join("lib")) + .await + .unwrap(); + tokio::fs::create_dir_all(gems_b.join("json-2.7.2").join("lib")) + .await + .unwrap(); + + let gempath = std::env::join_paths([home_a.path(), home_b.path()]).unwrap(); + let bin = tempfile::tempdir().unwrap(); + install_fake_gem_with_gempath(bin.path(), home_a.path(), gempath.to_str().unwrap()); + + let crawler = RubyCrawler; + let paths = with_path(bin.path(), || async { + crawler + .get_gem_paths_with_env( + &options_at(tmp.path()), + Some(env_root.as_os_str()), + None, + None, + ) + .await + }) + .await + .unwrap(); + + assert_eq!( + paths, + vec![env_flat, gems_a, gems_b], + "env-root store first, then every gem-env home (fallback restored); got {paths:?}" + ); +} + // ── global gem discovery ─────────────────────────────────────── #[tokio::test] @@ -667,8 +755,10 @@ async fn get_gem_paths_local_gemfile_no_gem_binary_returns_empty() { std::env::set_var("PATH", empty_path.path()); let crawler = RubyCrawler; + // Hermetic seam: ambient BUNDLE_PATH/BUNDLE_APP_CONFIG must not add + // install roots to this assertion. let paths = crawler - .get_gem_paths(&options_at(tmp.path())) + .get_gem_paths_with_env(&options_at(tmp.path()), None, None, None) .await .unwrap(); From 460b1bfbcf6092d8f9050841f4102ea90da372fe Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Thu, 20 Aug 2026 10:41:39 -0400 Subject: [PATCH 2/2] fix(gem): surface the config-root skip on the run warning channels; class-split fallback-home failure semantics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review-round fixes for the two verified Bugbot findings on #222. Finding 1 (Medium): the containment guard's `gem_bundle_config_path_ignored` was a bare eprintln inside the crawler — it never reached any --json `warnings[]` and printed under --silent, violating the repo-wide warning conventions (#219/#220 + the #223 omnibus silent fixes). The crawler is now print-free: the refusal is RECORDED on `BundleStoreDiscovery.skipped_config_path`, and a shared `config_path_ignored_warning(value)` builder feeds the CLI channels — scan pushes it onto the same run-level channel as the PnP layout refusals (JSON `warnings[]` on both the zero-package and >=1-package envelopes; one stderr line gated on !json && !silent), and apply carries it in its Envelope `warnings[]` plus one gated stderr line. Scoped like the crawl that hit it: local mode, gem in --ecosystems/manifest scope. Finding 2 (High, with nuance): with an env/config bundle root, get_gem_paths appends the gem-env fallback homes and the multi-copy fan-out patched EVERY copy with per-copy loud-fail — so a gem present in both the bundle store and a shared home (rvm @global, root-owned system dir) failed the WHOLE run on a permission failure or variant mismatch THERE, even though the copy bundler loads patched fine. Patching a shared home's copy is not itself wrong (plain apply always patched GEM_HOME when no store existed); the defect was failure semantics crossing store classes. Fix: discovery's store list is exposed (`RubyCrawler::discover_bundle_stores`, fs-probes only) and apply's gem fan-out classes each copy — bundle-path store copies stay PRIMARY (loud-fail, unchanged); gem-env fallback-home copies become BEST-EFFORT once at least one store copy applied: a variant mismatch or write failure there is a per-copy non-fatal Skipped event (`gem_fallback_home_skipped`, detail names the path and reason; gated stderr twin), never a run failure. Parity edge kept: with NO bundle-store copy (the historic fallback-only layout, and every --global run) the home copy IS primary and keeps loud-fail exactly as pre-#218 apply. TDD evidence (red -> green on the rebased tip): scan/apply --json missing the warnings[] entry and the --silent leak (3 tests, in_process_gem_config_warning.rs); mismatched-home-copy exit 1 and Failed-event-on-strict-refusal (in_process_gem_fallback_home.rs), with fallback-only loud parity and both-copies-patched pinned green throughout. CLI_CONTRACT.md documents the copy classes and the warning channels. Gates: touched files rustfmt-clean; cargo clippy --workspace --all-features -D warnings clean; core --lib 2535, cli --lib 436; crawlers::ruby 50, crawler_ruby_e2e 25; gem+npm multicopy 2+2; apply_network 11, apply_invariants 4, cli_gem_variant_mismatch_policy 6, cli_apply_silent 2, e2e_gem hermetic 8 (6 shown +cache selftests), e2e_scan, cli_scan_silent, docker_e2e_gem — all green. Co-Authored-By: Claude Fable 5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 32 +- crates/socket-patch-cli/src/commands/apply.rs | 240 +++++++++++-- .../socket-patch-cli/src/commands/scan/gc.rs | 8 +- .../socket-patch-cli/src/commands/scan/mod.rs | 31 +- .../tests/in_process_gem_config_warning.rs | 225 ++++++++++++ .../tests/in_process_gem_fallback_home.rs | 323 ++++++++++++++++++ .../src/crawlers/ruby_crawler.rs | 144 ++++++-- 7 files changed, 943 insertions(+), 60 deletions(-) create mode 100644 crates/socket-patch-cli/tests/in_process_gem_config_warning.rs create mode 100644 crates/socket-patch-cli/tests/in_process_gem_fallback_home.rs diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index d50406bf..8e42ada9 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -292,15 +292,29 @@ the model is **not uniform** today: `///gems/` and flat `/gems/`). The env variable is the user's own machine state, so it is honored verbatim (it may point outside `--cwd`; a leading `~` expands against home); the **config file is typically committed — untrusted input — so a config-sourced root that resolves - outside the project root is skipped with a `gem_bundle_config_path_ignored` stderr warning** - (`BUNDLE_PATH__SYSTEM: "true"` likewise drops the recorded path, as bundler itself ignores it). - Explicit env/config roots only count when `--cwd` holds a Bundler manifest/lockfile. When the - default `vendor/bundle` root holds no store, the gem homes `gem env` reports are appended (default - gems like rexml/json only ever live there). 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`, `setup`, `vex`) use the - highest-precedence copy. + outside the project root is skipped** (`BUNDLE_PATH__SYSTEM: "true"` likewise drops the recorded + path, as bundler itself ignores it). The skip is surfaced per the run-warning conventions: a + `gem_bundle_config_path_ignored` entry in the run-level `warnings[]` of `scan`/`apply` `--json` + envelopes (detail names the config value and the env-`BUNDLE_PATH` remedy), and one stderr + `Warning (gem_bundle_config_path_ignored): …` line on the human path, gated on `!--silent` + (`--silent` = errors only). Explicit env/config roots only count when `--cwd` holds a Bundler + manifest/lockfile. When the default `vendor/bundle` root holds no store, the gem homes `gem env` + reports are appended (default gems like rexml/json only ever live there). 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`, + `setup`, `vex`) use the highest-precedence copy. + + *Copy classes (additive to the multi-copy vocabulary):* a copy under a **bundle-path store** + (config/env/default root) is PRIMARY — a variant mismatch or write failure there fails the run, + as always. A copy in a **`gem env` fallback home** (rvm `@global`, `--user-install`, system gem + dirs — shared, often root-owned) is patched too when it matches and is writable, but becomes + BEST-EFFORT once at least one bundle-store copy applied: its mismatch/write failure surfaces as a + non-fatal `skipped` event (`errorCode: gem_fallback_home_skipped`, detail names the copy's path + and reason; gated stderr twin on the human path) instead of failing a run whose loaded copy is + patched. With **no** bundle-store copy (the historic fallback-only layout, and every `--global` + run) the fallback-home copy IS the primary install and keeps loud-fail parity with pre-bundle-path + `apply`. **Intended (gap):** the cwd-only ecosystems *should* also auto-discover per-subproject lockfiles when run from the repo root, matching the npm workspace model. The npm-vs-others asymmetry is a known diff --git a/crates/socket-patch-cli/src/commands/apply.rs b/crates/socket-patch-cli/src/commands/apply.rs index b58431cc..4e07cdea 100644 --- a/crates/socket-patch-cli/src/commands/apply.rs +++ b/crates/socket-patch-cli/src/commands/apply.rs @@ -1,7 +1,8 @@ use clap::Args; use socket_patch_core::api::client::get_api_client_with_overrides; +use socket_patch_core::crawlers::ruby_crawler::config_path_ignored_warning; use socket_patch_core::crawlers::{ - detect_npm_pkg_manager, CrawlerOptions, Ecosystem, NpmPkgManager, + detect_npm_pkg_manager, CrawlerOptions, Ecosystem, NpmPkgManager, RubyCrawler, }; use socket_patch_core::manifest::operations::read_manifest; use socket_patch_core::manifest::schema::{PatchFileInfo, PatchManifest, PatchRecord}; @@ -24,8 +25,8 @@ use crate::commands::lock_cli::acquire_or_emit; use crate::commands::vex::{generate_vex_from_manifest_path, VexEmbedArgs}; use crate::ecosystem_dispatch::{find_all_packages_for_purls, partition_purls}; use crate::json_envelope::{ - AppliedVia, Command, Envelope, EnvelopeError, PatchAction, PatchEvent, PatchEventFile, Status, - VexSummary, + AppliedVia, Command, Envelope, EnvelopeError, PatchAction, PatchEvent, PatchEventFile, + RunWarning, Status, VexSummary, }; /// Files whose pre-apply content matched NEITHER hash and were (or would @@ -770,12 +771,31 @@ pub async fn run(args: ApplyArgs) -> i32 { } match apply_patches_inner(&args, &manifest_path).await { - Ok((success, results, unmatched)) => { + Ok(ApplyOutcome { + success, + results, + unmatched, + run_warnings, + fallback_skips, + }) => { let patched_count = results .iter() .filter(|r| r.success && !r.files_patched.is_empty()) .count(); + // Run-level advisories + best-effort fallback-home skips on the + // human path: one gated stderr line each. `--silent` is + // errors-only, and under `--json` the envelope copies below are + // the machine channel — same gating as scan's run warnings. + if !args.common.json && !args.common.silent { + for w in &run_warnings { + eprintln!("Warning ({}): {}", w.code, w.detail); + } + for skip in &fallback_skips { + eprintln!("Warning (gem_fallback_home_skipped): {}", skip.detail()); + } + } + // Embedded VEX: only on a successful apply and only when // `--vex ` was passed. Re-read the manifest fresh so // verification observes the just-applied on-disk state. The @@ -835,6 +855,20 @@ pub async fn run(args: ApplyArgs) -> i32 { ), ); } + // Best-effort gem-env fallback-home copies left unpatched: + // one non-fatal Skipped event each, with the copy's path + // and reason. Never a Failed event — the bundle-store copy + // (the one bundler loads) applied, so the run stands. + for skip in &fallback_skips { + env.record( + PatchEvent::new(PatchAction::Skipped, skip.purl.clone()) + .with_reason("gem_fallback_home_skipped", skip.detail()), + ); + } + // Run-level advisories (the gem config-root containment + // skip): the envelope's `warnings[]` is their machine + // channel — stderr is suppressed under --json. + env.warnings.extend(run_warnings.iter().cloned()); if !success { env.mark_partial_failure(); } @@ -1086,10 +1120,54 @@ fn unmatched_purls( .collect() } +/// Everything `apply_patches_inner` reports back to `run`'s output +/// builders (JSON envelope + human summary). +struct ApplyOutcome { + /// Overall success — `false` fails the command (exit 1 / + /// `partialFailure`). + success: bool, + results: Vec, + /// In-scope manifest purls with no installed package on disk. + unmatched: Vec, + /// Run-level advisories: JSON `warnings[]`, one gated stderr line each + /// on the human path (`--silent` = errors only). Today: the gem + /// config-root containment skip. + run_warnings: Vec, + /// Gem-env fallback-home copies deliberately left unpatched + /// (best-effort class): one non-fatal `Skipped` event each in the + /// envelope, one gated stderr line each on the human path. + fallback_skips: Vec, +} + +/// One gem-env fallback-home copy the fan-out skipped best-effort (a +/// bundle-store copy applied; this shared-home copy mismatched or failed +/// to write). Carries what the warning must name: the package, the copy's +/// path, and why. +struct FallbackHomeSkip { + purl: String, + path: PathBuf, + why: String, +} + +impl FallbackHomeSkip { + /// The human/detail text shared by the JSON event reason and the + /// stderr line. + fn detail(&self) -> String { + format!( + "gem-env home copy at {} was not patched ({}); the project's \ + bundle-path copy — the one bundler loads — is patched. Shared \ + gem homes are machine-wide state: patch them explicitly with \ + `--global` if desired", + self.path.display(), + self.why + ) + } +} + async fn apply_patches_inner( args: &ApplyArgs, manifest_path: &Path, -) -> Result<(bool, Vec, Vec), String> { +) -> Result { let manifest = read_manifest(manifest_path) .await .map_err(|e| e.to_string())? @@ -1124,7 +1202,15 @@ async fn apply_patches_inner( let mut staged = match stage_patch_sources(&args.common, &scoped_manifest, socket_dir).await? { StageOutcome::Ready(s) => s, - StageOutcome::Unavailable => return Ok((false, Vec::new(), Vec::new())), + StageOutcome::Unavailable => { + return Ok(ApplyOutcome { + success: false, + results: Vec::new(), + unmatched: Vec::new(), + run_warnings: Vec::new(), + fallback_skips: Vec::new(), + }) + } }; // Vendor ownership wins for EVERY ecosystem: a purl recorded in @@ -1153,6 +1239,45 @@ async fn apply_patches_inner( global_prefix: args.common.global_prefix.clone(), }; + // Gem bundle-store discovery, re-run cheaply (filesystem probes only, + // no `gem env` shell-out) against the same ambient environment the + // crawler reads, for two consumers: + // * the config-skip advisory — a committed `.bundle/config` whose + // BUNDLE_PATH the containment guard refused must surface on the + // run's warning channels (JSON `warnings[]`; gated stderr), not + // vanish silently; + // * the store-class boundary for the gem fan-out below — copies + // under a bundle-path store are primary, everything else is a + // `gem env` fallback-home copy (best-effort once a store copy + // applied). + // Only when this run actually crawls gems locally: a --global run or + // one whose `--ecosystems`/manifest scope holds no gem purls never + // consults the config, so it must not warn about it either. + let gem_discovery = if partitioned.contains_key(&Ecosystem::Gem) + && !args.common.global + && args.common.global_prefix.is_none() + { + Some(RubyCrawler::discover_bundle_stores(&args.common.cwd).await) + } else { + None + }; + let gem_store_dirs: &[PathBuf] = gem_discovery + .as_ref() + .map(|d| d.stores.as_slice()) + .unwrap_or(&[]); + let mut run_warnings: Vec = Vec::new(); + if let Some(value) = gem_discovery + .as_ref() + .and_then(|d| d.skipped_config_path.as_deref()) + { + let (code, detail) = config_path_ignored_warning(value); + run_warnings.push(RunWarning { + code: code.to_string(), + detail, + }); + } + let mut fallback_skips: Vec = Vec::new(); + // Multi-copy aware: npm nests genuine duplicates of one `name@version` // (nested dupes, diamonds, `file:` dups), so the resolver returns EVERY // physical copy per PURL. Patching only one would leave a live, @@ -1175,7 +1300,13 @@ async fn apply_patches_inner( if !args.common.silent && !args.common.json { println!("No patches to apply."); } - return Ok((true, Vec::new(), Vec::new())); + return Ok(ApplyOutcome { + success: true, + results: Vec::new(), + unmatched: Vec::new(), + run_warnings, + fallback_skips, + }); } if all_packages.is_empty() { @@ -1198,7 +1329,13 @@ async fn apply_patches_inner( " Check that packages are installed and --cwd points to the right directory." ); } - return Ok((unmatched.is_empty(), results, unmatched)); + return Ok(ApplyOutcome { + success: unmatched.is_empty(), + results, + unmatched, + run_warnings, + fallback_skips, + }); } // Apply patches @@ -1274,8 +1411,30 @@ async fn apply_patches_inner( std::slice::from_ref(pkg_path) }; + // Copy CLASS decides FAILURE semantics (never write scope — + // patching a shared home's vulnerable copy is fine when it + // works): bundle-path store copies are PRIMARY and loud-fail; + // `gem env` fallback-home copies (rvm `@global`, system gem + // dirs — often root-owned, shared machine-wide) are + // BEST-EFFORT once a store copy applied — a mismatch or write + // failure there becomes a non-fatal per-copy Skipped warning, + // because the copy bundler actually loads is already patched. + // With NO store copy the fallback home IS the primary install + // (the historic pre-bundle-path layout, and every --global + // run) and keeps loud-fail parity. Store copies run first so + // best-effort is decidable when the fallback copies come up. + let (store_copies, home_copies): (Vec<&PathBuf>, Vec<&PathBuf>) = copy_paths + .iter() + .partition(|p| gem_store_dirs.iter().any(|s| p.starts_with(s))); + let mut any_store_copy_applied = false; + let mut any_copy_applied = false; - for pkg_path in copy_paths { + for (pkg_path, is_store_copy) in store_copies + .into_iter() + .map(|p| (p, true)) + .chain(home_copies.into_iter().map(|p| (p, false))) + { + let best_effort = !is_store_copy && any_store_copy_applied; let mut applied = false; // Did at least one variant reach `apply_package_patch`? A // variant reaches it only after passing the first-file @@ -1284,9 +1443,9 @@ async fn apply_patches_inner( // not be reported as "package_not_installed" even if the patch // itself then fails. Tracks the "matched but failed" case so the // failure message is honest and `unmatched` stays accurate. - // Both are PER COPY: a copy that matches no variant must fail - // loudly even when a sibling copy applied cleanly — that copy - // is a real on-disk gem some bundler loads. + // Both are PER COPY: a primary copy that matches no variant + // must fail loudly even when a sibling copy applied cleanly — + // that copy is a real on-disk gem some bundler loads. let mut attempted = false; for variant_purl in &variants { @@ -1358,11 +1517,28 @@ async fn apply_patches_inner( matched_manifest_purls.insert(variant_purl.clone()); if result.success { applied = true; + results.push(result); // No `break`: apply *every* matching variant. PyPI/gem // have exactly one installed distribution (the rest // hash-mismatch and were skipped above), so this // applies a single variant for them; Maven's coexisting // classifier jars each get patched. + } else if best_effort { + // A write failure on a BEST-EFFORT fallback-home copy + // (root-owned rvm `@global`, a system gem dir) is a + // per-copy non-fatal skip, never a run failure: the + // bundle-store copy — the one bundler loads — already + // applied. The failed result is NOT recorded (its + // Failed event would flip `partialFailure`); the skip + // rides `fallback_skips` into the envelope instead. + fallback_skips.push(FallbackHomeSkip { + purl: base_purl.clone(), + path: pkg_path.clone(), + why: result + .error + .clone() + .unwrap_or_else(|| "unknown error".to_string()), + }); } else { // A variant that reached apply IS the installed // distribution, so a failure here is a real apply @@ -1382,19 +1558,37 @@ async fn apply_patches_inner( result.error.as_deref().unwrap_or("unknown error") ); } + results.push(result); } - results.push(result); } if applied { any_copy_applied = true; + if is_store_copy { + any_store_copy_applied = true; + } + } else if best_effort { + // Nothing applied on a best-effort fallback-home copy. + // Attempted-but-failed variants already recorded their + // per-copy skip above; a copy no variant matched gets + // one here — the shared home holds a different (or + // locally diverged) distribution, and the copy bundler + // loads is patched, so this is advisory, not an error. + if !attempted { + fallback_skips.push(FallbackHomeSkip { + purl: base_purl.clone(), + path: pkg_path.clone(), + why: "no release variant in the manifest matches this copy".to_string(), + }); + } } else { - // Nothing applied for this copy. `has_errors` was already set - // per-variant above when a variant was attempted-but-failed; - // set it here too for the no-variant-attempted case so both - // paths fail the command — per copy, so a second store copy - // that matches no variant fails loudly instead of silently - // staying vulnerable behind a sibling copy's success. + // Nothing applied for this PRIMARY copy. `has_errors` was + // already set per-variant above when a variant was + // attempted-but-failed; set it here too for the + // no-variant-attempted case so both paths fail the command + // — per copy, so a second store copy that matches no + // variant fails loudly instead of silently staying + // vulnerable behind a sibling copy's success. has_errors = true; if !attempted && !args.common.silent && !args.common.json { // No variant matched the installed distribution at all — @@ -1523,7 +1717,13 @@ async fn apply_patches_inner( // means it can run repeatedly (CI dry-runs, deploy hooks) without // mutating patch state. - Ok((!has_errors, results, unmatched)) + Ok(ApplyOutcome { + success: !has_errors, + results, + unmatched, + run_warnings, + fallback_skips, + }) } #[cfg(test)] diff --git a/crates/socket-patch-cli/src/commands/scan/gc.rs b/crates/socket-patch-cli/src/commands/scan/gc.rs index 1686734c..e75aff9a 100644 --- a/crates/socket-patch-cli/src/commands/scan/gc.rs +++ b/crates/socket-patch-cli/src/commands/scan/gc.rs @@ -624,11 +624,9 @@ mod tests { let (manifest_path, socket_dir, blob_path) = seed_manifest_with_blob(tmp.path(), "pkg:npm/gone@1.0.0", &after_hash); - let _holder = socket_patch_core::patch::apply_lock::acquire( - &socket_dir, - std::time::Duration::ZERO, - ) - .expect("test holder must win the fresh lock"); + let _holder = + socket_patch_core::patch::apply_lock::acquire(&socket_dir, std::time::Duration::ZERO) + .expect("test holder must win the fresh lock"); let scanned: HashSet = HashSet::new(); let gc = run_apply_gc( diff --git a/crates/socket-patch-cli/src/commands/scan/mod.rs b/crates/socket-patch-cli/src/commands/scan/mod.rs index 84ddb1d6..b97e2dd7 100644 --- a/crates/socket-patch-cli/src/commands/scan/mod.rs +++ b/crates/socket-patch-cli/src/commands/scan/mod.rs @@ -11,7 +11,8 @@ use socket_patch_core::api::client::{ build_proxy_fallback_client, get_api_client_with_overrides, is_fallback_candidate, }; use socket_patch_core::api::types::{BatchPackagePatches, PatchSearchResult}; -use socket_patch_core::crawlers::{CrawlerOptions, Ecosystem}; +use socket_patch_core::crawlers::ruby_crawler::config_path_ignored_warning; +use socket_patch_core::crawlers::{CrawlerOptions, Ecosystem, RubyCrawler}; use socket_patch_core::manifest::operations::read_manifest; use socket_patch_core::manifest::schema::PatchManifest; use socket_patch_core::telemetry::{track_patch_scan_failed, track_patch_scanned}; @@ -1451,7 +1452,33 @@ pub async fn run(mut args: ScanArgs) -> i32 { // empty) and a stderr line on the human path; exit code and `status` // stay deliberately unchanged (same posture as hosted refusals, which // exit 0 with `redirected: 0`). - let layout_refusals = unsupported_layout_warnings(&lockfile_only); + let mut layout_refusals = unsupported_layout_warnings(&lockfile_only); + // Config-sourced gem bundle root refused by the crawler's containment + // guard (a committed `.bundle/config` whose BUNDLE_PATH resolves + // outside the project — untrusted input that would otherwise become a + // scan/apply WRITE-target root). The crawl above consulted and + // silently skipped it; surface the skip on the SAME run-level channel + // as the layout refusals (JSON `warnings[]` on both the zero-package + // and ≥1-package envelopes; a gated stderr line on the human path). + // Scoped like the crawl that hit it: local mode, with gem not filtered + // out by `--ecosystems`. Cheap re-probe: filesystem only, no `gem env` + // shell-out. + if !crawler_options.global + && crawler_options.global_prefix.is_none() + && args + .common + .ecosystems + .as_ref() + .is_none_or(|list| list.iter().any(|e| e == Ecosystem::Gem.cli_name())) + { + if let Some(value) = RubyCrawler::discover_bundle_stores(&args.common.cwd) + .await + .skipped_config_path + { + let (code, detail) = config_path_ignored_warning(&value); + layout_refusals.push((code.to_string(), detail)); + } + } if !lockfile_only.packages.is_empty() { for pkg in &lockfile_only.packages { if let Some(eco) = Ecosystem::from_purl(&pkg.purl) { diff --git a/crates/socket-patch-cli/tests/in_process_gem_config_warning.rs b/crates/socket-patch-cli/tests/in_process_gem_config_warning.rs new file mode 100644 index 00000000..d8bcc5ae --- /dev/null +++ b/crates/socket-patch-cli/tests/in_process_gem_config_warning.rs @@ -0,0 +1,225 @@ +//! Machine-readable surfacing of the gem config-root containment skip. +//! +//! A repo-committed `.bundle/config` whose `BUNDLE_PATH` resolves outside +//! the project root is SKIPPED by the crawler's containment guard (see +//! `resolve_config_bundle_path`). That skip must be observable per the +//! repo-wide warning conventions: +//! * `--json` envelopes of scan and apply carry a run-level `warnings[]` +//! entry with code `gem_bundle_config_path_ignored` and the config +//! value + remedy in `detail`; +//! * non-JSON runs print ONE stderr warning, gated on `!--silent` +//! (`--silent` = errors only — a bare crawler eprintln violated that). + +use std::path::{Path, PathBuf}; +use std::process::Command; + +use sha2::{Digest, Sha256}; + +const PURL: &str = "pkg:gem/rack@3.1.0"; +const CODE: &str = "gem_bundle_config_path_ignored"; + +fn binary() -> PathBuf { + env!("CARGO_BIN_EXE_socket-patch").into() +} + +fn git_sha256(content: &[u8]) -> String { + let header = format!("blob {}\0", content.len()); + let mut hasher = Sha256::new(); + hasher.update(header.as_bytes()); + hasher.update(content); + hex::encode(hasher.finalize()) +} + +/// Project fixture: Gemfile + `.bundle/config` pointing `BUNDLE_PATH` at +/// an ABSOLUTE directory outside the project (holding a real store, so +/// the only reason it goes undiscovered is the containment skip), plus — +/// when `with_store` — a legit `vendor/bundle` scoped store carrying a +/// patchable rack@3.1.0 and a staged manifest + blob. +fn build_project(root: &Path, outside: &Path, with_store: bool) { + std::fs::write(root.join("Gemfile"), b"source 'https://rubygems.org'\n").unwrap(); + std::fs::create_dir_all(root.join(".bundle")).unwrap(); + std::fs::write( + root.join(".bundle").join("config"), + format!("---\nBUNDLE_PATH: \"{}\"\n", outside.display()), + ) + .unwrap(); + // The out-of-tree store the config names. + std::fs::create_dir_all(outside.join("gems").join("rack-3.1.0").join("lib")).unwrap(); + std::fs::create_dir_all(outside.join("specifications")).unwrap(); + + if with_store { + let original = b"module Rack\n VERSION = 'VULNERABLE'\nend\n"; + let mut patched = original.to_vec(); + patched.extend_from_slice(b"# SOCKET-PATCHED\n"); + let before_hash = git_sha256(original); + let after_hash = git_sha256(&patched); + + let gem_lib = root + .join("vendor") + .join("bundle") + .join("ruby") + .join("3.2.0") + .join("gems") + .join("rack-3.1.0") + .join("lib"); + std::fs::create_dir_all(&gem_lib).unwrap(); + std::fs::write(gem_lib.join("rack.rb"), original).unwrap(); + + let socket = root.join(".socket"); + std::fs::create_dir_all(socket.join("blobs")).unwrap(); + std::fs::write( + socket.join("manifest.json"), + format!( + r#"{{ "patches": {{ + "{PURL}": {{ + "uuid": "636f6e66-6967-4761-8264-000000000000", + "exportedAt": "2024-01-01T00:00:00Z", + "files": {{ "lib/rack.rb": {{ + "beforeHash": "{before_hash}", "afterHash": "{after_hash}" + }}}}, + "vulnerabilities": {{}}, "description": "config-warning fixture", + "license": "MIT", "tier": "free" + }} + }}}}"# + ), + ) + .unwrap(); + std::fs::write(socket.join("blobs").join(&after_hash), &patched).unwrap(); + } +} + +/// Run the binary with SOCKET_*/BUNDLE_* scrubbed and PATH pointed at an +/// empty dir, so no ambient bundler config and no real `gem` binary can +/// perturb discovery. +fn run(root: &Path, args: &[&str]) -> (i32, String, String) { + let empty_path = root.join("empty-bin"); + std::fs::create_dir_all(&empty_path).unwrap(); + let mut cmd = Command::new(binary()); + cmd.args(args).arg("--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"); + cmd.env("PATH", &empty_path); + 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(), + ) +} + +fn assert_warning_in(env: &serde_json::Value, value_fragment: &str, ctx: &str) { + let warnings = env + .get("warnings") + .and_then(|w| w.as_array()) + .unwrap_or_else(|| panic!("{ctx}: envelope must carry warnings[].\nenvelope: {env}")); + let w = warnings + .iter() + .find(|w| w.get("code").and_then(|c| c.as_str()) == Some(CODE)) + .unwrap_or_else(|| panic!("{ctx}: warnings[] must contain code={CODE}.\nenvelope: {env}")); + let detail = w + .get("detail") + .and_then(|d| d.as_str()) + .unwrap_or_else(|| panic!("{ctx}: warning must carry detail.\nenvelope: {env}")); + assert!( + detail.contains(value_fragment), + "{ctx}: detail must name the skipped config value ({value_fragment}); got: {detail}" + ); + assert!( + detail.contains("BUNDLE_PATH"), + "{ctx}: detail must explain the remedy in terms of BUNDLE_PATH; got: {detail}" + ); +} + +/// `scan --json` on a project whose committed config points out of tree +/// must carry the machine-readable warning (zero-package early-return +/// path: no `gem` binary + skipped store = nothing discovered, exactly +/// the run where the skip explains the emptiness). +#[test] +fn scan_json_carries_config_path_ignored_warning() { + let tmp = tempfile::tempdir().unwrap(); + let outside = tempfile::tempdir().unwrap(); + build_project(tmp.path(), outside.path(), false); + + let (code, stdout, stderr) = run(tmp.path(), &["scan", "--json", "--yes"]); + assert_eq!( + code, 0, + "scan exits 0.\nstdout:\n{stdout}\nstderr:\n{stderr}" + ); + let env: serde_json::Value = serde_json::from_str(stdout.trim()) + .unwrap_or_else(|e| panic!("scan must emit JSON: {e}; stdout={stdout}")); + let outside_str = outside.path().display().to_string(); + assert_warning_in(&env, &outside_str, "scan --json"); +} + +/// `apply --json` must carry the same run-level warning while the legit +/// `vendor/bundle` store still patches cleanly (exit 0, applied 1). +#[test] +fn apply_json_carries_config_path_ignored_warning() { + let tmp = tempfile::tempdir().unwrap(); + let outside = tempfile::tempdir().unwrap(); + build_project(tmp.path(), outside.path(), true); + + let (code, stdout, stderr) = run( + tmp.path(), + &["apply", "--json", "--offline", "--ecosystems", "gem"], + ); + let env: serde_json::Value = serde_json::from_str(stdout.trim()) + .unwrap_or_else(|e| panic!("apply must emit JSON: {e}; stdout={stdout}")); + assert_eq!( + code, 0, + "apply exits 0.\nenvelope: {env}\nstderr:\n{stderr}" + ); + assert_eq!(env["status"], "success", "envelope: {env}"); + assert_eq!( + env["summary"]["applied"], 1, + "the in-tree store copy still patches.\nenvelope: {env}" + ); + let outside_str = outside.path().display().to_string(); + assert_warning_in(&env, &outside_str, "apply --json"); +} + +/// Non-JSON runs: exactly one gated stderr warning. Loud control first +/// (without --silent the warning must appear, with the code and the +/// config value), then the gate (--silent = errors only: no warning). +#[test] +fn apply_stderr_warning_gates_on_silent() { + let tmp = tempfile::tempdir().unwrap(); + let outside = tempfile::tempdir().unwrap(); + build_project(tmp.path(), outside.path(), true); + + // Loud control: the warning must reach stderr (proves the silent + // assertion below isn't passing vacuously). + let (code, _stdout, stderr) = run(tmp.path(), &["apply", "--offline", "--ecosystems", "gem"]); + assert_eq!(code, 0, "loud apply exits 0.\nstderr:\n{stderr}"); + assert!( + stderr.contains(CODE), + "non-silent stderr must carry the {CODE} warning; got:\n{stderr}" + ); + assert_eq!( + stderr.matches(CODE).count(), + 1, + "exactly ONE warning line (not one per discovery call); got:\n{stderr}" + ); + assert!( + stderr.contains(&outside.path().display().to_string()), + "the warning must name the config value; got:\n{stderr}" + ); + + // Reset the store bytes so the second run applies identically. + // (apply is idempotent — already-patched just skips — so no reset + // is actually required for exit semantics; keep the run as-is.) + let (code, _stdout, stderr) = run( + tmp.path(), + &["apply", "--offline", "--ecosystems", "gem", "--silent"], + ); + assert_eq!(code, 0, "silent apply exits 0.\nstderr:\n{stderr}"); + assert!( + !stderr.contains(CODE), + "--silent is errors-only: the {CODE} warning must not print; got:\n{stderr}" + ); +} diff --git a/crates/socket-patch-cli/tests/in_process_gem_fallback_home.rs b/crates/socket-patch-cli/tests/in_process_gem_fallback_home.rs new file mode 100644 index 00000000..60c18ae8 --- /dev/null +++ b/crates/socket-patch-cli/tests/in_process_gem_fallback_home.rs @@ -0,0 +1,323 @@ +//! Store-class semantics for the gem multi-copy fan-out. +//! +//! `get_gem_paths` appends the `gem env` fallback homes (rvm `@global`, +//! `--user-install`, system gem dirs) after the project's bundle-path +//! stores, and the multi-copy fan-out patches EVERY copy. Patching a +//! shared home's copy is fine (vulnerable bytes are vulnerable bytes) — +//! but its FAILURE semantics must not cross store classes: +//! +//! * copies in bundle-path stores are PRIMARY — loud-fail (unchanged); +//! * copies in gem-env fallback homes are BEST-EFFORT once at least one +//! bundle-store copy applied: a variant mismatch or write failure +//! there becomes a per-copy non-fatal `Skipped` event +//! (`gem_fallback_home_skipped`, with the path and reason), never a +//! run failure — the copy bundler actually loads was patched; +//! * with NO bundle-store copy (the historic fallback-only layout) the +//! home copy IS primary: loud-fail exactly as plain apply always +//! behaved. +//! +//! Unix-only: the fallback homes come from a fake `gem` binary on PATH. + +#![cfg(unix)] + +use std::path::{Path, PathBuf}; +use std::process::Command; + +use sha2::{Digest, Sha256}; + +const BASE_PURL: &str = "pkg:gem/rack@3.1.0"; +const QUALIFIED_PURL: &str = "pkg:gem/rack@3.1.0?platform=ruby"; +const SKIP_CODE: &str = "gem_fallback_home_skipped"; + +const ORIGINAL: &[u8] = b"module Rack\n VERSION = 'VULNERABLE'\nend\n"; +const MARKER: &[u8] = b"# SOCKET-PATCHED-FALLBACK\n"; + +fn binary() -> PathBuf { + env!("CARGO_BIN_EXE_socket-patch").into() +} + +fn git_sha256(content: &[u8]) -> String { + let header = format!("blob {}\0", content.len()); + let mut hasher = Sha256::new(); + hasher.update(header.as_bytes()); + hasher.update(content); + hex::encode(hasher.finalize()) +} + +fn patched_bytes() -> Vec { + let mut v = ORIGINAL.to_vec(); + v.extend_from_slice(MARKER); + v +} + +/// Fake `gem` answering `env gemdir` with `home` (and an empty `gempath` +/// failure), so the crawler's fallback resolves to exactly one home. +fn install_fake_gem(bin_dir: &Path, home: &Path) { + use std::os::unix::fs::PermissionsExt; + let script = format!( + "#!/bin/sh\nif [ \"$1\" = env ] && [ \"$2\" = gemdir ]; then\n printf '%s\\n' \"{}\"\n exit 0\nfi\nexit 1\n", + home.display() + ); + let bin = bin_dir.join("gem"); + std::fs::write(&bin, script).unwrap(); + std::fs::set_permissions(&bin, std::fs::Permissions::from_mode(0o755)).unwrap(); +} + +/// Stage a gem copy (`/gems/rack-3.1.0/lib/rack.rb` = `bytes`) plus +/// the `specifications/` marker; returns the staged file path. +fn stage_copy(root: &Path, bytes: &[u8]) -> PathBuf { + let file = root + .join("gems") + .join("rack-3.1.0") + .join("lib") + .join("rack.rb"); + std::fs::create_dir_all(file.parent().unwrap()).unwrap(); + std::fs::write(&file, bytes).unwrap(); + std::fs::create_dir_all(root.join("specifications")).unwrap(); + file +} + +fn stage_manifest(root: &Path, purl: &str) { + let before_hash = git_sha256(ORIGINAL); + let after_hash = git_sha256(&patched_bytes()); + let socket = root.join(".socket"); + std::fs::create_dir_all(socket.join("blobs")).unwrap(); + std::fs::write( + socket.join("manifest.json"), + format!( + r#"{{ "patches": {{ + "{purl}": {{ + "uuid": "66616c6c-6261-4b63-8368-6f6d65000000", + "exportedAt": "2024-01-01T00:00:00Z", + "files": {{ "lib/rack.rb": {{ + "beforeHash": "{before_hash}", "afterHash": "{after_hash}" + }}}}, + "vulnerabilities": {{}}, "description": "fallback-home fixture", + "license": "MIT", "tier": "free" + }} + }}}}"# + ), + ) + .unwrap(); + std::fs::write(socket.join("blobs").join(&after_hash), patched_bytes()).unwrap(); +} + +struct Fixture { + project: tempfile::TempDir, + store_file: Option, + home_file: PathBuf, + bin_dir: PathBuf, + store_root: Option, +} + +/// Project with a Gemfile; optionally an env-`BUNDLE_PATH` store copy +/// (pristine); a gem-env fallback home copy with `home_bytes`; manifest +/// keyed by `purl`. +fn build_fixture(with_store: bool, home_bytes: &[u8], purl: &str) -> Fixture { + let project = tempfile::tempdir().unwrap(); + let root = project.path(); + std::fs::write(root.join("Gemfile"), b"source 'https://rubygems.org'\n").unwrap(); + + let store_root = with_store.then(|| root.join("bundle-store")); + let store_file = store_root.as_ref().map(|s| stage_copy(s, ORIGINAL)); + + let home = root.join("gem-home"); + let home_file = stage_copy(&home, home_bytes); + + let bin_dir = root.join("fake-bin"); + std::fs::create_dir_all(&bin_dir).unwrap(); + install_fake_gem(&bin_dir, &home); + + stage_manifest(root, purl); + Fixture { + project, + store_file, + home_file, + bin_dir, + store_root, + } +} + +/// Run apply with SOCKET_*/BUNDLE_* scrubbed, PATH = the fake-gem bin dir +/// only, and BUNDLE_PATH set to the store root when present. +fn run_apply(fx: &Fixture, extra: &[&str]) -> (i32, String, String) { + let mut cmd = Command::new(binary()); + cmd.args(["apply", "--offline", "--ecosystems", "gem", "--cwd"]) + .arg(fx.project.path()) + .args(extra); + 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"); + cmd.env("PATH", &fx.bin_dir); + if let Some(store_root) = &fx.store_root { + cmd.env("BUNDLE_PATH", store_root); + } + let out = cmd.output().expect("run socket-patch apply"); + ( + out.status.code().unwrap_or(-1), + String::from_utf8_lossy(&out.stdout).to_string(), + String::from_utf8_lossy(&out.stderr).to_string(), + ) +} + +fn parse_env(stdout: &str) -> serde_json::Value { + serde_json::from_str(stdout.trim()) + .unwrap_or_else(|e| panic!("apply must emit JSON: {e}; stdout={stdout}")) +} + +fn find_skip_event<'a>(env: &'a serde_json::Value) -> Option<&'a serde_json::Value> { + env["events"].as_array().and_then(|events| { + events.iter().find(|e| { + e["action"] == "skipped" + // `with_reason` serializes its stable tag as `errorCode`. + && e.get("errorCode").and_then(|c| c.as_str()) == Some(SKIP_CODE) + }) + }) +} + +/// (a) Store copy patched + home copy that matches NO variant (foreign +/// bytes, qualified record): the home copy becomes a per-copy non-fatal +/// `Skipped` event naming its path — exit 0, `applied` counts the store +/// copy. Before the fix this failed the WHOLE run ("no matching variant +/// found", exit 1) even though the copy bundler loads patched fine. +#[test] +fn mismatched_fallback_home_copy_is_nonfatal_when_store_patched() { + let foreign = b"totally different bytes\n"; + let fx = build_fixture(true, foreign, QUALIFIED_PURL); + + let (code, stdout, stderr) = run_apply(&fx, &["--json"]); + let env = parse_env(&stdout); + assert_eq!( + code, 0, + "a mismatched fallback-home copy must not fail the run.\nenvelope: {env}\nstderr:\n{stderr}" + ); + assert_eq!(env["status"], "success", "envelope: {env}"); + assert_eq!( + env["summary"]["applied"], 1, + "the bundle-store copy counts as applied.\nenvelope: {env}" + ); + assert_eq!( + std::fs::read(fx.store_file.as_ref().unwrap()).unwrap(), + patched_bytes(), + "store copy must be patched" + ); + assert_eq!( + std::fs::read(&fx.home_file).unwrap(), + foreign, + "the mismatched home copy must be left untouched" + ); + + let skip = find_skip_event(&env).unwrap_or_else(|| { + panic!("envelope must carry a skipped event with reasonCode={SKIP_CODE}.\nenvelope: {env}") + }); + assert_eq!(skip["purl"], BASE_PURL, "envelope: {env}"); + let reason = skip["reason"].as_str().unwrap_or_default(); + assert!( + reason.contains( + &fx.home_file + .parent() + .unwrap() + .parent() + .unwrap() + .display() + .to_string() + ) || reason.contains(&fx.home_file.display().to_string()), + "skip reason must name the fallback-home copy path; got: {reason}" + ); +} + +/// (b) Parity pin: with NO bundle-store copy, the fallback-home copy IS +/// the primary install — a mismatch there keeps the historic loud +/// failure (exit 1), exactly as plain apply behaved before #218. +#[test] +fn fallback_only_mismatch_keeps_loud_failure_parity() { + let fx = build_fixture(false, b"totally different bytes\n", QUALIFIED_PURL); + + let (code, stdout, _stderr) = run_apply(&fx, &["--json"]); + let env = parse_env(&stdout); + assert_ne!( + code, 0, + "fallback-only mismatch must still fail the run (parity).\nenvelope: {env}" + ); + assert!( + find_skip_event(&env).is_none(), + "no best-effort skip without a patched store copy.\nenvelope: {env}" + ); +} + +/// (c) Both copies healthy: both patched (the multi-copy guarantee is +/// unchanged by the class split), exit 0, applied counts both. +#[test] +fn writable_fallback_home_copy_still_patched() { + let fx = build_fixture(true, ORIGINAL, QUALIFIED_PURL); + + let (code, stdout, stderr) = run_apply(&fx, &["--json"]); + let env = parse_env(&stdout); + assert_eq!(code, 0, "envelope: {env}\nstderr:\n{stderr}"); + assert_eq!( + env["summary"]["applied"], 2, + "both copies count.\nenvelope: {env}" + ); + assert_eq!( + std::fs::read(fx.store_file.as_ref().unwrap()).unwrap(), + patched_bytes() + ); + assert_eq!(std::fs::read(&fx.home_file).unwrap(), patched_bytes()); + assert!( + find_skip_event(&env).is_none(), + "healthy home copies are patched, never best-effort-skipped.\nenvelope: {env}" + ); +} + +/// An ATTEMPTED apply that FAILS on the fallback-home copy converts to +/// the same non-fatal per-copy skip when the store copy applied. The +/// deterministic non-root failure: an UNQUALIFIED record's home copy with +/// locally-diverged bytes under `--strict` — the singleton exemption +/// drives it past the variant gate into `apply_package_patch`, whose +/// strict mismatch policy then refuses the write. (A root-owned rvm +/// `@global` EACCES is the production shape; owner-run tests can't +/// produce it — `DirWriteGuard` defeats read-only-dir setups by design.) +/// Before the fix: a `Failed` event + exit 1 even though the copy bundler +/// loads patched fine. +#[test] +fn failing_fallback_home_copy_write_is_nonfatal_when_store_patched() { + let mut diverged = ORIGINAL.to_vec(); + diverged.extend_from_slice(b"# local tweak\n"); + let fx = build_fixture(true, &diverged, BASE_PURL); + + let (code, stdout, stderr) = run_apply(&fx, &["--json", "--strict"]); + let env = parse_env(&stdout); + assert_eq!( + code, 0, + "a failing fallback-home write must not fail the run once the store copy applied.\nenvelope: {env}\nstderr:\n{stderr}" + ); + assert_eq!(env["status"], "success", "envelope: {env}"); + assert_eq!( + std::fs::read(fx.store_file.as_ref().unwrap()).unwrap(), + patched_bytes(), + "store copy must be patched" + ); + assert_eq!( + std::fs::read(&fx.home_file).unwrap(), + diverged, + "strict refusal leaves the diverged home copy untouched" + ); + let skip = find_skip_event(&env).unwrap_or_else(|| { + panic!("write failure must surface as a {SKIP_CODE} skipped event.\nenvelope: {env}") + }); + assert_eq!(skip["purl"], BASE_PURL, "envelope: {env}"); + let failed_events = env["events"] + .as_array() + .unwrap() + .iter() + .filter(|e| e["action"] == "failed") + .count(); + assert_eq!( + failed_events, 0, + "no Failed event for a best-effort home copy.\nenvelope: {env}" + ); +} diff --git a/crates/socket-patch-core/src/crawlers/ruby_crawler.rs b/crates/socket-patch-core/src/crawlers/ruby_crawler.rs index 276676d2..b6bc0f0f 100644 --- a/crates/socket-patch-core/src/crawlers/ruby_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/ruby_crawler.rs @@ -236,14 +236,7 @@ impl RubyCrawler { /// these two are the unit-test seam pinning the store list shape. #[cfg(test)] async fn get_vendor_bundle_paths(cwd: &Path) -> Vec { - Self::discover_bundle_stores_with_env( - cwd, - std::env::var_os("BUNDLE_PATH").as_deref(), - std::env::var_os("BUNDLE_APP_CONFIG").as_deref(), - ambient_home().as_deref(), - ) - .await - .stores + Self::discover_bundle_stores(cwd).await.stores } /// [`Self::discover_bundle_stores_with_env`], flattened to just the @@ -300,17 +293,16 @@ impl RubyCrawler { let default_root = normalize_lexically(&default_root).unwrap_or(default_root); let mut roots: Vec = Vec::new(); + let mut skipped_config_path = None; if Self::has_bundler_manifest(cwd).await { if let Some(value) = Self::app_config_bundle_path(cwd, app_config_env).await { match resolve_config_bundle_path(cwd, &value, home) { Some(root) => roots.push(root), - None => eprintln!( - "Warning (gem_bundle_config_path_ignored): bundler app config \ - BUNDLE_PATH {value:?} resolves outside the project root; \ - ignoring it as an install root (a committed .bundle/config is \ - untrusted input — set BUNDLE_PATH in the environment to use an \ - out-of-tree bundle path)" - ), + // Refused by the containment guard. Recorded — not + // printed: the crawler has no --silent/--json context, + // so the CLI surfaces it (see + // [`config_path_ignored_warning`]). + None => skipped_config_path = Some(value), } } if let Some(v) = bundle_path_env.filter(|v| !v.is_empty()) { @@ -340,9 +332,26 @@ impl RubyCrawler { BundleStoreDiscovery { stores, default_root_has_stores, + skipped_config_path, } } + /// Local-mode Bundler install-root discovery against the AMBIENT + /// environment — the same probe [`Self::get_gem_paths`] runs, exposed + /// for CLI consumers that need what the flat path list drops: the + /// store/fallback CLASS boundary (`stores`) and the config-skip + /// advisory (`skipped_config_path`). Cheap: filesystem probes only, + /// no `gem env` shell-out. + pub async fn discover_bundle_stores(cwd: &Path) -> BundleStoreDiscovery { + Self::discover_bundle_stores_with_env( + cwd, + std::env::var_os("BUNDLE_PATH").as_deref(), + std::env::var_os("BUNDLE_APP_CONFIG").as_deref(), + ambient_home().as_deref(), + ) + .await + } + /// The installed-gem `gems/` dirs under one bundler install root, in /// both layouts bundler produces: /// @@ -632,15 +641,45 @@ impl Default for RubyCrawler { } } -/// Result of probing the Bundler install roots: the discovered -/// installed-gem `gems/` stores in root-precedence order, plus whether any -/// of them sit under the implicit project-local `vendor/bundle` root — -/// which keeps its historic [`RubyCrawler::get_gem_paths`] early-return -/// (a deployment install suppresses the `gem env` fallback; env/config -/// roots must not). -struct BundleStoreDiscovery { - stores: Vec, - default_root_has_stores: bool, +/// Result of probing the Bundler install roots. +/// +/// Public so CLI consumers (apply's store-class split, scan/apply's +/// config-skip advisory) can see what local-mode discovery decided; the +/// crawler itself stays print-free — surfacing the skip on stderr / the +/// JSON envelope is the CLI's job, where `--silent`/`--json` gating lives. +pub struct BundleStoreDiscovery { + /// The discovered installed-gem `gems/` stores, in root-precedence + /// order (local config > env > default `vendor/bundle`). Copies found + /// under these are the PRIMARY class in apply's multi-copy fan-out; + /// paths outside them are `gem env` fallback-home copies. + pub stores: Vec, + /// Whether any store sits under the implicit project-local + /// `vendor/bundle` root — which keeps its historic + /// [`RubyCrawler::get_gem_paths`] early-return (a deployment install + /// suppresses the `gem env` fallback; env/config roots must not). + pub default_root_has_stores: bool, + /// A config-sourced `BUNDLE_PATH` value the containment guard REFUSED + /// (it resolved outside the project root — see + /// [`resolve_config_bundle_path`]). Recorded, never printed: callers + /// surface it via [`config_path_ignored_warning`] on their own + /// warning channel. + pub skipped_config_path: Option, +} + +/// The stable warning `(code, detail)` for a config-sourced `BUNDLE_PATH` +/// refused by the containment guard. One builder so scan's run-level +/// `warnings[]`, apply's envelope `warnings[]`, and the gated stderr lines +/// all carry byte-identical text. +pub fn config_path_ignored_warning(value: &str) -> (&'static str, String) { + ( + "gem_bundle_config_path_ignored", + format!( + "bundler app config BUNDLE_PATH {value:?} resolves outside the project \ + root; ignoring it as an install root (a committed .bundle/config is \ + untrusted input — set BUNDLE_PATH in the environment to use an \ + out-of-tree bundle path)" + ), + ) } /// The ambient home directory as an env value (`HOME`, else Windows' @@ -1489,6 +1528,63 @@ mod tests { ); } + /// The containment refusal is RECORDED on the discovery result (for + /// the CLI's warning channels), keyed by the verbatim config value — + /// and stays `None` for a contained value or a `path.system` drop + /// (bundler itself ignores the path there; nothing was refused). + #[tokio::test] + async fn discovery_records_skipped_config_path() { + let dir = tempfile::tempdir().unwrap(); + let outside = tempfile::tempdir().unwrap(); + tokio::fs::write(dir.path().join("Gemfile"), b"gem \"foo\"\n") + .await + .unwrap(); + tokio::fs::create_dir_all(dir.path().join(".bundle")) + .await + .unwrap(); + let value = outside.path().display().to_string(); + tokio::fs::write( + dir.path().join(".bundle").join("config"), + format!("---\nBUNDLE_PATH: \"{value}\"\n"), + ) + .await + .unwrap(); + + let discovery = + RubyCrawler::discover_bundle_stores_with_env(dir.path(), None, None, None).await; + assert_eq!( + discovery.skipped_config_path.as_deref(), + Some(value.as_str()), + "the refused config value must be recorded verbatim" + ); + // The warning builder names the value and carries the stable code. + let (code, detail) = config_path_ignored_warning(&value); + assert_eq!(code, "gem_bundle_config_path_ignored"); + assert!(detail.contains(&value) && detail.contains("BUNDLE_PATH")); + + // Contained value → no skip recorded. + tokio::fs::write( + dir.path().join(".bundle").join("config"), + "---\nBUNDLE_PATH: \"vendor/mygems\"\n", + ) + .await + .unwrap(); + let discovery = + RubyCrawler::discover_bundle_stores_with_env(dir.path(), None, None, None).await; + assert_eq!(discovery.skipped_config_path, None); + + // path.system=true → bundler ignores the path; not a refusal. + tokio::fs::write( + dir.path().join(".bundle").join("config"), + format!("---\nBUNDLE_PATH: \"{value}\"\nBUNDLE_PATH__SYSTEM: \"true\"\n"), + ) + .await + .unwrap(); + let discovery = + RubyCrawler::discover_bundle_stores_with_env(dir.path(), None, None, None).await; + assert_eq!(discovery.skipped_config_path, None); + } + /// Unit contract for the config-root containment policy itself. /// Real (absolute) tempdir paths keep the assertions valid on Windows, /// where a `/`-rooted literal is NOT absolute.