Repository navigation
Fix gem crawl ignoring Bundler path.system (#915) - #916
Open
Mikola Lysenko (mikolalysenko) wants to merge 4 commits into
Open
Mikola Lysenko (mikolalysenko) wants to merge 4 commits into
Mikola Lysenko (mikolalysenko) wants to merge 4 commits into
Conversation
Assisted-by: Claude Code:claude-opus-5-5
A project that switched from vendor/bundle back to system gems (`bundle config set path.system true`, or BUNDLE_PATH__SYSTEM=true) usually still has the old gitignored vendor/bundle. The gem crawler always counted that leftover store and, because it held gems, stopped looking in the `gem env` homes. Agent apply then patched only the unused copy, and vex attested not_affected while Bundler kept loading the unpatched system gem. The crawler now works out which Bundler settings tier decides the install path (local config, then environment, then global config) and, when that tier sets a truthy path.system, skips the default vendor/bundle root so the system gem homes are crawled. path.system values now follow Bundler's own boolean coercion, so "1" or "yes" count as true too. Fixes #915 Assisted-by: Claude Code:claude-opus-5-5
main has failed socket-patch-core's lib tests since Gradle support (#646) and the digest helpers (#865) both landed. The guard test production_digests_go_through_the_helpers flags three files #646 added that still hash inline: crawlers/gradle_cache.rs, patch/jvm_jar.rs and patch/sidecars/maven.rs. That breaks test, test-release and coverage on every open PR. Each inline sha1/sha256 call now goes through sha1_hex_of or sha256_hex_of, which compute the same lowercase hex. Behaviour is unchanged. Assisted-by: Claude Code:claude-opus-5-5 (cherry picked from commit 659ac2c)
Collaborator
Author
|
[agent] Generated by Claude Code |
Mikola Lysenko (mikolalysenko)
marked this pull request as ready for review
October 6, 2026 05:06
Collaborator
Author
|
BugBot review Generated by Claude Code |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Env path ignores winning path.system
- Moved uses_system_gems computation earlier and gated env BUNDLE_PATH addition with it, ensuring both env path and default root are skipped when path.system is truthy.
Or push these changes by commenting:
@cursor push 204a52709d
Preview (204a52709d)
diff --git a/crates/socket-patch-cli/src/commands/list.rs b/crates/socket-patch-cli/src/commands/list.rs
--- a/crates/socket-patch-cli/src/commands/list.rs
+++ b/crates/socket-patch-cli/src/commands/list.rs
@@ -431,7 +431,10 @@
detail: detail.clone(),
});
} else if !args.common.silent {
- eprintln!("Warning: {}", crate::commands::rollback::capitalize_first(detail));
+ eprintln!(
+ "Warning: {}",
+ crate::commands::rollback::capitalize_first(detail)
+ );
}
}
let vendor_state = crate::commands::vendor_state_lenient(&loaded.vendor, args.common.silent);
@@ -773,12 +776,18 @@
let listings = HostedListing::from_pins(
&[
pin("pkg:npm/minimist@1.2.2", &record.uuid),
- pin("pkg:npm/other@1.0.0", "33333333-3333-4333-8333-333333333333"),
+ pin(
+ "pkg:npm/other@1.0.0",
+ "33333333-3333-4333-8333-333333333333",
+ ),
],
Some(&legacy),
);
assert_eq!(listings[0].record, record);
- assert_eq!(listings[1].record.uuid, "33333333-3333-4333-8333-333333333333");
+ assert_eq!(
+ listings[1].record.uuid,
+ "33333333-3333-4333-8333-333333333333"
+ );
assert!(listings[1].record.vulnerabilities.is_empty());
assert_eq!(listings[1].lockfiles, vec!["yarn.lock".to_string()]);
}
diff --git a/crates/socket-patch-cli/src/commands/mod.rs b/crates/socket-patch-cli/src/commands/mod.rs
--- a/crates/socket-patch-cli/src/commands/mod.rs
+++ b/crates/socket-patch-cli/src/commands/mod.rs
@@ -1,7 +1,7 @@
pub mod apply;
pub(crate) mod bun_preflight;
+pub(crate) mod composer_hints;
pub(crate) mod context;
-pub(crate) mod composer_hints;
pub(crate) mod fetch_stage;
pub mod get;
pub mod hosted_bundle;
@@ -9,11 +9,11 @@
pub(crate) mod lock_cli;
pub mod remove;
pub mod repair;
-pub(crate) mod vendored_backend;
pub mod rollback;
pub mod scan;
pub mod update;
pub mod vendor;
+pub(crate) mod vendored_backend;
pub mod vex;
pub(crate) mod vex_consumed;
pub(crate) mod vex_sources;
@@ -141,9 +141,11 @@
common: &crate::args::GlobalArgs,
root: &Path,
) -> socket_patch_core::patch::redirect::RedirectState {
- hosted_state_from_pins(&socket_patch_core::patch::redirect::upstream::HostedPin::all(
- &discover_wiring(common, root).await,
- ))
+ hosted_state_from_pins(
+ &socket_patch_core::patch::redirect::upstream::HostedPin::all(
+ &discover_wiring(common, root).await,
+ ),
+ )
}
/// [`hosted_state_from_lockfiles`] over already-discovered pins. A purl
@@ -153,10 +155,8 @@
) -> socket_patch_core::patch::redirect::RedirectState {
let mut state = socket_patch_core::patch::redirect::RedirectState::new();
for pin in pins {
- state
- .records
- .entry(pin.purl.clone())
- .or_insert_with(|| socket_patch_core::manifest::schema::PatchRecord {
+ state.records.entry(pin.purl.clone()).or_insert_with(|| {
+ socket_patch_core::manifest::schema::PatchRecord {
uuid: pin.uuid.clone(),
exported_at: String::new(),
files: Default::default(),
@@ -164,7 +164,8 @@
description: String::new(),
license: String::new(),
tier: String::new(),
- });
+ }
+ });
}
state
}
@@ -191,4 +192,3 @@
}
}
}
-
diff --git a/crates/socket-patch-cli/src/commands/scan/discovery.rs b/crates/socket-patch-cli/src/commands/scan/discovery.rs
--- a/crates/socket-patch-cli/src/commands/scan/discovery.rs
+++ b/crates/socket-patch-cli/src/commands/scan/discovery.rs
@@ -168,29 +168,32 @@
}
// `(ledger key, base purl, entry)`; the artifact fallback has no
// entries to probe, so it never reports unwired keys.
- let candidates: Vec<(String, String, Option<&socket_patch_core::vendor::VendorEntry>)> =
- match state {
- Ok(state) => state
- .entries
- .iter()
- .map(|(key, entry)| {
- (
- key.clone(),
- strip_purl_qualifiers(&entry.base_purl).to_string(),
- Some(entry),
- )
- })
- .collect(),
- // Corrupt/unreadable ledger (a MISSING file is Ok(empty) above):
- // recover the vendored set from the committed artifacts, or
- // `scan --prune` (whose ledger exemption also degrades to empty)
- // would delete still-vendored packages' manifest entries and blobs.
- Err(_) => vendored_purls_from_artifacts(common)
- .await
- .into_iter()
- .map(|base| (base.clone(), base, None))
- .collect(),
- };
+ let candidates: Vec<(
+ String,
+ String,
+ Option<&socket_patch_core::vendor::VendorEntry>,
+ )> = match state {
+ Ok(state) => state
+ .entries
+ .iter()
+ .map(|(key, entry)| {
+ (
+ key.clone(),
+ strip_purl_qualifiers(&entry.base_purl).to_string(),
+ Some(entry),
+ )
+ })
+ .collect(),
+ // Corrupt/unreadable ledger (a MISSING file is Ok(empty) above):
+ // recover the vendored set from the committed artifacts, or
+ // `scan --prune` (whose ledger exemption also degrades to empty)
+ // would delete still-vendored packages' manifest entries and blobs.
+ Err(_) => vendored_purls_from_artifacts(common)
+ .await
+ .into_iter()
+ .map(|base| (base.clone(), base, None))
+ .collect(),
+ };
// Composer by release identity: a ledger `@3.0.2.0` is the crawled
// `@3.0.2`, not a second package to supplement.
let key = |p: &str| composer_purl_identity(p).unwrap_or_else(|| normalize_purl(p).into_owned());
@@ -1045,7 +1048,9 @@
..GlobalArgs::default()
};
let state = socket_patch_core::vendor::load_state(root).await;
- vendored_ledger_supplement(&args, crawled, &state).await.packages
+ vendored_ledger_supplement(&args, crawled, &state)
+ .await
+ .packages
}
/// A ledger entry vendored as `@3.0.2.0` is the crawled composer
@@ -1080,7 +1085,9 @@
out.iter().map(|p| &p.purl).collect::<Vec<_>>()
);
- let out = vendored_ledger_supplement(&args, &[], &Ok(state)).await.packages;
+ let out = vendored_ledger_supplement(&args, &[], &Ok(state))
+ .await
+ .packages;
assert_eq!(
out.iter().map(|p| p.purl.as_str()).collect::<Vec<_>>(),
vec!["pkg:composer/psr/log@3.0.2.0"]
@@ -1183,7 +1190,10 @@
let state = npm_ledger_with_lock(tmp.path(), lock.as_deref()).await;
let out = vendored_ledger_supplement(&args, &[], &state).await;
assert_eq!(
- out.packages.iter().map(|p| p.purl.as_str()).collect::<Vec<_>>(),
+ out.packages
+ .iter()
+ .map(|p| p.purl.as_str())
+ .collect::<Vec<_>>(),
vec!["pkg:npm/left-pad@1.3.0"],
"lock={lock:?}"
);
diff --git a/crates/socket-patch-cli/src/commands/scan/hosted.rs b/crates/socket-patch-cli/src/commands/scan/hosted.rs
--- a/crates/socket-patch-cli/src/commands/scan/hosted.rs
+++ b/crates/socket-patch-cli/src/commands/scan/hosted.rs
@@ -982,7 +982,8 @@
socket_patch_core::utils::fs::read_regular_to_string_sync(path).ok()
})
};
- let rewrite_options = || RewriteOptions {
+ let rewrite_options = || {
+ RewriteOptions {
dry_run: common.dry_run,
targets_pipenv_lock,
pipenv_major,
@@ -994,6 +995,7 @@
npm_allow_remote_config: !common.no_npm_allow_remote_config,
npm_outer: &npm_outer,
blocking: true,
+ }
};
// The rollout gate plans again without its deferred rows: keep what
// the second pass needs.
@@ -2431,13 +2433,19 @@
/// artifacts, then verify with `vex`. After a vendored→hosted takeover
/// (`vendored_removed`) the commit also has to carry the deleted vendored
/// ledger entries and artifacts.
-fn format_next_steps(files: &[String], edits: &[socket_patch_core::patch::redirect::FileEdit], vendored_removed: bool) -> Vec<String> {
+fn format_next_steps(
+ files: &[String],
+ edits: &[socket_patch_core::patch::redirect::FileEdit],
+ vendored_removed: bool,
+) -> Vec<String> {
if files.is_empty() && !vendored_removed {
return Vec::new();
}
let mut commit: Vec<String> = Vec::new();
if vendored_removed {
- commit.push(".socket/vendor/ (the removed vendored ledger entries and artifacts)".to_string());
+ commit.push(
+ ".socket/vendor/ (the removed vendored ledger entries and artifacts)".to_string(),
+ );
}
commit.extend(files.iter().cloned());
let npm = files
@@ -4624,19 +4632,43 @@
use super::npm_allow_remote_one_line;
let hosts = ["patch.socket.dev"];
let cases = [
- (npm_allow_remote_configured_detail(&hosts, true, false), "Note: set"),
- (npm_allow_remote_configured_detail(&hosts, false, false), "Note: set"),
- (npm_allow_remote_configured_detail(&hosts, true, true), "Note: would set"),
- (npm_allow_remote_already_detail(&hosts), "Note: .npmrc already"),
- (npm_allow_remote_user_set_detail(&hosts, "none"), "Warning: npm >=12"),
- (npm_allow_remote_env_set_detail(&hosts, "npm_config_allow_remote", "none"), "Warning: npm >=12"),
+ (
+ npm_allow_remote_configured_detail(&hosts, true, false),
+ "Note: set",
+ ),
+ (
+ npm_allow_remote_configured_detail(&hosts, false, false),
+ "Note: set",
+ ),
+ (
+ npm_allow_remote_configured_detail(&hosts, true, true),
+ "Note: would set",
+ ),
+ (
+ npm_allow_remote_already_detail(&hosts),
+ "Note: .npmrc already",
+ ),
+ (
+ npm_allow_remote_user_set_detail(&hosts, "none"),
+ "Warning: npm >=12",
+ ),
+ (
+ npm_allow_remote_env_set_detail(&hosts, "npm_config_allow_remote", "none"),
+ "Warning: npm >=12",
+ ),
(npm_allow_remote_manual_detail(&hosts), "Warning: npm >=12"),
- (npm_allow_remote_unreadable_detail(&hosts, "is a symlink"), "Warning: npm >=12"),
+ (
+ npm_allow_remote_unreadable_detail(&hosts, "is a symlink"),
+ "Warning: npm >=12",
+ ),
];
for (detail, start) in cases {
let line = npm_allow_remote_one_line(&detail);
assert!(line.starts_with(start), "{line}");
- assert!(!line.contains('\n') && line.ends_with("(details: --verbose)."), "{line}");
+ assert!(
+ !line.contains('\n') && line.ends_with("(details: --verbose)."),
+ "{line}"
+ );
}
}
}
diff --git a/crates/socket-patch-cli/src/commands/scan/policy.rs b/crates/socket-patch-cli/src/commands/scan/policy.rs
--- a/crates/socket-patch-cli/src/commands/scan/policy.rs
+++ b/crates/socket-patch-cli/src/commands/scan/policy.rs
@@ -11,9 +11,9 @@
use socket_patch_core::api::types::PatchSearchResult;
use socket_patch_core::manifest::schema::PatchManifest;
use socket_patch_core::policy::{
- canon, find_repo_root_with_warnings, policy_block, FilteredEntry, RetainedEntry, patch_severity_order, repo_relative_checked, sanitize, severity_name,
- DiskPolicyFs, FilterReason, Offers, PolicyError, PolicySource, PolicyWarning, Root, SelectionPolicy,
- PATCHES_DISABLED,
+ canon, find_repo_root_with_warnings, patch_severity_order, policy_block, repo_relative_checked,
+ sanitize, severity_name, DiskPolicyFs, FilterReason, FilteredEntry, Offers, PolicyError,
+ PolicySource, PolicyWarning, RetainedEntry, Root, SelectionPolicy, PATCHES_DISABLED,
};
use socket_patch_core::utils::purl::normalize_purl;
@@ -42,12 +42,18 @@
/// Load the policy for `args` (4.5): `--global` scans have no repo and read
/// no file; everything else reads the repo root's socket.yml.
pub(crate) fn load_invocation_policy(args: &ScanArgs) -> Result<InvocationPolicy, PolicyLoadError> {
- let overrides = args.socket_yml.overrides().map_err(PolicyLoadError::Usage)?;
+ let overrides = args
+ .socket_yml
+ .overrides()
+ .map_err(PolicyLoadError::Usage)?;
let cwd = std::fs::canonicalize(&args.common.cwd).unwrap_or_else(|_| args.common.cwd.clone());
if args.common.is_global() {
- let policy = SelectionPolicy::load(&socket_patch_core::policy::MemoryPolicyFs::default(), &overrides)
- .map_err(PolicyLoadError::Policy)?
- .0;
+ let policy = SelectionPolicy::load(
+ &socket_patch_core::policy::MemoryPolicyFs::default(),
+ &overrides,
+ )
+ .map_err(PolicyLoadError::Policy)?
+ .0;
return Ok(InvocationPolicy {
policy,
repo_root: cwd,
@@ -56,8 +62,8 @@
});
}
let (repo_root, mut warnings) = find_repo_root_with_warnings(&cwd);
- let (policy, load_warnings) =
- SelectionPolicy::load(&DiskPolicyFs::new(&repo_root), &overrides).map_err(PolicyLoadError::Policy)?;
+ let (policy, load_warnings) = SelectionPolicy::load(&DiskPolicyFs::new(&repo_root), &overrides)
+ .map_err(PolicyLoadError::Policy)?;
warnings.extend(load_warnings);
Ok(InvocationPolicy {
policy,
@@ -138,7 +144,12 @@
impl ScanPolicy {
/// The policy for the project rooted at `root_dir`.
- pub(crate) fn for_root(invocation: &InvocationPolicy, root_dir: &Path, explicit: bool, global: bool) -> Self {
+ pub(crate) fn for_root(
+ invocation: &InvocationPolicy,
+ root_dir: &Path,
+ explicit: bool,
+ global: bool,
+ ) -> Self {
let root_dir = std::fs::canonicalize(root_dir).unwrap_or_else(|_| root_dir.to_path_buf());
let project = repo_relative_checked(&invocation.repo_root, &root_dir).unwrap_or_default();
let root_verdict = if global {
@@ -171,7 +182,9 @@
severity: None,
});
}
- let announce_warnings = !invocation.warned.swap(true, std::sync::atomic::Ordering::Relaxed);
+ let announce_warnings = !invocation
+ .warned
+ .swap(true, std::sync::atomic::Ordering::Relaxed);
Self {
policy: invocation.policy.clone(),
warnings,
@@ -224,7 +237,10 @@
/// exclude stays in the query (so `upgradeAvailable` can be reported)
/// but joins the retained set, which never reaches a writer.
pub(crate) fn admit_crawled(&self, purl: &str) -> bool {
- let verdict = self.root_verdict.clone().and_then(|()| self.policy.admits_purl(purl));
+ let verdict = self
+ .root_verdict
+ .clone()
+ .and_then(|()| self.policy.admits_purl(purl));
let reason = match verdict {
Ok(()) => return true,
Err(reason) => reason,
@@ -334,7 +350,8 @@
// (not when a lower-ranked admitted patch simply wins).
let top_withheld = self.policy.admits_severity(patch_severity_order(&group[0]));
if let Err(reason) = top_withheld {
- let upgrade_withheld = chosen.is_some() && chosen == recorded_at && recorded_at != Some(0);
+ let upgrade_withheld =
+ chosen.is_some() && chosen == recorded_at && recorded_at != Some(0);
if chosen.is_none() || upgrade_withheld {
report.filtered.push(FilteredEntry {
purl: Some(canon(&purl)),
@@ -522,17 +539,20 @@
let verdict = if !policy.enabled() {
Err(FilterReason::Disabled)
} else {
- root_verdict.clone().and_then(|()| policy.admits_purl(purl)).and_then(|()| {
- // The floor only hides a package when none of its patches pass.
- match group
- .iter()
- .map(|p| policy.admits_severity(patch_severity_order(p)))
- .find(Result::is_ok)
- {
- Some(ok) => ok,
- None => policy.admits_severity(patch_severity_order(group[0])),
- }
- })
+ root_verdict
+ .clone()
+ .and_then(|()| policy.admits_purl(purl))
+ .and_then(|()| {
+ // The floor only hides a package when none of its patches pass.
+ match group
+ .iter()
+ .map(|p| policy.admits_severity(patch_severity_order(p)))
+ .find(Result::is_ok)
+ {
+ Some(ok) => ok,
+ None => policy.admits_severity(patch_severity_order(group[0])),
+ }
+ })
};
if let Err(reason) = verdict {
out.push((
diff --git a/crates/socket-patch-cli/src/commands/scan/rollout.rs b/crates/socket-patch-cli/src/commands/scan/rollout.rs
--- a/crates/socket-patch-cli/src/commands/scan/rollout.rs
+++ b/crates/socket-patch-cli/src/commands/scan/rollout.rs
@@ -4,8 +4,10 @@
use std::collections::{BTreeMap, BTreeSet, HashSet};
-use socket_patch_core::rollout::{canonical_base_purl, severity_label, MaxNew, MaxNewSource, Recorded, RolloutPlan};
pub(crate) use socket_patch_core::rollout::stage::*;
+use socket_patch_core::rollout::{
+ canonical_base_purl, severity_label, MaxNew, MaxNewSource, Recorded, RolloutPlan,
+};
use super::discovery::UpdateInfo;
@@ -208,11 +210,11 @@
mod tests {
use super::*;
use socket_patch_core::api::types::PatchSearchResult;
+ use socket_patch_core::api::types::VulnerabilityResponse;
use socket_patch_core::manifest::schema::PatchManifest;
- use std::path::Path;
- use socket_patch_core::api::types::VulnerabilityResponse;
use socket_patch_core::manifest::schema::PatchRecord;
use std::collections::HashMap;
+ use std::path::Path;
fn offer(purl: &str, uuid: &str, published: &str, severities: &[&str]) -> PatchSearchResult {
PatchSearchResult {
@@ -357,13 +359,21 @@
let stored = manifest(&[("pkg:composer/psr/log@3.0.2.0", "old")]);
let recorded = RecordedIndex::new(Some(&stored), &[]);
let offers = offers_from_results(
- &[offer("pkg:composer/psr/log@v3.0.2", "new", "2026-02-01T00:00:00Z", &["high"])],
+ &[offer(
+ "pkg:composer/psr/log@v3.0.2",
+ "new",
+ "2026-02-01T00:00:00Z",
+ &["high"],
+ )],
false,
);
let rows = classify(&offers, &recorded, "");
let plan = socket_patch_core::rollout::plan_rollout(
rows.into_iter().map(|row| row.candidate).collect(),
- &MaxNew { value: Some(0), source: MaxNewSource::Flag },
+ &MaxNew {
+ value: Some(0),
+ source: MaxNewSource::Flag,
+ },
false,
&BTreeSet::new(),
);
diff --git a/crates/socket-patch-cli/src/commands/scan/rollout_args.rs b/crates/socket-patch-cli/src/commands/scan/rollout_args.rs
--- a/crates/socket-patch-cli/src/commands/scan/rollout_args.rs
+++ b/crates/socket-patch-cli/src/commands/scan/rollout_args.rs
@@ -1,7 +1,6 @@
//! `scan --max-new-patches` (see the rollout guide,
//! `docs/configuration.md#gradual-rollout`).
-
use clap::Args;
pub(crate) use socket_patch_core::rollout::stage::RolloutCarry;
use socket_patch_core::rollout::{resolve_max_new, MaxNew};
@@ -77,7 +76,6 @@
}
}
-
#[cfg(test)]
mod tests {
use super::*;
diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs
--- a/crates/socket-patch-cli/src/commands/vendor.rs
+++ b/crates/socket-patch-cli/src/commands/vendor.rs
@@ -283,10 +283,7 @@
/// entry (fail-safe): ecosystems other than npm, cargo and pypi (whose
/// probe covers the requirements flavor only) have no in-use probe yet,
/// and a missing/unreadable lockfile proves nothing.
-pub(crate) async fn dispatch_in_use_one(
- entry: &VendorEntry,
- project_root: &Path,
-) -> Option<bool> {
+pub(crate) async fn dispatch_in_use_one(entry: &VendorEntry, project_root: &Path) -> Option<bool> {
match entry.ecosystem.as_str() {
"npm" => vendor::npm_flavor::vendored_entry_in_use(entry, project_root).await,
// Cargo probes the lock entry's shape: detached + `[patch]` pointing
diff --git a/crates/socket-patch-cli/tests/apply/apply_network.rs b/crates/socket-patch-cli/tests/apply/apply_network.rs
--- a/crates/socket-patch-cli/tests/apply/apply_network.rs
+++ b/crates/socket-patch-cli/tests/apply/apply_network.rs
@@ -940,7 +940,10 @@
"a legacy package archive must not cover the patch; stdout={stdout}\nstderr={stderr}"
);
let content = std::fs::read(tmp.path().join("node_modules/pkgcache/index.js")).unwrap();
- assert_eq!(content, before, "the file must not be patched from the legacy archive");
+ assert_eq!(
+ content, before,
+ "the file must not be patched from the legacy archive"
+ );
let requests = mock.received_requests().await.unwrap_or_default();
let blob_path = format!("/v0/orgs/{ORG_SLUG}/patches/blob/{after_hash}");
@@ -1043,10 +1046,7 @@
v["summary"]["applied"], 1,
"the drifted nested copy must be warn-overwritten.\nstdout={v:#}"
);
- assert_eq!(
- v["summary"]["failed"], 0,
- "no copy may fail.\nstdout={v:#}"
- );
+ assert_eq!(v["summary"]["failed"], 0, "no copy may fail.\nstdout={v:#}");
// The nested copy's blob was fetched on demand…
let requests = mock.received_requests().await.unwrap();
diff --git a/crates/socket-patch-cli/tests/apply/in_process_gem_config_warning.rs b/crates/socket-patch-cli/tests/apply/in_process_gem_config_warning.rs
--- a/crates/socket-patch-cli/tests/apply/in_process_gem_config_warning.rs
+++ b/crates/socket-patch-cli/tests/apply/in_process_gem_config_warning.rs
@@ -201,7 +201,9 @@
"non-silent stderr must carry the {CODE} warning; got:\n{stderr}"
);
assert_eq!(
- stderr.matches("Warning: bundler app config BUNDLE_PATH").count(),
+ stderr
+ .matches("Warning: bundler app config BUNDLE_PATH")
+ .count(),
1,
"exactly ONE warning line (not one per discovery call); got:\n{stderr}"
);
diff --git a/crates/socket-patch-cli/tests/cli/covgap_output.rs b/crates/socket-patch-cli/tests/cli/covgap_output.rs
--- a/crates/socket-patch-cli/tests/cli/covgap_output.rs
+++ b/crates/socket-patch-cli/tests/cli/covgap_output.rs
@@ -168,9 +168,8 @@
.expect("spawn socket-patch in PTY");
drop(pair.slave);
- let reader_handle = crate::pty_io::PtyOutput::spawn(
- pair.master.try_clone_reader().expect("clone reader"),
- );
+ let reader_handle =
+ crate::pty_io::PtyOutput::spawn(pair.master.try_clone_reader().expect("clone reader"));
// Watchdog: detached kill after `timeout`; a no-op if the child exits
// naturally first.
@@ -261,7 +260,10 @@
"\n",
Duration::from_secs(15),
);
- assert_eq!(code, 0, "remove with bare Enter must succeed; got: {output}");
+ assert_eq!(
+ code, 0,
+ "remove with bare Enter must succeed; got: {output}"
+ );
// The interactive confirm MUST have run — otherwise this test passes
// vacuously against a regression that drops the TTY gate and
// auto-proceeds. Match the distinctive prompt verbatim (the loose
diff --git a/crates/socket-patch-cli/tests/cli/interactive_prompts_e2e.rs b/crates/socket-patch-cli/tests/cli/interactive_prompts_e2e.rs
--- a/crates/socket-patch-cli/tests/cli/interactive_prompts_e2e.rs
+++ b/crates/socket-patch-cli/tests/cli/interactive_prompts_e2e.rs
@@ -112,9 +112,8 @@
// closed. The previous design used a chunked read+mpsc loop
// because it interleaved with a try_wait poll; the simplified
// design serializes wait → drop master → read_to_end joins.
- let reader_handle = crate::pty_io::PtyOutput::spawn(
- pair.master.try_clone_reader().expect("clone reader"),
- );
+ let reader_handle =
+ crate::pty_io::PtyOutput::spawn(pair.master.try_clone_reader().expect("clone reader"));
// Watchdog: detach a thread that kills the child after `timeout`.
// The cloned ChildKiller is independent of the main `child`
diff --git a/crates/socket-patch-cli/tests/cli_config_fallback.rs b/crates/socket-patch-cli/tests/cli_config_fallback.rs
--- a/crates/socket-patch-cli/tests/cli_config_fallback.rs
+++ b/crates/socket-patch-cli/tests/cli_config_fallback.rs
@@ -59,8 +59,7 @@
let mut cmd = Command::new(BINARY);
// Human mode: core's proxy advisory (the oracle below) is muted under
// `--json`/`--silent`.
- cmd.args(["scan", "-e", "npm", "--cwd"])
- .arg(project);
+ cmd.args(["scan", "-e", "npm", "--cwd"]).arg(project);
for (key, _) in std::env::vars_os() {
let name = key.to_string_lossy();
if name.starts_with("SOCKET_") {
@@ -298,7 +297,9 @@
json_cmd.arg("--json");
let json_out = run(json_cmd);
assert!(
- json_out.stderr.contains("could not parse socket-cli config"),
+ json_out
+ .stderr
+ .contains("could not parse socket-cli config"),
"the parse warning must reach stderr under --json too; got:\n{}",
json_out.stderr
);
diff --git a/crates/socket-patch-cli/tests/cli_get_silent.rs b/crates/socket-patch-cli/tests/cli_get_silent.rs
--- a/crates/socket-patch-cli/tests/cli_get_silent.rs
+++ b/crates/socket-patch-cli/tests/cli_get_silent.rs
@@ -25,10 +25,7 @@
for var in GLOBAL_ARG_ENV_VARS {
cmd.env_remove(var);
}
- for var in [
- "SOCKET_SAVE_ONLY",
- "SOCKET_ALL_RELEASES",
- ] {
+ for var in ["SOCKET_SAVE_ONLY", "SOCKET_ALL_RELEASES"] {
cmd.env_remove(var);
}
cmd.env("SOCKET_TELEMETRY_DISABLED", "1");
diff --git a/crates/socket-patch-cli/tests/cli_parse_list.rs b/crates/socket-patch-cli/tests/cli_parse_list.rs
--- a/crates/socket-patch-cli/tests/cli_parse_list.rs
+++ b/crates/socket-patch-cli/tests/cli_parse_list.rs
@@ -370,7 +370,11 @@
let out = run_list_binary(tmp.path(), &["--json"]);
let v: serde_json::Value = serde_json::from_str(String::from_utf8_lossy(&out.stdout).trim())
.expect("stdout must be valid JSON envelope");
- assert_eq!(out.status.code(), Some(0), "missing manifest is an empty list");
+ assert_eq!(
+ out.status.code(),
+ Some(0),
+ "missing manifest is an empty list"
+ );
assert_eq!(v["status"], "success", "envelope: {v}");
assert_eq!(v["summary"]["discovered"], 0, "envelope: {v}");
}
@@ -1313,7 +1317,10 @@
assert_eq!(v["status"], "success", "envelope={v}");
let warnings = v["warnings"].as_array().expect("warnings[] present");
assert_eq!(warnings.len(), 1, "envelope={v}");
- assert_eq!(warnings[0]["code"], "redirect_ledger_corrupt", "envelope={v}");
+ assert_eq!(
+ warnings[0]["code"], "redirect_ledger_corrupt",
+ "envelope={v}"
+ );
assert!(
out.stderr.is_empty(),
"--json must keep stderr clean: {}",
diff --git a/crates/socket-patch-cli/tests/cli_parse_rollback.rs b/crates/socket-patch-cli/tests/cli_parse_rollback.rs
--- a/crates/socket-patch-cli/tests/cli_parse_rollback.rs
+++ b/crates/socket-patch-cli/tests/cli_parse_rollback.rs
@@ -366,7 +366,11 @@
/// relied on the rejection get a test-visible flip instead of a silent one.
#[test]
fn multiple_targets_parse_in_order() {
- let args = parse_rollback(&["pkg:npm/foo@1", "packages/api/**", "b0630680-4da6-45f9-bba8-b888e0ffd58c"]);
+ let args = parse_rollback(&[
+ "pkg:npm/foo@1",
+ "packages/api/**",
+ "b0630680-4da6-45f9-bba8-b888e0ffd58c",
+ ]);
assert_eq!(
args.targets,
vec![
diff --git a/crates/socket-patch-cli/tests/cli_parse_scan.rs b/crates/socket-patch-cli/tests/cli_parse_scan.rs
--- a/crates/socket-patch-cli/tests/cli_parse_scan.rs
+++ b/crates/socket-patch-cli/tests/cli_parse_scan.rs
@@ -898,7 +898,11 @@
("NONE", None),
] {
let args = parse_scan(&["--max-new-patches", raw]);
- assert_eq!(args.rollout.max_new_patches, Some(MaxNewPatches(want)), "{raw}");
+ assert_eq!(
+ args.rollout.max_new_patches,
+ Some(MaxNewPatches(want)),
+ "{raw}"
+ );
}
}
@@ -989,20 +993,33 @@
assert_eq!(parse_scan(&[]).socket_yml.min_severity, None);
assert_eq!(overrides(&[], &[]).unwrap().min_severity, None);
assert_eq!(
- overrides(&["--min-severity", "High"], &[]).unwrap().min_severity,
+ overrides(&["--min-severity", "High"], &[])
+ .unwrap()
+ .min_severity,
Some((Some(1), OverrideSource::Flag))
);
assert_eq!(
- overrides(&["--min-severity", "none"], &[("SOCKET_MIN_SEVERITY", "critical")]).unwrap().min_severity,
+ overrides(
+ &["--min-severity", "none"],
+ &[("SOCKET_MIN_SEVERITY", "critical")]
+ )
+ .unwrap()
+ .min_severity,
Some((None, OverrideSource::Flag))
);
assert_eq!(
- overrides(&[], &[("SOCKET_MIN_SEVERITY", "moderate")]).unwrap().min_severity,
+ overrides(&[], &[("SOCKET_MIN_SEVERITY", "moderate")])
+ .unwrap()
+ .min_severity,
Some((Some(2), OverrideSource::Env))
);
- assert_eq!(overrides(&[], &[("SOCKET_MIN_SEVERITY", "")]).unwrap().min_severity, None);
+ assert_eq!(
+ overrides(&[], &[("SOCKET_MIN_SEVERITY", "")])
+ .unwrap()
+ .min_severity,
+ None
+ );
assert!(overrides(&[], &[("SOCKET_MIN_SEVERITY", "severe")]).is_err());
assert!(try_parse_scan(&["--min-severity", "severe"]).is_err());
assert!(overrides(&["--no-socket-yml"], &[]).unwrap().bypass);
}
-
diff --git a/crates/socket-patch-cli/tests/coverage_fix_apply_silent_mute_exit.rs b/crates/socket-patch-cli/tests/coverage_fix_apply_silent_mute_exit.rs
--- a/crates/socket-patch-cli/tests/coverage_fix_apply_silent_mute_exit.rs
+++ b/crates/socket-patch-cli/tests/coverage_fix_apply_silent_mute_exit.rs
@@ -149,7 +149,9 @@
);
let chatter = stderr_chatter(&stderr);
assert!(
- chatter.iter().any(|l| l.contains("could not be downloaded")),
+ chatter
+ .iter()
+ .any(|l| l.contains("could not be downloaded")),
"--silent must keep the download-failure error (errors only, \
never nothing); stderr was: {stderr:?}"
);
diff --git a/crates/socket-patch-cli/tests/covgap_commands_scan_hosted.rs b/crates/socket-patch-cli/tests/covgap_commands_scan_hosted.rs
--- a/crates/socket-patch-cli/tests/covgap_commands_scan_hosted.rs
+++ b/crates/socket-patch-cli/tests/covgap_commands_scan_hosted.rs
@@ -566,8 +566,7 @@
"the human skipped line must name purl + reason; stderr=\n{stderr}"
);
assert!(
- stderr.contains("Warning: ")
- && stderr.contains("could not be reverted"),
+ stderr.contains("Warning: ") && stderr.contains("could not be reverted"),
"the takeover pre-warning must reach human stderr; stderr=\n{stderr}"
);
}
@@ -814,7 +813,10 @@
let lock_before = std::fs::read(root.join("package-lock.json")).unwrap();
let assert_ignored = |code: i32, doc: &Value, label: &str| {
- assert_eq!(code, 0, "{label}: a pre-v5 ledger is never an error: {doc:#}");
+ assert_eq!(
+ code, 0,
+ "{label}: a pre-v5 ledger is never an error: {doc:#}"
+ );
assert_eq!(doc["status"], "success", "{label}: {doc:#}");
assert!(
!doc.to_string().contains("redirect-state.json"),
@@ -1017,7 +1019,10 @@
for extra in [&[][..], &["--silent"][..]] {
let (code, stdout, stderr) = scan_hosted(root, &server.uri(), extra, &[]);
- assert_eq!(code, 0, "{extra:?}: an empty discovery exits 0; stderr=\n{stderr}");
+ assert_eq!(
+ code, 0,
+ "{extra:?}: an empty discovery exits 0; stderr=\n{stderr}"
+ );
if extra.is_empty() {
assert!(
stdout.contains("No patches available for installed packages."),
@@ -1404,16 +1409,22 @@
],
... diff truncated: showing 800 of 6626 linesYou can send follow-ups to the cloud agent here.
When the Bundler tier that wins sets path.system, Bundler also ignores an env BUNDLE_PATH below it or beside it. If that value named the leftover vendor/bundle, the crawler still probed it as the default root and hid the system gem homes again, so apply and vex kept targeting the unused copy. The env root is now skipped in that case too. Assisted-by: Claude Code:claude-opus-5-5
Collaborator
Author
|
BugBot review Generated by Claude Code |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit ccebcd0. Configure here.
Mikola Lysenko (mikolalysenko)
added
the
Ready for review
Collaborator
Author
|
Burn-down agent: labeled Ready for review at
Slack announcement is still pending because this run's Slack connector has no send tool, so the Generated by Claude Code |
Tanmay Singla (Tanmay182003)
approved these changes
Oct 6, 2026
This was referenced Oct 6, 2026
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
LLM Description written by Claude Code:claude-opus-5-5
Fixes #915
Summary
A project that moved from
vendor/bundleback to system gems (bundle config set path.system trueorBUNDLE_PATH__SYSTEM=true) usually still has the old, gitignoredvendor/bundle. Bundler ignores it and loads the system copy. The gem crawler still counted it, though, and because it held gems it switched off thegem envhomes. As a result:applypatched only the unused copy and reported success, andvexattestednot_affectedwhile the app ran the unpatched gem.Now the crawler skips the default
vendor/bundleroot when the Bundler tier that decides the install path sets a truthypath.system. The system gem homes are crawled, patched and verified instead.Root cause
RubyCrawler::discover_bundle_stores_implpushed<cwd>/vendor/bundleunconditionally. A store there setsdefault_root_has_stores, which suppresses thegem envfallback ingem_paths_and_discovery. The crawler already parsedpath.system, but only to drop the recordedBUNDLE_PATH. It never applied the setting to the implicit default root.Changes
bundler_path_systemreproduces Bundler'sSettings#pathtier selection: the first of the local app config, the environment, then the global config that setspath,path.systemordisable_shared_gems. If that tier'spath.systemis truthy, the default root is not probed. As with the explicit roots, this only applies when--cwdholds a Bundler manifest.bundler_truthyfollows Bundler'sto_boolcoercion: anything exceptfalse/f/no/n/0/empty is true. I checked this against Bundler 4.0.18, where1andyesare true.parse_bundle_config_pathuses it too; it used to accept only the exact string"true".path.systemwins, an envBUNDLE_PATHthat it shadows (or that sits in the same tier) is skipped too, so a value namingvendor/bundlecan't bring the leftover store back. This was Bugbot's finding on de26452, fixed in ccebcd0.BUNDLE_PATH__SYSTEMis threaded into discovery for every ambient caller:get_gem_paths,crawl_all_with_discovery, anddiscover_bundle_stores, which feedsvexand the hosted stale-install probe.vendor/bundlebehavior is unchanged.I checked the tier semantics against real Bundler 4.0.18 (
Bundler.settings.path.use_system_gems?/Bundler.bundle_path):path.systembeats an envBUNDLE_PATH;BUNDLE_PATHbeats a globalpath.system;path.system: "1"counts as true.CI note
main(9c43dfc) is red incoverage/test (macos|windows)on theutils::digestarchitecture guard. This PR carries #878's fix as a cherry-pick (de26452), which becomes a no-op once #878 merges.Test evidence
Red without the fix (
git stashofruby_crawler.rs), green with it:cargo test -p socket-patch-core --lib ruby_crawler: 4 failed before (local_path_system_drops_leftover_vendor_bundle,env_path_system_drops_leftover_vendor_bundle,global_path_system_drops_leftover_vendor_bundle,parse_bundle_config_path_contract), 81/81 pass after. Theshadowed_path_system_keeps_vendor_bundlecontrol passes both ways.cargo test -p socket-patch-core --test crawler_ruby_e2e path_system: before the fix it failed withleft: [".../app/vendor/bundle/ruby/3.3.0/gems"] right: [".../system-gem-home/gems"]; it passes after. This test uses the ambient env plus a fakegem envbinary and covers the local-config, env and control cases, includingfind_each_by_purllocating the loaded copy.Also run locally:
cargo clippy --workspace --all-features -- -D warnings: clean.crawler_ruby_e2e: 28/28.in_process_gem_apply: 11/11.in_process_gem_multi_platform: 7/7.e2e_redirect_gem_stale_install: 32/32.cargo test -p socket-patch-core --lib: 5251 passed, 4 failed. The 4 failures are permission-denial tests (copy_treesymlinked root,vlt_healunremovable lock,pypi_poetry/pypi_requirementswrite failure). They can't fail as intended when run as root in this sandbox, and they're in modules this PR doesn't touch. CI runs them as non-root.cargo test --workspace --all-featuresran out of sandbox disk, so CI is the record for the rest.npm/,pypi/,gem/only dispatch to the binary).Per-issue checklist
path.system: truewhen a leftovervendor/bundleexists, so agentapplypatches the unused copy andvexattestsnot_affectedwhile Bundler loads the unpatched system gem #915 localpath.system: true+ leftovervendor/bundle:local_path_system_drops_leftover_vendor_bundleand the e2elocalcasepath.system: truewhen a leftovervendor/bundleexists, so agentapplypatches the unused copy andvexattestsnot_affectedwhile Bundler loads the unpatched system gem #915 envBUNDLE_PATH__SYSTEM=truevariant:env_path_system_drops_leftover_vendor_bundleand the e2eenvcasepath.system:global_path_system_drops_leftover_vendor_bundleshadowed_path_system_keeps_vendor_bundleWhy this cluster
This is a single-issue p1 picked ahead of older p1s because it produces a false VEX attestation and has a narrow, well-understood fix.
🤖 Generated with Claude Code
Note
Medium Risk
Changes Ruby gem root discovery and patching targets for Bundler path.system projects; incorrect tier logic could still miss or mis-target gems, but behavior is heavily tested and scoped to install-root selection.
Overview
Fixes #915: when Bundler’s winning install-path tier sets a truthy
path.system(local.bundle/config, envBUNDLE_PATH__SYSTEM, or global config), gem discovery no longer probes the defaultvendor/bundleroot or a shadowed envBUNDLE_PATH. Leftover gitignored stores there can’t suppress thegem envfallback, so apply, scan, and vex target the system copy Bundler actually loads.Implementation mirrors Bundler’s tier order via
bundler_path_systemandbundler_truthy(Bundlerto_bool, not only"true").parse_bundle_config_pathandconfig_path_systemuse the same coercion. Ambient discovery threadsBUNDLE_PATH__SYSTEMthroughget_gem_paths,crawl_all, anddiscover_bundle_stores. CLI_CONTRACT.md documents the rule; unit and e2e tests cover local/env/global variants and shadowing controls.Separately, Gradle cache / JVM jar / Maven sidecar code routes SHA-1/SHA-256 through
crate::utils::digestinstead of ad-hocsha1/sha2calls (cherry-pick alignment with #878).Reviewed by Cursor Bugbot for commit ccebcd0. Configure here.
Generated by Claude Code