From 451e3fb659a72ba1e2d58c0a39dbda2a51f73481 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 21:22:09 +0000 Subject: [PATCH 1/3] Start fix for #896 Assisted-by: Claude Code:claude-opus-5-5 From 6fc745487273fad377e51c21211fab8597b9c4b4 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 21:32:27 +0000 Subject: [PATCH 2/3] Accept BUNDLE_GEMFILE through a symlinked dir A project reached through a symlinked directory (macOS /tmp, a symlinked workspace) had BUNDLE_GEMFILE=$PWD/Gemfile, or the same path in .bundle/config, refused as "another manifest". Hosted scan, vendored mode, vex and rollback then refused a correctly configured project. Bundler expands the setting lexically and opens it through the OS, so compare the file name lexically and the directory by resolved identity. The same applies to BUNDLE_LOCKFILE and to deciding whether the env value keeps Bundler's root. A root Gemfile that itself links elsewhere stays distinct, since Bundler's root follows the spelled directory. Fixes #896 Assisted-by: Claude Code:claude-opus-5-5 --- .../src/crawlers/ruby_crawler.rs | 35 +++++ .../src/formats/gem/manifest.rs | 129 +++++++++++++++++- 2 files changed, 158 insertions(+), 6 deletions(-) diff --git a/crates/socket-patch-core/src/crawlers/ruby_crawler.rs b/crates/socket-patch-core/src/crawlers/ruby_crawler.rs index ec70c30f0..0a218d254 100644 --- a/crates/socket-patch-core/src/crawlers/ruby_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/ruby_crawler.rs @@ -3742,6 +3742,41 @@ mod tests { ); } + /// #896: a project reached through a symlinked directory, with + /// `bundle config set --local gemfile "$PWD/Gemfile"` written through the + /// link (or the physical path with `--cwd` through the link): bundler + /// loads the project's own Gemfile, so the app config is followed, not + /// refused as another manifest. + #[cfg(unix)] + #[tokio::test] + async fn loaded_manifest_follows_a_symlinked_spelling_of_the_project() { + use crate::formats::gem::manifest::{GemfileSetting, LoadedManifest}; + let dir = tempfile::tempdir().unwrap(); + let real = dir.path().join("real/app"); + std::fs::create_dir_all(real.join(".bundle")).unwrap(); + std::fs::write(real.join("Gemfile"), "").unwrap(); + std::os::unix::fs::symlink(dir.path().join("real"), dir.path().join("link")).unwrap(); + let link = dir.path().join("link/app"); + for (root, spelled) in [(&real, &link), (&link, &real)] { + std::fs::write( + real.join(".bundle/config"), + format!( + "---\nBUNDLE_GEMFILE: \"{}\"\n", + spelled.join("Gemfile").display() + ), + ) + .unwrap(); + assert_eq!( + bundler_loaded_manifest_with_env(root, BundlerEnv::default()).await, + LoadedManifest::Configured { + manifest: "Gemfile", + by: GemfileSetting::AppConfig + }, + "{root:?}" + ); + } + } + /// #577: a global `gemfile` setting is the third tier: below the local /// config and the environment, and refused like a local one when it /// names a manifest socket-patch does not wire. diff --git a/crates/socket-patch-core/src/formats/gem/manifest.rs b/crates/socket-patch-core/src/formats/gem/manifest.rs index 2b5ef9d1d..071d13e89 100644 --- a/crates/socket-patch-core/src/formats/gem/manifest.rs +++ b/crates/socket-patch-core/src/formats/gem/manifest.rs @@ -43,8 +43,13 @@ //! nothing on disk says which bundler runs: callers refuse it with //! [`twin_manifest_refusal`] (#751). //! -//! The model is pure: -//! the disk and environment reads live in +//! A configured path names a root file when its file name matches and its +//! directory is the root's, compared lexically and, failing that, by the +//! directories' resolved identity: bundler expands the setting lexically +//! and opens it through the OS, so a symlinked spelling of the project +//! directory (macOS `/tmp` → `/private/tmp`) names the same file (#896). +//! That directory lookup is the model's only disk read; the other disk and +//! environment reads live in //! [`crate::crawlers::ruby_crawler::bundler_loaded_manifest`]. use std::ffi::OsStr; @@ -200,8 +205,10 @@ impl LoadedManifest { } let target = resolve_against(root, &value); let expected = resolve_against(root, Path::new(lock)); - if target.is_some() && target == expected { - return self; + if let (Some(target), Some(expected)) = (target, expected) { + if same_entry(&target, &expected) { + return self; + } } LoadedManifest::UnsupportedLockfile { value: value.display().to_string(), @@ -252,6 +259,27 @@ fn resolve_against(root: &Path, value: &Path) -> Option { .and_then(|p| normalize_lexically(&p)) } +/// Whether two resolved directories are the same directory: equal, or both +/// resolving to the same physical path through symlinks. +fn same_dir(a: &Path, b: &Path) -> bool { + a == b + || matches!( + (std::fs::canonicalize(a), std::fs::canonicalize(b)), + (Ok(a), Ok(b)) if a == b + ) +} + +/// Whether two resolved paths name the same directory entry: the same file +/// name in the [`same_dir`] directory. Only the directory is resolved, so a +/// root `Gemfile` that is itself a symlink to another manifest stays +/// distinct from that manifest, as bundler's root follows the spelled +/// directory. +fn same_entry(a: &Path, b: &Path) -> bool { + a == b + || (a.file_name() == b.file_name() + && matches!((a.parent(), b.parent()), (Some(a), Some(b)) if same_dir(a, b))) +} + /// Classify the configured `BUNDLE_GEMFILE` against `root`, which also /// anchors a relative value: the app config value first, then the /// environment — unless the environment names a manifest outside `root`, @@ -269,7 +297,10 @@ pub fn classify( let global = global_value.filter(|v| !v.is_empty()).map(PathBuf::from); let env_keeps_root = |env: &Path| { let dir = resolve_against(root, env).and_then(|p| p.parent().map(Path::to_path_buf)); - dir.is_some() && dir == resolve_against(root, Path::new("")) + match (dir, resolve_against(root, Path::new(""))) { + (Some(dir), Some(root)) => same_dir(&dir, &root), + _ => false, + } }; let (value, by) = match (env, config) { (Some(env), Some(config)) if env_keeps_root(&env) => (config, GemfileSetting::AppConfig), @@ -291,7 +322,7 @@ pub fn classify( let root = resolve_against(root, Path::new("")); if let (Some(target), Some(root)) = (target, root) { for manifest in ["Gemfile", "gems.rb"] { - if target == root.join(manifest) { + if same_entry(&target, &root.join(manifest)) { return LoadedManifest::Configured { manifest, by }; } } @@ -609,6 +640,92 @@ mod tests { } } + /// A project directory reached through a symlink (macOS `/tmp` → + /// `/private/tmp`, a symlinked workspace): bundler expands + /// `BUNDLE_GEMFILE` lexically and opens it through the OS, so a value + /// spelling the root's own `Gemfile` through the link is that Gemfile, + /// whichever side (the setting or `--cwd`) uses the link (#896). + #[cfg(unix)] + #[test] + fn a_symlinked_spelling_of_the_root_is_the_roots_own_manifest() { + let tmp = tempfile::tempdir().unwrap(); + let real = tmp.path().join("real/app"); + std::fs::create_dir_all(&real).unwrap(); + std::fs::write(real.join("Gemfile"), "").unwrap(); + std::os::unix::fs::symlink(tmp.path().join("real"), tmp.path().join("link")).unwrap(); + let link = tmp.path().join("link/app"); + let configured = |by| LoadedManifest::Configured { + manifest: "Gemfile", + by, + }; + for (root, spelled) in [(&real, &link), (&link, &real)] { + let value = spelled.join("Gemfile"); + // `BUNDLE_GEMFILE=$PWD/Gemfile` with a logical `$PWD`. + assert_eq!( + classify(root, Some(value.as_os_str()), None, None), + configured(GemfileSetting::Env), + "{root:?} {value:?}" + ); + // `bundle config set --local gemfile "$PWD/Gemfile"`. + assert_eq!( + classify(root, None, value.to_str(), None), + configured(GemfileSetting::AppConfig) + ); + assert_eq!( + classify(root, None, None, value.to_str()), + configured(GemfileSetting::GlobalConfig) + ); + // The env value keeps bundler's root (same directory), so the + // app config still wins over it. + let next = spelled.join("Gemfile.next"); + assert_eq!( + classify(root, Some(next.as_os_str()), Some("Gemfile"), None), + configured(GemfileSetting::AppConfig) + ); + // `BUNDLE_LOCKFILE` naming the pair's own lock the same way. + let lock = spelled.join("Gemfile.lock"); + assert_eq!( + classify(root, None, None, None).with_lockfile( + root, + Some(lock.as_os_str()), + None, + None, + false + ), + LoadedManifest::Default + ); + // Another file in the same directory is still another manifest. + assert!(matches!( + classify(root, Some(next.as_os_str()), None, None), + LoadedManifest::Unsupported { .. } + )); + } + } + + /// Only the DIRECTORY is resolved: a root `Gemfile` that is itself a + /// symlink to a manifest elsewhere does not make that manifest the + /// project's, since bundler's root (and its lock) follow the spelled + /// directory, not the link target. + #[cfg(unix)] + #[test] + fn a_manifest_the_roots_gemfile_links_to_is_still_another_manifest() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join("app"); + let other = tmp.path().join("other"); + std::fs::create_dir_all(&root).unwrap(); + std::fs::create_dir_all(&other).unwrap(); + std::fs::write(other.join("Gemfile"), "").unwrap(); + std::os::unix::fs::symlink(other.join("Gemfile"), root.join("Gemfile")).unwrap(); + let value = other.join("Gemfile"); + assert_eq!( + classify(&root, Some(value.as_os_str()), None, None), + LoadedManifest::Unsupported { + value: value.display().to_string(), + by: GemfileSetting::Env + } + ); + } + #[test] fn config_lockfile_reads_bundlers_own_spelling() { assert_eq!( From 0528f9fe164770c9969d0c0fe4cf4eafddf7d76d Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 22:09:10 +0000 Subject: [PATCH 3/3] Compare gem manifest dirs by file identity Address review of the symlink fix: decide directory identity with same-file (device and inode) so a bind mount or Windows junction also counts, check it once per classify, and keep an in-memory project's paths lexical so the host's own directories never decide its verdict. Refs #896 Assisted-by: Claude Code:claude-opus-5-5 --- .../src/crawlers/ruby_crawler.rs | 7 +- .../src/formats/gem/manifest.rs | 123 ++++++++++++++++-- 2 files changed, 114 insertions(+), 16 deletions(-) diff --git a/crates/socket-patch-core/src/crawlers/ruby_crawler.rs b/crates/socket-patch-core/src/crawlers/ruby_crawler.rs index 0a218d254..d0edbb072 100644 --- a/crates/socket-patch-core/src/crawlers/ruby_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/ruby_crawler.rs @@ -1703,7 +1703,8 @@ pub(crate) async fn bundler_loaded_manifest_in( ProjectView::Snapshot(snap) => { // The only files the probe reads: the app config, the global // config and `gems.rb` (the environment it also reads is fixed - // for the run). + // for the run, and the directory identity of a configured + // absolute path is not a file the snapshot fingerprints). let root = snap.root_reading(bundler_config_files(snap.root_reading::<&Path>([]))); bundler_loaded_manifest(root).await } @@ -1713,7 +1714,7 @@ pub(crate) async fn bundler_loaded_manifest_in( let lockfile = config.as_deref().and_then(manifest::config_lockfile); let root = Path::new("/"); let gems_rb = view.is_file("gems.rb"); - let loaded = manifest::classify(root, None, gemfile.as_deref(), None); + let loaded = manifest::classify_lexical(root, gemfile.as_deref()); // `/` only stands in for the project root: bundler opens an // absolute lockfile as is, never the project's own lock, so // it must not compare equal to the pair's lock at `/`. @@ -1725,7 +1726,7 @@ pub(crate) async fn bundler_loaded_manifest_in( }; } } - loaded.with_lockfile(root, None, lockfile.as_deref(), None, gems_rb) + loaded.with_lockfile_lexical(root, lockfile.as_deref(), gems_rb) } } } diff --git a/crates/socket-patch-core/src/formats/gem/manifest.rs b/crates/socket-patch-core/src/formats/gem/manifest.rs index 071d13e89..41d8ff1c3 100644 --- a/crates/socket-patch-core/src/formats/gem/manifest.rs +++ b/crates/socket-patch-core/src/formats/gem/manifest.rs @@ -187,6 +187,44 @@ impl LoadedManifest { lockfile_config: Option<&str>, lockfile_global: Option<&str>, gems_rb_present: bool, + ) -> LoadedManifest { + self.with_lockfile_by( + root, + lockfile_env, + lockfile_config, + lockfile_global, + gems_rb_present, + same_dir_on_disk, + ) + } + + /// [`LoadedManifest::with_lockfile`] for a `root` that only stands in + /// for a project not on this disk (an in-memory view): paths compare + /// lexically, never through this host's directories. + pub(crate) fn with_lockfile_lexical( + self, + root: &Path, + lockfile_config: Option<&str>, + gems_rb_present: bool, + ) -> LoadedManifest { + self.with_lockfile_by( + root, + None, + lockfile_config, + None, + gems_rb_present, + |a, b| a == b, + ) + } + + fn with_lockfile_by( + self, + root: &Path, + lockfile_env: Option<&OsStr>, + lockfile_config: Option<&str>, + lockfile_global: Option<&str>, + gems_rb_present: bool, + same_dir: SameDir, ) -> LoadedManifest { let Some((_, lock)) = self.pair(gems_rb_present) else { return self; @@ -206,7 +244,7 @@ impl LoadedManifest { let target = resolve_against(root, &value); let expected = resolve_against(root, Path::new(lock)); if let (Some(target), Some(expected)) = (target, expected) { - if same_entry(&target, &expected) { + if same_entry(&target, &expected, same_dir) { return self; } } @@ -259,22 +297,22 @@ fn resolve_against(root: &Path, value: &Path) -> Option { .and_then(|p| normalize_lexically(&p)) } -/// Whether two resolved directories are the same directory: equal, or both -/// resolving to the same physical path through symlinks. -fn same_dir(a: &Path, b: &Path) -> bool { - a == b - || matches!( - (std::fs::canonicalize(a), std::fs::canonicalize(b)), - (Ok(a), Ok(b)) if a == b - ) +/// How two resolved directories are judged the same directory. +type SameDir = fn(&Path, &Path) -> bool; + +/// Whether two resolved directories are the same directory on this disk: +/// equal, or the same directory by file identity (a symlinked spelling, a +/// bind mount, a Windows junction). +fn same_dir_on_disk(a: &Path, b: &Path) -> bool { + a == b || same_file::is_same_file(a, b).unwrap_or(false) } /// Whether two resolved paths name the same directory entry: the same file -/// name in the [`same_dir`] directory. Only the directory is resolved, so a +/// name in the `same_dir` directory. Only the directory is resolved, so a /// root `Gemfile` that is itself a symlink to another manifest stays /// distinct from that manifest, as bundler's root follows the spelled /// directory. -fn same_entry(a: &Path, b: &Path) -> bool { +fn same_entry(a: &Path, b: &Path, same_dir: SameDir) -> bool { a == b || (a.file_name() == b.file_name() && matches!((a.parent(), b.parent()), (Some(a), Some(b)) if same_dir(a, b))) @@ -291,6 +329,29 @@ pub fn classify( gemfile_env: Option<&OsStr>, config_value: Option<&str>, global_value: Option<&str>, +) -> LoadedManifest { + classify_by( + root, + gemfile_env, + config_value, + global_value, + same_dir_on_disk, + ) +} + +/// [`classify`] of an app config value for a `root` that only stands in +/// for a project not on this disk (an in-memory view): paths compare +/// lexically, never through this host's directories. +pub(crate) fn classify_lexical(root: &Path, config_value: Option<&str>) -> LoadedManifest { + classify_by(root, None, config_value, None, |a, b| a == b) +} + +fn classify_by( + root: &Path, + gemfile_env: Option<&OsStr>, + config_value: Option<&str>, + global_value: Option<&str>, + same_dir: SameDir, ) -> LoadedManifest { let env = gemfile_env.filter(|v| !v.is_empty()).map(PathBuf::from); let config = config_value.filter(|v| !v.is_empty()).map(PathBuf::from); @@ -321,8 +382,11 @@ pub fn classify( let target = resolve_against(root, &value); let root = resolve_against(root, Path::new("")); if let (Some(target), Some(root)) = (target, root) { - for manifest in ["Gemfile", "gems.rb"] { - if same_entry(&target, &root.join(manifest)) { + let manifest = ["Gemfile", "gems.rb"] + .into_iter() + .find(|m| target.file_name() == Some(OsStr::new(m))); + if let Some(manifest) = manifest { + if same_entry(&target, &root.join(manifest), same_dir) { return LoadedManifest::Configured { manifest, by }; } } @@ -702,6 +766,39 @@ mod tests { } } + /// An in-memory view's root only stands in for a project that is not on + /// this disk, so its paths never resolve through this host's + /// directories: a symlinked spelling stays another manifest there. + #[cfg(unix)] + #[test] + fn the_lexical_variants_never_read_this_hosts_directories() { + let tmp = tempfile::tempdir().unwrap(); + let real = tmp.path().join("real/app"); + std::fs::create_dir_all(&real).unwrap(); + std::os::unix::fs::symlink(tmp.path().join("real"), tmp.path().join("link")).unwrap(); + let link = tmp.path().join("link/app"); + let value = link.join("Gemfile"); + assert_eq!( + classify_lexical(&real, value.to_str()), + LoadedManifest::Unsupported { + value: value.display().to_string(), + by: GemfileSetting::AppConfig + } + ); + assert_eq!( + classify_lexical(&real, Some("Gemfile")), + LoadedManifest::Configured { + manifest: "Gemfile", + by: GemfileSetting::AppConfig + } + ); + let lock = link.join("Gemfile.lock"); + assert!(matches!( + LoadedManifest::Default.with_lockfile_lexical(&real, lock.to_str(), false), + LoadedManifest::UnsupportedLockfile { .. } + )); + } + /// Only the DIRECTORY is resolved: a root `Gemfile` that is itself a /// symlink to a manifest elsewhere does not make that manifest the /// project's, since bundler's root (and its lock) follow the spelled