From 7333034544c2e8a831aeee69d994880cc2ad5b07 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 17:27:22 +0000 Subject: [PATCH 1/4] Start fix for #951 Assisted-by: Claude Code:claude-opus-5-5 From ff4f2c4333b5b7f815773892ffae59f4e1b44780 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 17:50:52 +0000 Subject: [PATCH 2/4] Drop trailing # comments from .bundle/config Bundler cuts a .bundle/config value at its first `#` (RubyGems and Bundler 2.5.6+), but socket-patch kept the comment as part of the value. A hand-commented `BUNDLE_PATH: .gems # note` therefore made agent apply patch the system gem copy, and vex attest not_affected, while Bundler loaded the unpatched .gems copy. A commented BUNDLE_CACHE_PATH also hid stale archives from the hosted stale-install guard, and commented path.system / gemfile values were misread too. The shared config value reader now strips the comment the way Bundler's config loader does. Older Bundler keeps the comment in the value, so for directory settings (path, cache_path) the legacy reading is used when only its directory exists on disk. Fixes #951 Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/e2e_redirect_gem_stale_install.rs | 3 + .../tests/in_process_alternate_installers.rs | 95 ++++++- .../src/crawlers/ruby_crawler.rs | 263 ++++++++++++++++-- 3 files changed, 339 insertions(+), 22 deletions(-) diff --git a/crates/socket-patch-cli/tests/e2e_redirect_gem_stale_install.rs b/crates/socket-patch-cli/tests/e2e_redirect_gem_stale_install.rs index 24817325f..f94a760cb 100644 --- a/crates/socket-patch-cli/tests/e2e_redirect_gem_stale_install.rs +++ b/crates/socket-patch-cli/tests/e2e_redirect_gem_stale_install.rs @@ -630,8 +630,11 @@ async fn gem_hosted_stale_archive_at_configured_cache_path_warns_and_is_not_atte let server = MockServer::start().await; mount_api(&server, None).await; let moved = "---\nBUNDLE_CACHE_PATH: \"vendor/gems\"\n"; + // #951: Bundler drops a trailing `# comment` from the value. + let commented = "---\nBUNDLE_CACHE_PATH: vendor/gems # committed gem cache\n"; for (label, config, env, cache_dir) in [ ("app-config", Some(moved), &[][..], "gems"), + ("app-config-commented", Some(commented), &[][..], "gems"), ( "env", None, diff --git a/crates/socket-patch-cli/tests/in_process_alternate_installers.rs b/crates/socket-patch-cli/tests/in_process_alternate_installers.rs index 6d5553413..0b7d2ebe5 100644 --- a/crates/socket-patch-cli/tests/in_process_alternate_installers.rs +++ b/crates/socket-patch-cli/tests/in_process_alternate_installers.rs @@ -5,7 +5,7 @@ //! supports venv, pyenv, conda, system installs. This file exercises //! the layout variants the crawlers must handle in production. -use std::path::Path; +use std::path::{Path, PathBuf}; use std::process::Command; use serial_test::serial; @@ -872,6 +872,99 @@ gem 'colorize', '1.1.0' assert_patched(&lib_file, &patched, &before_hash, &after_hash); } +/// #951: a hand-commented `.bundle/config` path +/// (`BUNDLE_PATH: .gems # project-local gems`). Bundler 2.5.6+ drops the +/// comment and installs into `.gems`; older Bundler keeps it in the +/// directory name. Either way apply must patch the copy Bundler loads +/// (asked from Bundler itself), not fall back to the system gem home. +#[tokio::test] +#[serial] +async fn bundler_commented_config_path_apply_patches_loaded_gem() { + if !has("bundle") || !has("gem") { + println!("SKIP: bundle/gem not on PATH"); + return; + } + + let tmp = tempfile::tempdir().unwrap(); + std::fs::write( + tmp.path().join("Gemfile"), + "source 'https://rubygems.org'\ngem 'colorize', '1.1.0'\n", + ) + .unwrap(); + std::fs::create_dir_all(tmp.path().join(".bundle")).unwrap(); + std::fs::write( + tmp.path().join(".bundle/config"), + "---\nBUNDLE_PATH: .gems # project-local gems\n", + ) + .unwrap(); + let status = pm_command("bundle", &["BUNDLE_"]) + .args(["install", "--quiet"]) + .current_dir(tmp.path()) + .output() + .expect("bundle install"); + if !status.status.success() { + println!( + "SKIP: bundle install failed: {}", + String::from_utf8_lossy(&status.stderr) + ); + return; + } + let loaded = pm_command("bundle", &["BUNDLE_"]) + .args([ + "exec", + "ruby", + "-e", + "print Gem.loaded_specs.fetch('colorize').full_gem_path", + ]) + .current_dir(tmp.path()) + .output() + .expect("bundle exec"); + assert!( + loaded.status.success(), + "bundle exec failed: {}", + String::from_utf8_lossy(&loaded.stderr) + ); + let lib_file = PathBuf::from(String::from_utf8(loaded.stdout).unwrap()).join("lib/colorize.rb"); + assert!( + lib_file.starts_with(tmp.path().canonicalize().unwrap()) + || lib_file.starts_with(tmp.path()), + "bundler must load the project copy: {lib_file:?}" + ); + + let original = std::fs::read(&lib_file).expect("read"); + let before_hash = git_sha256(&original); + let mut patched = original.clone(); + patched.extend_from_slice(b"\n# SOCKET-PATCH-BUNDLER-MARKER\n"); + let after_hash = git_sha256(&patched); + + let socket = tmp.path().join(".socket"); + std::fs::create_dir_all(socket.join("blobs")).unwrap(); + std::fs::write( + socket.join("manifest.json"), + format!( + r#"{{ "patches": {{ + "pkg:gem/colorize@1.1.0": {{ + "uuid": "bundler-uuid-0951", + "exportedAt": "2024-01-01T00:00:00Z", + "files": {{ "package/lib/colorize.rb": {{ + "beforeHash": "{before_hash}", "afterHash": "{after_hash}" + }}}}, + "vulnerabilities": {{}}, "description": "x", + "license": "MIT", "tier": "free" + }} + }}}}"# + ), + ) + .unwrap(); + std::fs::write(socket.join("blobs").join(&after_hash), &patched).unwrap(); + + let mut args = default_apply(tmp.path()); + args.common.ecosystems = Some(vec!["gem".to_string()]); + let code = apply_run(args).await; + assert_eq!(code, 0, "the Bundler-loaded gem must be patchable"); + assert_patched(&lib_file, &patched, &before_hash, &after_hash); +} + // --------------------------------------------------------------------------- // bun install layout // --------------------------------------------------------------------------- diff --git a/crates/socket-patch-core/src/crawlers/ruby_crawler.rs b/crates/socket-patch-core/src/crawlers/ruby_crawler.rs index 6a6f99c93..4984d33fc 100644 --- a/crates/socket-patch-core/src/crawlers/ruby_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/ruby_crawler.rs @@ -368,7 +368,7 @@ impl RubyCrawler { let mut skipped_config_root = None; if Self::has_bundler_manifest(cwd).await { if let Some(value) = - Self::app_config_bundle_path(cwd, app_config_env, ignore_config).await + Self::app_config_bundle_path(cwd, app_config_env, ignore_config, home).await { match resolve_config_bundle_path(cwd, &value, home) { Some(root) => roots.push(root), @@ -406,10 +406,19 @@ impl RubyCrawler { let shadowed = bundle_path_env.is_some() || (!ignore_config && Self::app_config_sets_path(cwd, app_config_env).await); if !shadowed { - if let Some(value) = read_global_config(global_config, false) - .await - .and_then(|text| parse_bundle_config_path(&text)) - { + let text = read_global_config(global_config, false).await; + let value = match text { + Some(text) => { + bundle_config_dir_reading( + parse_bundle_config_path(&text), + parse_legacy_bundle_config_path(&text), + |value| resolve_bundle_path(cwd, Path::new(value), home), + ) + .await + } + None => None, + }; + if let Some(value) = value { roots.push(resolve_bundle_path(cwd, Path::new(&value), home)); } } @@ -623,6 +632,7 @@ impl RubyCrawler { cwd: &Path, app_config_env: Option<&OsStr>, ignore_config: bool, + home: Option<&Path>, ) -> Option { if ignore_config { return None; @@ -637,7 +647,12 @@ impl RubyCrawler { let contents = crate::utils::fs::read_regular_to_string(&config) .await .ok()?; - parse_bundle_config_path(&contents) + bundle_config_dir_reading( + parse_bundle_config_path(&contents), + parse_legacy_bundle_config_path(&contents), + |value| resolve_bundle_path(cwd, Path::new(value), home), + ) + .await } /// Get global gem paths by querying `gem env` and checking well-known locations. @@ -1367,20 +1382,26 @@ pub async fn bundler_app_cache_dir_with_env( ignore_config: bool, global_config: Option<&Path>, ) -> PathBuf { - let mut configured = read_app_config(root, app_config_env, ignore_config) + // Component-wise, so `vendor/gems` uses the native separator. + let resolve = |value: PathBuf| root.join(normalize_lexically(&value).unwrap_or(value)); + let cache_path = |text: Option| async { + let text = text?; + bundle_config_dir_reading( + bundle_config_setting(&text, "BUNDLE_CACHE_PATH"), + legacy_bundle_config_setting(&text, "BUNDLE_CACHE_PATH"), + |value| resolve(PathBuf::from(value)), + ) .await - .and_then(|text| bundle_config_setting(&text, "BUNDLE_CACHE_PATH")) .map(PathBuf::from) + }; + let mut configured = cache_path(read_app_config(root, app_config_env, ignore_config).await) + .await .or_else(|| cache_env.filter(|v| !v.is_empty()).map(PathBuf::from)); if configured.is_none() { - configured = read_global_config(global_config, ignore_config) - .await - .and_then(|text| bundle_config_setting(&text, "BUNDLE_CACHE_PATH")) - .map(PathBuf::from); + configured = cache_path(read_global_config(global_config, ignore_config).await).await; } match configured { - // Component-wise, so `vendor/gems` uses the native separator. - Some(value) => root.join(normalize_lexically(&value).unwrap_or(value)), + Some(value) => resolve(value), None => root.join("vendor").join("cache"), } } @@ -1472,16 +1493,26 @@ fn resolve_config_bundle_path( /// 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 { + parse_bundle_config_path_with(contents, unquote_bundle_config_value) +} + +/// [`parse_bundle_config_path`] as a Bundler before 2.5.6 reads it (see +/// [`unquote_legacy_bundle_config_value`]). +fn parse_legacy_bundle_config_path(contents: &str) -> Option { + parse_bundle_config_path_with(contents, unquote_legacy_bundle_config_value) +} + +fn parse_bundle_config_path_with(contents: &str, unquote: fn(&str) -> &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 = unquote_bundle_config_value(rest); + let v = unquote(rest); if !v.is_empty() { path = Some(v.to_string()); } } else if let Some(rest) = line.strip_prefix("BUNDLE_PATH__SYSTEM:") { - path_system = unquote_bundle_config_value(rest) == "true"; + path_system = unquote(rest) == "true"; } } if path_system { @@ -1505,24 +1536,93 @@ pub(crate) fn bundle_config_setting(contents: &str, key: &str) -> Option /// path. Callers that decide whether a lower tier applies need presence, /// not just a non-empty value. pub(crate) fn bundle_config_setting_including_empty(contents: &str, key: &str) -> Option { + bundle_config_setting_with(contents, key, unquote_bundle_config_value) +} + +/// [`bundle_config_setting`] as a Bundler before 2.5.6 reads it (see +/// [`unquote_legacy_bundle_config_value`]). +fn legacy_bundle_config_setting(contents: &str, key: &str) -> Option { + bundle_config_setting_with(contents, key, unquote_legacy_bundle_config_value) + .filter(|value| !value.is_empty()) +} + +fn bundle_config_setting_with( + contents: &str, + key: &str, + unquote: fn(&str) -> &str, +) -> Option { let mut found = None; for line in contents.lines() { if let Some(rest) = line.strip_prefix(key).and_then(|r| r.strip_prefix(':')) { - let v = unquote_bundle_config_value(rest); - found = Some(v.to_string()); + found = Some(unquote(rest).to_string()); } } found } -/// Unwrap one bundler app-config scalar: trim, then strip one matching -/// pair of double or single quotes (bundler double-quotes what it writes). +/// Unwrap one bundler app-config scalar the way Bundler's config loader +/// does (`Gem::YAMLSerializer`, RubyGems 3.5.6+, which Bundler 2.4+ uses +/// when present; Bundler's own copy from 2.5.6): trim, strip one matching +/// pair of double or single quotes (bundler double-quotes what it writes), +/// then `strip_comment` — cut the value at its first `#` and trim, unless +/// it starts with `#` (#951). The quote pair must close the line, so +/// `"vendor/bundle" # note` keeps its quotes, as it does for Bundler. pub(crate) fn unquote_bundle_config_value(rest: &str) -> &str { let v = rest.trim(); + strip_bundle_config_comment(unquote_matching_pair(v).unwrap_or(v)) +} + +/// [`unquote_bundle_config_value`] for the loader Bundler used before +/// `strip_comment` (Bundler < 2.4, or 2.4–2.5.5 on RubyGems < 3.5.6): the +/// `# comment` stays part of the value. +fn unquote_legacy_bundle_config_value(rest: &str) -> &str { + let v = rest.trim(); + unquote_matching_pair(v).unwrap_or(v) +} + +fn unquote_matching_pair(v: &str) -> Option<&str> { v.strip_prefix('"') .and_then(|s| s.strip_suffix('"')) .or_else(|| v.strip_prefix('\'').and_then(|s| s.strip_suffix('\''))) - .unwrap_or(v) +} + +/// Bundler's `YAMLSerializer#strip_comment`. +fn strip_bundle_config_comment(v: &str) -> &str { + match v.split_once('#') { + Some((value, _)) if !v.starts_with('#') => value.trim(), + _ => v, + } +} + +/// A directory setting whose value carries a `# comment` reads differently +/// in the two Bundler config-loader eras (see +/// [`unquote_legacy_bundle_config_value`]), and the installed Bundler's era +/// is not known here. Bundler creates the directory it uses, so take the +/// current reading unless only the legacy reading's directory exists. +/// Values without a comment read the same in both eras. +async fn bundle_config_dir_reading( + current: Option, + legacy: Option, + resolve: impl Fn(&str) -> PathBuf, +) -> Option { + let Some(legacy) = legacy.filter(|legacy| current.as_ref() != Some(legacy)) else { + return current; + }; + let is_dir = |path: PathBuf| async move { + tokio::fs::metadata(path) + .await + .is_ok_and(|meta| meta.is_dir()) + }; + if let Some(value) = ¤t { + if is_dir(resolve(value)).await { + return current; + } + } + if is_dir(resolve(&legacy)).await { + Some(legacy) + } else { + current + } } /// Whether a PURL-derived gem coordinate is safe to join onto the gem root. @@ -4059,4 +4159,125 @@ mod tests { } assert!(found > 50, "vacuous fixtures: {found}"); } + + /// #951: Bundler's config loader (`Gem::YAMLSerializer#strip_comment`, + /// RubyGems / Bundler 2.5.6+) cuts a `.bundle/config` value at its + /// first `#` unless the value starts with one, and applies that to the + /// unquoted value. Pinned against the real loader's output. + #[test] + fn bundle_config_values_drop_a_trailing_comment_like_bundler() { + let text = "---\nBUNDLE_PATH: .gems # project-local gems\n\ + BUNDLE_A: \"a#b\"\nBUNDLE_B: x#y\nBUNDLE_C: #z\n\ + BUNDLE_D: \"vendor/bundle\" # quoted then commented\n\ + BUNDLE_E: \"a # b\"\nBUNDLE_F: ' q ' \n"; + let get = |key| bundle_config_setting_including_empty(text, key); + assert_eq!(get("BUNDLE_PATH").as_deref(), Some(".gems")); + assert_eq!(get("BUNDLE_A").as_deref(), Some("a")); + assert_eq!(get("BUNDLE_B").as_deref(), Some("x")); + // A value that STARTS with `#` is kept whole. + assert_eq!(get("BUNDLE_C").as_deref(), Some("#z")); + // The closing quote is not at the end of the line, so the loader + // matches no quote pair: the quotes stay part of the value. + assert_eq!(get("BUNDLE_D").as_deref(), Some("\"vendor/bundle\"")); + assert_eq!(get("BUNDLE_E").as_deref(), Some("a")); + // No `#`: the unquoted value is kept as is. + assert_eq!(get("BUNDLE_F").as_deref(), Some(" q ")); + } + + /// #951: a commented `path`, `path.system`, `cache_path` and `gemfile` + /// read the way Bundler reads them. + #[tokio::test] + async fn commented_bundle_config_settings_follow_bundler() { + assert_eq!( + parse_bundle_config_path("---\nBUNDLE_PATH: .gems # project-local gems\n"), + Some(".gems".to_string()) + ); + assert_eq!( + parse_bundle_config_path( + "---\nBUNDLE_PATH: vendor/bundle\nBUNDLE_PATH__SYSTEM: true # use system gems\n" + ), + None + ); + assert_eq!( + crate::formats::gem::manifest::config_gemfile("---\nBUNDLE_GEMFILE: gems.rb # twin\n") + .as_deref(), + Some("gems.rb") + ); + + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + std::fs::create_dir_all(root.join(".bundle")).unwrap(); + std::fs::write( + root.join(".bundle").join("config"), + "---\nBUNDLE_CACHE_PATH: vendor/gems # committed gem cache\n", + ) + .unwrap(); + assert_eq!( + bundler_app_cache_dir_with_env(root, None, None, false, None).await, + root.join("vendor").join("gems") + ); + } + + /// #951 repro: `BUNDLE_PATH: .gems # comment` must discover the + /// `.gems` store Bundler installs into, not a directory named after + /// the whole line. + #[tokio::test] + async fn commented_app_config_bundle_path_discovers_the_bundler_store() { + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("Gemfile"), b"gem \"colorize\"\n").unwrap(); + std::fs::create_dir_all(dir.path().join(".bundle")).unwrap(); + std::fs::write( + dir.path().join(".bundle").join("config"), + "---\nBUNDLE_PATH: .gems # project-local gems\n", + ) + .unwrap(); + let gems = dir + .path() + .join(".gems") + .join("ruby") + .join("3.3.0") + .join("gems"); + std::fs::create_dir_all(gems.join("colorize-0.8.1").join("lib")).unwrap(); + std::fs::create_dir_all(dir.path().join(".gems/ruby/3.3.0/specifications")).unwrap(); + + let paths = RubyCrawler::get_vendor_bundle_paths_with_env(dir.path(), None, None).await; + assert_eq!(paths, vec![gems]); + } + + /// Bundler before 2.5.6 (or 2.4/2.5 on RubyGems before 3.5.6) keeps the + /// comment in the value and installs into a directory named after it. + /// When only that legacy-era directory exists, discovery must still + /// find the store (the pre-#951 behaviour), not fall back to the + /// system gem homes. + #[tokio::test] + async fn commented_bundle_path_keeps_the_legacy_bundler_store() { + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("Gemfile"), b"gem \"colorize\"\n").unwrap(); + std::fs::create_dir_all(dir.path().join(".bundle")).unwrap(); + std::fs::write( + dir.path().join(".bundle").join("config"), + "---\nBUNDLE_PATH: .gems # note\n", + ) + .unwrap(); + let root = dir.path().join(".gems # note"); + let gems = root.join("ruby").join("2.7.0").join("gems"); + std::fs::create_dir_all(gems.join("colorize-0.8.1").join("lib")).unwrap(); + std::fs::create_dir_all(root.join("ruby/2.7.0/specifications")).unwrap(); + + let paths = RubyCrawler::get_vendor_bundle_paths_with_env(dir.path(), None, None).await; + assert_eq!(paths, vec![gems]); + + // Same for the cache path: only the legacy-era dir exists. + std::fs::write( + dir.path().join(".bundle").join("config"), + "---\nBUNDLE_CACHE_PATH: vendor/gems # c\n", + ) + .unwrap(); + let legacy_cache = dir.path().join("vendor").join("gems # c"); + std::fs::create_dir_all(&legacy_cache).unwrap(); + assert_eq!( + bundler_app_cache_dir_with_env(dir.path(), None, None, false, None).await, + legacy_cache + ); + } } From 3f8e1c1cd53d1b5f3ca229b368955fd212e7aca4 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 18:24:21 +0000 Subject: [PATCH 3/4] Route Gradle digests through utils::digest 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 659ac2c24e5c5904e743b4bc98ea4645da2ed6a1) --- crates/socket-patch-core/src/crawlers/gradle_cache.rs | 9 ++++----- crates/socket-patch-core/src/patch/jvm_jar.rs | 7 ++----- crates/socket-patch-core/src/patch/sidecars/maven.rs | 4 +--- 3 files changed, 7 insertions(+), 13 deletions(-) diff --git a/crates/socket-patch-core/src/crawlers/gradle_cache.rs b/crates/socket-patch-core/src/crawlers/gradle_cache.rs index ef295ee27..afd7c4fba 100644 --- a/crates/socket-patch-core/src/crawlers/gradle_cache.rs +++ b/crates/socket-patch-core/src/crawlers/gradle_cache.rs @@ -70,8 +70,7 @@ pub fn hash_eq(dir_name: &str, sha1_hex: &str) -> bool { /// Whether `bytes` are the pristine download Gradle stored in the hash /// directory `dir_name` (their sha1 names it). pub fn pristine(dir_name: &str, bytes: &[u8]) -> bool { - use sha1::{Digest, Sha1}; - hash_eq(dir_name, &hex::encode(Sha1::digest(bytes))) + hash_eq(dir_name, &crate::utils::digest::sha1_hex_of(bytes)) } /// Whether `path` is a version directory of a `files-2.1` tree @@ -432,8 +431,6 @@ impl DerivedIndex { /// The [`DerivedCopies`] of the jar `jar_leaf` whose pristine bytes /// hash to `pristine_sha1`. pub fn query(&self, jar_leaf: &str, pristine_sha1: &str) -> DerivedCopies { - use sha1::{Digest, Sha1}; - let instrumented = format!("instrumented-{jar_leaf}"); let mut out = DerivedCopies { incomplete: self.incomplete, @@ -460,7 +457,9 @@ impl DerivedIndex { out.stale.push(path.clone()); } else if name == jar_leaf || name == instrumented { match crate::utils::fs::read_regular_to_bytes_sync(path) { - Ok(bytes) if hash_eq(&hex::encode(Sha1::digest(&bytes)), pristine_sha1) => { + Ok(bytes) + if hash_eq(&crate::utils::digest::sha1_hex_of(&bytes), pristine_sha1) => + { out.stale.push(path.clone()) } Ok(_) => out.unknown.push(path.clone()), diff --git a/crates/socket-patch-core/src/patch/jvm_jar.rs b/crates/socket-patch-core/src/patch/jvm_jar.rs index 82d679406..f38a84403 100644 --- a/crates/socket-patch-core/src/patch/jvm_jar.rs +++ b/crates/socket-patch-core/src/patch/jvm_jar.rs @@ -25,8 +25,6 @@ use std::collections::HashMap; use std::path::{Path, PathBuf}; -use sha1::Digest as _; - use crate::crawlers::gradle_cache; use crate::hash::git_sha256::compute_git_sha256_from_bytes; use crate::manifest::schema::PatchFileInfo; @@ -353,12 +351,11 @@ fn unpatched_members( } fn sha256_hex(bytes: &[u8]) -> String { - use sha2::Digest as _; - hex::encode(sha2::Sha256::digest(bytes)) + crate::utils::digest::sha256_hex_of(bytes) } fn sha1_hex(bytes: &[u8]) -> String { - hex::encode(sha1::Sha1::digest(bytes)) + crate::utils::digest::sha1_hex_of(bytes) } /// `/jvm-originals/.jar`. diff --git a/crates/socket-patch-core/src/patch/sidecars/maven.rs b/crates/socket-patch-core/src/patch/sidecars/maven.rs index f2f5a2466..8798bfce6 100644 --- a/crates/socket-patch-core/src/patch/sidecars/maven.rs +++ b/crates/socket-patch-core/src/patch/sidecars/maven.rs @@ -17,8 +17,6 @@ use std::path::{Path, PathBuf}; -use sha1::Digest as _; - use super::{ SidecarAdvisory, SidecarAdvisoryCode, SidecarError, SidecarFile, SidecarFileAction, SidecarPayload, SidecarSeverity, @@ -44,7 +42,7 @@ impl Algo { fn digest(self, bytes: &[u8]) -> String { match self { - Algo::Sha1 => hex::encode(sha1::Sha1::digest(bytes)), + Algo::Sha1 => crate::utils::digest::sha1_hex_of(bytes), Algo::Md5 => hex::encode(md5(bytes)), } } From 52542db91d2703e919cabd39fa4b422eeedef83a Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 18:34:29 +0000 Subject: [PATCH 4/4] Keep commented path.system from reviving a path A `BUNDLE_PATH__SYSTEM: true # note` line makes Bundler 2.5.6+ ignore the recorded BUNDLE_PATH and load system gems. The era fallback added for #951 could still pick the recorded path when a leftover directory existed there, so apply would patch a copy Bundler never loads. The legacy reading now only competes with another directory reading; an unset current reading always stands. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/crawlers/ruby_crawler.rs | 42 +++++++++++++++---- 1 file changed, 35 insertions(+), 7 deletions(-) diff --git a/crates/socket-patch-core/src/crawlers/ruby_crawler.rs b/crates/socket-patch-core/src/crawlers/ruby_crawler.rs index 4984d33fc..78656b213 100644 --- a/crates/socket-patch-core/src/crawlers/ruby_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/ruby_crawler.rs @@ -1600,25 +1600,29 @@ fn strip_bundle_config_comment(v: &str) -> &str { /// is not known here. Bundler creates the directory it uses, so take the /// current reading unless only the legacy reading's directory exists. /// Values without a comment read the same in both eras. +/// +/// Only two directory readings are weighed against each other. When the +/// current reading is unset — e.g. a commented `path.system: true` that +/// only the current loader honours — the legacy path's directory is no +/// evidence of the era (it may be a leftover install, the #915 shape), so +/// the current reading stands. async fn bundle_config_dir_reading( current: Option, legacy: Option, resolve: impl Fn(&str) -> PathBuf, ) -> Option { - let Some(legacy) = legacy.filter(|legacy| current.as_ref() != Some(legacy)) else { + let (Some(value), Some(legacy)) = (current.as_deref(), legacy) else { return current; }; + if value == legacy { + return current; + } let is_dir = |path: PathBuf| async move { tokio::fs::metadata(path) .await .is_ok_and(|meta| meta.is_dir()) }; - if let Some(value) = ¤t { - if is_dir(resolve(value)).await { - return current; - } - } - if is_dir(resolve(&legacy)).await { + if !is_dir(resolve(value)).await && is_dir(resolve(&legacy)).await { Some(legacy) } else { current @@ -4280,4 +4284,28 @@ mod tests { legacy_cache ); } + + /// Bugbot on #953: a commented `path.system: true` drops the recorded + /// path under the current loader. A leftover directory at that + /// recorded path must not bring it back through the legacy reading. + #[tokio::test] + async fn commented_path_system_true_ignores_a_leftover_recorded_path() { + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("Gemfile"), b"gem \"foo\"\n").unwrap(); + std::fs::create_dir_all(dir.path().join(".bundle")).unwrap(); + std::fs::write( + dir.path().join(".bundle").join("config"), + "---\nBUNDLE_PATH: vendor/mygems\nBUNDLE_PATH__SYSTEM: true # use system gems\n", + ) + .unwrap(); + let root = dir.path().join("vendor").join("mygems"); + std::fs::create_dir_all(root.join("gems").join("foo-1.0.0").join("lib")).unwrap(); + std::fs::create_dir_all(root.join("specifications")).unwrap(); + + let paths = RubyCrawler::get_vendor_bundle_paths_with_env(dir.path(), None, None).await; + assert!( + paths.is_empty(), + "a commented path.system=true must still drop the config root: {paths:?}" + ); + } }