Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 33 additions & 1 deletion crates/socket-patch-cli/CLI_CONTRACT.md
Original file line number Diff line number Diff line change
Expand Up @@ -278,12 +278,44 @@ 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 `<cwd>/vendor/bundle/...`; pypi at `<cwd>/.venv`); it does **not**
rooted at `--cwd` (pypi looks at `<cwd>/.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 `<cwd>/.bundle/config` — what
`bundle config set --local path` records), then the **`BUNDLE_PATH` environment variable**, then the
default `<cwd>/vendor/bundle` — each in both store layouts bundler produces (scoped
`<root>/<engine>/<abi>/gems/` and flat `<root>/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** (`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
defect, guarded by the `#[ignore]`d gap pin
Expand Down
477 changes: 355 additions & 122 deletions crates/socket-patch-cli/src/commands/apply.rs

Large diffs are not rendered by default.

8 changes: 3 additions & 5 deletions crates/socket-patch-cli/src/commands/scan/gc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> = HashSet::new();
let gc = run_apply_gc(
Expand Down
31 changes: 29 additions & 2 deletions crates/socket-patch-cli/src/commands/scan/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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) {
Expand Down
155 changes: 142 additions & 13 deletions crates/socket-patch-cli/src/ecosystem_dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<engine>/<abi>/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<String, Vec<PathBuf>>,
_purls: &[String],
packages: HashMap<String, CrawledPackage>,
) {
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
Expand Down Expand Up @@ -382,7 +406,14 @@ pub async fn find_all_packages_for_purls(
options: &CrawlerOptions,
silent: bool,
) -> HashMap<String, Vec<PathBuf>> {
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
Expand Down Expand Up @@ -535,7 +566,10 @@ mod tests {
let mut out: HashMap<String, Vec<PathBuf>> = 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]
Expand All @@ -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<String, Vec<PathBuf>> = 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]
Expand Down Expand Up @@ -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<String, Vec<PathBuf>> = 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]
Expand Down Expand Up @@ -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")]));
}

Expand Down Expand Up @@ -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
/// (`<engine>/<abi>/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
Expand Down
Loading
Loading