diff --git a/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs b/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs index 93cebb4f4..04df63544 100644 --- a/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs +++ b/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs @@ -466,6 +466,11 @@ enum Driver { /// run must redirect nothing and attest nothing. Bundler >= 4 only; the /// fixture asserts the contract itself and yields `None`. ScanVexCustomLockfile, + /// [`Driver::ScanVexCustomLockfile`] with the lock named by a + /// `lockfile "custom.lock"` call at the top of the Gemfile instead of + /// the config (#1477): bundler 4's DSL spelling, which outranks every + /// `BUNDLE_LOCKFILE` tier. Bundler >= 4 only. + ScanVexCustomLockfileDsl, /// [`Driver::ScanVex`] on a `Gemfile` + `gems.rb` twin (#751): bundler /// 1.x loads the `Gemfile` and >= 2 loads `gems.rb`, and the scan cannot /// see which runs, so it must redirect and attest nothing and leave all @@ -523,6 +528,9 @@ impl Driver { "scan --mode hosted (config Gemfile.next, env BUNDLE_GEMFILE=Gemfile)" } Driver::ScanVexCustomLockfile => "scan --mode hosted (lockfile custom.lock)", + Driver::ScanVexCustomLockfileDsl => { + "scan --mode hosted (Gemfile `lockfile \"custom.lock\"` call)" + } Driver::ScanVexTwin => "scan --mode hosted (Gemfile + gems.rb twin)", Driver::ScanVexGroupBlock => "scan --mode hosted (gem in a group block)", Driver::ScanVexSourceBlock => "scan --mode hosted (gem in a source block)", @@ -641,7 +649,9 @@ async fn redirect_scanned_project( })?; // Drivers that only mean something on one bundler line. let only = match driver { - Driver::ScanVexCustomLockfile if !bundler.at_least(4, 0) => { + Driver::ScanVexCustomLockfile | Driver::ScanVexCustomLockfileDsl + if !bundler.at_least(4, 0) => + { Some("custom lockfiles need bundler >= 4") } _ => None, @@ -976,7 +986,7 @@ async fn redirect_scanned_project( } // The pre-redirect (registry) pair — what reverting the patch commit // restores; the manifest-less VEX legs revert to it. - let pristine_gemfile = std::fs::read(proj.join(gemfile_name)).unwrap(); + let mut pristine_gemfile = std::fs::read(proj.join(gemfile_name)).unwrap(); let pristine_lock = lock_before.clone().into_bytes(); // Drop the pristine materialization before redirecting: bundler never // refetches an already-installed gem, so with it in place the scan @@ -1010,8 +1020,18 @@ async fn redirect_scanned_project( String::from_utf8_lossy(&cfg.stderr) ); } - let custom_lockfile = driver == Driver::ScanVexCustomLockfile; - if custom_lockfile { + let custom_lockfile_dsl = driver == Driver::ScanVexCustomLockfileDsl; + if custom_lockfile_dsl { + // The Gemfile's own `lockfile "custom.lock"` call, as a bundler 4 + // project writes it; the default lock stays behind as a leftover. + std::fs::copy(proj.join(lock_name), proj.join("custom.lock")).unwrap(); + let mut gemfile = b"lockfile \"custom.lock\"\n".to_vec(); + gemfile.extend_from_slice(&pristine_gemfile); + std::fs::write(proj.join(gemfile_name), &gemfile).unwrap(); + pristine_gemfile = gemfile; + } + let custom_lockfile = driver == Driver::ScanVexCustomLockfile || custom_lockfile_dsl; + if driver == Driver::ScanVexCustomLockfile { // `bundle config set --local lockfile custom.lock`; the default // lock stays behind as a leftover bundler 4 ignores. std::fs::copy(proj.join(lock_name), proj.join("custom.lock")).unwrap(); @@ -1064,6 +1084,7 @@ async fn redirect_scanned_project( | Driver::ScanVexMirrorSourceEnv | Driver::ScanVexMirrorAllEnv | Driver::ScanVexCustomLockfile + | Driver::ScanVexCustomLockfileDsl | Driver::ScanVexTwin | Driver::ScanVexDualBoot | Driver::ScanVexDualBootEnvGemfile @@ -1287,6 +1308,7 @@ async fn redirect_scanned_project( Driver::ScanVexDualBoot | Driver::ScanVexDualBootEnvGemfile | Driver::ScanVexCustomLockfile + | Driver::ScanVexCustomLockfileDsl | Driver::ScanVexTwin | Driver::ScanVexDuplicateDeclaration | Driver::ScanVexEvalGemfile @@ -2615,6 +2637,27 @@ async fn gem_hosted_bundler4_custom_lockfile_redirects_nothing() { assert!(fx.is_none(), "the custom-lockfile driver asserts in place"); } +/// #1477: bundler 4's Gemfile `lockfile "custom.lock"` call makes bundler +/// read `custom.lock` just like the setting. The hosted scan used to pin +/// the leftover `Gemfile.lock`, rewrite the Gemfile, leave `custom.lock` +/// untouched and report no refusal, so every frozen install then failed; +/// it must refuse, write nothing, and still install frozen. +#[tokio::test(flavor = "multi_thread")] +#[ignore = "host capstone: shells out to a real ruby/gem/bundler (>= 4.0 for this arm); \ + the unpinned `test` job skips it, an e2e job with a pinned toolchain runs it via --ignored"] +async fn gem_hosted_bundler4_gemfile_lockfile_call_redirects_nothing() { + let fx = redirect_scanned_project( + "custom-lockfile-dsl", + Spelling::Gemfile, + true, + true, + None, + Driver::ScanVexCustomLockfileDsl, + ) + .await; + assert!(fx.is_none(), "the custom-lockfile driver asserts in place"); +} + /// #751: bundler 1.x loads a twin's `Gemfile` and bundler >= 2 its /// `gems.rb`. The hosted scan used to wire `gems.rb` and attest while /// bundler 1.17 installed the unpatched gem from the `Gemfile`; since a diff --git a/crates/socket-patch-cli/tests/hosted_memory_engine.rs b/crates/socket-patch-cli/tests/hosted_memory_engine.rs index 39a3fb0cd..a770d98a3 100644 --- a/crates/socket-patch-cli/tests/hosted_memory_engine.rs +++ b/crates/socket-patch-cli/tests/hosted_memory_engine.rs @@ -1101,6 +1101,29 @@ async fn a_bundler4_custom_lockfile_is_refused() { assert_gem_lock_unsupported(&output); } +/// #1477: bundler 4's Gemfile `lockfile "custom.lock"` call names the +/// lock bundler reads just like the setting, so the project is refused the +/// same way, with nothing written. +#[tokio::test] +async fn a_bundler4_gemfile_lockfile_call_is_refused() { + let (server, input) = gem_server_and_input().await; + let mut files = input.clone(); + files.insert("custom.lock".to_string(), input["Gemfile.lock"].clone()); + let mut gemfile = b"lockfile \"custom.lock\"\n".to_vec(); + gemfile.extend_from_slice(&input["Gemfile"]); + files.insert("Gemfile".to_string(), gemfile); + let output = run_engine(&server, build_input(&files, &[], options(false))).await; + let project = &output.projects[0]; + assert!(project.error.is_none(), "{:?}", project.error); + assert!(project.redirected.is_empty(), "{:?}", project.redirected); + assert!( + changed_paths(&output).is_empty(), + "{:?}", + changed_paths(&output) + ); + assert_gem_lock_unsupported(&output); +} + /// The run says the project's gems were not scanned, rather than finding /// none: the lock inventory's `gem_lock_unsupported` diagnosis. fn assert_gem_lock_unsupported(output: &HostedScanOutput) { diff --git a/crates/socket-patch-core/src/crawlers/ruby_crawler.rs b/crates/socket-patch-core/src/crawlers/ruby_crawler.rs index 18d30314e..f2643b0b5 100644 --- a/crates/socket-patch-core/src/crawlers/ruby_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/ruby_crawler.rs @@ -1711,9 +1711,9 @@ fn expand_tilde(value: &Path, home: Option<&Path>) -> PathBuf { value.to_path_buf() } -/// The files [`bundler_loaded_manifest`] reads for `root`: the app config, -/// the global config when there is one, and the `gems.rb` its pair choice -/// probes. +/// The files [`bundler_settings`] reads for `root`: the app config, the +/// global config when there is one, and the `gems.rb` its pair choice +/// probes. The manifest's own `lockfile` call is read through the view. fn bundler_config_files(root: &Path) -> Vec { let app = bundler_app_config_dir(root, std::env::var_os("BUNDLE_APP_CONFIG").as_deref()) .join("config"); @@ -1725,9 +1725,18 @@ fn bundler_config_files(root: &Path) -> Vec { /// [`crate::formats::gem::manifest::classify`] for `root` on disk: the /// manifest bundler loads, reading the ambient `BUNDLE_GEMFILE` / -/// `BUNDLE_LOCKFILE` / `BUNDLE_APP_CONFIG` and the app config file. +/// `BUNDLE_LOCKFILE` / `BUNDLE_APP_CONFIG`, the app config file and the +/// manifest's own `lockfile` call. pub async fn bundler_loaded_manifest(root: &Path) -> crate::formats::gem::manifest::LoadedManifest { - bundler_loaded_manifest_with_env( + bundler_settings(root) + .await + .resolve_in(root, &ProjectView::Disk(root)) + .await +} + +/// [`bundler_settings_with_env`] with the ambient environment. +async fn bundler_settings(root: &Path) -> BundlerSettings { + bundler_settings_with_env( root, BundlerEnv { gemfile: std::env::var_os("BUNDLE_GEMFILE").as_deref(), @@ -1759,7 +1768,8 @@ pub struct BundlerEnv<'a> { /// (or a snapshot of one) reads the ambient environment and the app config /// like bundler; a memory view has no environment, so only its own /// `.bundle/config` counts (its `BUNDLE_GEMFILE` and bundler 4's -/// `BUNDLE_LOCKFILE`). +/// `BUNDLE_LOCKFILE`). Either way the manifest's own `lockfile` call is +/// read through the view. pub(crate) async fn bundler_loaded_manifest_in( view: &ProjectView<'_>, ) -> crate::formats::gem::manifest::LoadedManifest { @@ -1767,34 +1777,120 @@ pub(crate) async fn bundler_loaded_manifest_in( match view { ProjectView::Disk(root) => bundler_loaded_manifest(root).await, 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, and the directory identity of a configured + // The only files the settings probe reads: the app config, the + // global config and `gems.rb` (the environment it also reads is + // fixed 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 + bundler_settings(root).await.resolve_in(root, view).await } ProjectView::Memory(_) => { let config = view.read_text(".bundle/config").await.ok(); let gemfile = config.as_deref().and_then(manifest::config_gemfile); - let lockfile = config.as_deref().and_then(manifest::config_lockfile); + // `/` only stands in for the project root, so paths compare + // lexically. let root = Path::new("/"); - let gems_rb = view.is_file("gems.rb"); - 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 `/`. - if let Some(value) = lockfile.as_deref() { - if Path::new(value).has_root() && loaded.pair(gems_rb).is_some() { - return manifest::LoadedManifest::UnsupportedLockfile { - value: value.to_string(), - by: manifest::GemfileSetting::AppConfig, - }; - } + BundlerSettings { + loaded: manifest::classify_lexical(root, gemfile.as_deref()), + lockfile_env: None, + lockfile_config: config.as_deref().and_then(manifest::config_lockfile), + lockfile_global: None, + gems_rb_present: view.is_file("gems.rb"), + lexical: true, + } + .resolve_in(root, view) + .await + } + } +} + +/// What bundler's settings say about a project, short of the manifest's +/// own `lockfile` call: the manifest it loads and the `BUNDLE_LOCKFILE` +/// tiers. +struct BundlerSettings { + loaded: crate::formats::gem::manifest::LoadedManifest, + lockfile_env: Option, + lockfile_config: Option, + lockfile_global: Option, + gems_rb_present: bool, + /// The root only stands in for a project not on this disk. + lexical: bool, +} + +impl BundlerSettings { + /// The manifest bundler loads, with its lock: bundler 4's `lockfile` + /// call in the loaded manifest (read through `view`) outranks every + /// `BUNDLE_LOCKFILE` tier, as `Dsl#to_definition` does (#1477). + async fn resolve_in( + self, + root: &Path, + view: &ProjectView<'_>, + ) -> crate::formats::gem::manifest::LoadedManifest { + let gems_rb = self.gems_rb_present; + let call = match self.loaded.pair(gems_rb) { + Some((manifest_name, _)) => gemfile_lockfile_call_in(view, manifest_name).await, + None => None, + }; + match (call, self.lexical) { + (Some(call), false) => self.loaded.with_gemfile_lockfile(root, &call, gems_rb), + (Some(call), true) => self + .loaded + .with_gemfile_lockfile_lexical(root, &call, gems_rb), + (None, false) => self.loaded.with_lockfile( + root, + self.lockfile_env.as_deref(), + self.lockfile_config.as_deref(), + self.lockfile_global.as_deref(), + gems_rb, + ), + (None, true) => { + self.loaded + .with_lockfile_lexical(root, self.lockfile_config.as_deref(), gems_rb) + } + } + } +} + +/// The `lockfile` call bundler evaluates for `manifest` (root-relative): +/// the manifest's own, merged with those of the files it `eval_gemfile`s +/// by a literal path inside the project, followed to a bounded depth. +/// Read through `view` and decoded lossily, so a Gemfile in a legacy +/// encoding is still scanned. +async fn gemfile_lockfile_call_in( + view: &ProjectView<'_>, + manifest: &str, +) -> Option { + use crate::formats::gem::manifest::{eval_gemfile_paths, gemfile_lockfile}; + use crate::utils::relpath::{is_anchored, resolve_rel}; + const MAX_FILES: usize = 32; + let mut queue = std::collections::VecDeque::from([manifest.to_string()]); + let mut seen: HashSet = HashSet::new(); + let mut found: Option = None; + while let Some(rel) = queue.pop_front() { + if seen.len() >= MAX_FILES || !seen.insert(rel.clone()) { + continue; + } + let Ok(bytes) = view.read_bytes(&rel).await else { + continue; + }; + let text = String::from_utf8_lossy(&bytes); + if let Some(call) = gemfile_lockfile(&text) { + found = Some(match found.take() { + None => call, + Some(prev) => prev.merge(call), + }); + } + let dir = rel.rsplit_once('/').map_or("", |(dir, _)| dir); + for path in eval_gemfile_paths(&text) { + if is_anchored(&path) { + continue; + } + if let Some(next) = resolve_rel(dir, &path, 0) { + queue.push_back(next); } - loaded.with_lockfile_lexical(root, lockfile.as_deref(), gems_rb) } } + found } /// The ONE lockfile bundler reads for the project `view` shows: the lock of @@ -1848,11 +1944,18 @@ pub async fn bundler_loaded_manifest_with_env( root: &Path, env: BundlerEnv<'_>, ) -> crate::formats::gem::manifest::LoadedManifest { + bundler_settings_with_env(root, env) + .await + .resolve_in(root, &ProjectView::Disk(root)) + .await +} + +/// The [`BundlerSettings`] of `root` under `env`. +async fn bundler_settings_with_env(root: &Path, env: BundlerEnv<'_>) -> BundlerSettings { use crate::formats::gem::manifest; let config = read_app_config(root, env.app_config, env.ignore_config).await; let config = config.as_deref(); let gemfile = config.and_then(manifest::config_gemfile); - let lockfile = config.and_then(manifest::config_lockfile); // Bundler's `File.file?`, which the lock readers' `is_file` mirrors: a // regular file, through symlinks. A directory or a dangling symlink // named `gems.rb` leaves the `Gemfile` pair loaded. @@ -1861,19 +1964,19 @@ pub async fn bundler_loaded_manifest_with_env( .is_ok_and(|m| m.is_file()); let global = read_global_config(env.global_config, env.ignore_config).await; let global = global.as_deref(); - manifest::classify( - root, - env.gemfile, - gemfile.as_deref(), - global.and_then(manifest::config_gemfile).as_deref(), - ) - .with_lockfile( - root, - env.lockfile, - lockfile.as_deref(), - global.and_then(manifest::config_lockfile).as_deref(), + BundlerSettings { + loaded: manifest::classify( + root, + env.gemfile, + gemfile.as_deref(), + global.and_then(manifest::config_gemfile).as_deref(), + ), + lockfile_env: env.lockfile.map(OsStr::to_os_string), + lockfile_config: config.and_then(manifest::config_lockfile), + lockfile_global: global.and_then(manifest::config_lockfile), gems_rb_present, - ) + lexical: false, + } } /// `Bundler.root` when the configured `BUNDLE_GEMFILE` moves it out of @@ -2694,6 +2797,106 @@ mod tests { assert_eq!(m, LoadedManifest::Default); } + /// #1477: bundler 4's `lockfile` call in the loaded manifest outranks + /// `BUNDLE_LOCKFILE` from the environment and the app config, in either + /// direction; it is read from `gems.rb` when that is the manifest. + #[tokio::test] + async fn loaded_manifest_reads_the_gemfile_lockfile_call() { + use crate::formats::gem::manifest::{GemfileSetting, LoadedManifest}; + let dir = tempfile::tempdir().unwrap(); + let gemfile = |text: &str| std::fs::write(dir.path().join("Gemfile"), text).unwrap(); + gemfile("lockfile \"custom.lock\"\ngem \"rack\"\n"); + let m = bundler_loaded_manifest_with_env(dir.path(), BundlerEnv::default()).await; + assert_eq!( + m, + LoadedManifest::UnsupportedLockfile { + value: "custom.lock".into(), + by: GemfileSetting::GemfileCall + } + ); + // The environment naming the default lock does not override it. + let m = bundler_loaded_manifest_with_env( + dir.path(), + BundlerEnv { + lockfile: Some(std::ffi::OsStr::new("Gemfile.lock")), + ..BundlerEnv::default() + }, + ) + .await; + assert!(matches!( + m, + LoadedManifest::UnsupportedLockfile { + by: GemfileSetting::GemfileCall, + .. + } + )); + // A call naming the default lock wins over a custom config value. + gemfile("lockfile \"Gemfile.lock\"\ngem \"rack\"\n"); + std::fs::create_dir(dir.path().join(".bundle")).unwrap(); + std::fs::write( + dir.path().join(".bundle/config"), + "---\nBUNDLE_LOCKFILE: \"custom.lock\"\n", + ) + .unwrap(); + let m = bundler_loaded_manifest_with_env(dir.path(), BundlerEnv::default()).await; + assert_eq!(m, LoadedManifest::Default); + // `lockfile false`: bundler reads no lock at all. + gemfile("lockfile false\n"); + let m = bundler_loaded_manifest_with_env(dir.path(), BundlerEnv::default()).await; + assert!( + matches!(m, LoadedManifest::UnsupportedLockfile { .. }), + "{m:?}" + ); + // The call lives in the manifest bundler loads: gems.rb over Gemfile. + std::fs::remove_file(dir.path().join(".bundle/config")).unwrap(); + gemfile("gem \"rack\"\n"); + std::fs::write(dir.path().join("gems.rb"), "lockfile \"x.lock\"\n").unwrap(); + let m = bundler_loaded_manifest_with_env(dir.path(), BundlerEnv::default()).await; + assert!( + matches!(m, LoadedManifest::UnsupportedLockfile { .. }), + "{m:?}" + ); + std::fs::write(dir.path().join("gems.rb"), "gem \"rack\"\n").unwrap(); + gemfile("lockfile \"x.lock\"\n"); + let m = bundler_loaded_manifest_with_env(dir.path(), BundlerEnv::default()).await; + assert_eq!(m, LoadedManifest::Default); + std::fs::remove_file(dir.path().join("gems.rb")).unwrap(); + // A call in an `eval_gemfile`d file runs in the same builder, + // relative to the evaluating file's directory. + std::fs::create_dir(dir.path().join("gemfiles")).unwrap(); + gemfile("eval_gemfile \"gemfiles/common.rb\"\ngem \"rack\"\n"); + std::fs::write( + dir.path().join("gemfiles/common.rb"), + "eval_gemfile \"lock.rb\"\n", + ) + .unwrap(); + std::fs::write( + dir.path().join("gemfiles/lock.rb"), + "lockfile \"custom.lock\"\n", + ) + .unwrap(); + let m = bundler_loaded_manifest_with_env(dir.path(), BundlerEnv::default()).await; + assert!( + matches!( + m, + LoadedManifest::UnsupportedLockfile { + by: GemfileSetting::GemfileCall, + .. + } + ), + "{m:?}" + ); + // A Gemfile in a legacy encoding is still read. + let mut latin1 = b"# encoding: iso-8859-1\n# caf\xe9\n".to_vec(); + latin1.extend_from_slice(b"lockfile \"custom.lock\"\n"); + std::fs::write(dir.path().join("Gemfile"), latin1).unwrap(); + let m = bundler_loaded_manifest_with_env(dir.path(), BundlerEnv::default()).await; + assert!( + matches!(m, LoadedManifest::UnsupportedLockfile { .. }), + "{m:?}" + ); + } + /// #483: bundler's cache dir is the `cache_path` setting — the app /// config value first, then `BUNDLE_CACHE_PATH`, else `vendor/cache`. #[tokio::test] diff --git a/crates/socket-patch-core/src/formats/gem/gemfile.rs b/crates/socket-patch-core/src/formats/gem/gemfile.rs index 883ccf9c2..8e9f6aba2 100644 --- a/crates/socket-patch-core/src/formats/gem/gemfile.rs +++ b/crates/socket-patch-core/src/formats/gem/gemfile.rs @@ -121,7 +121,7 @@ fn split_args(code: &str) -> Option> { /// A leading quoted literal of `s`: `(content, rest after the closing /// quote)`. -fn leading_quoted(s: &str) -> Option<(&str, &str)> { +pub(crate) fn leading_quoted(s: &str) -> Option<(&str, &str)> { let q = s.chars().next().filter(|c| *c == '"' || *c == '\'')?; let body = &s[1..]; let mut escaped = false; diff --git a/crates/socket-patch-core/src/formats/gem/manifest.rs b/crates/socket-patch-core/src/formats/gem/manifest.rs index 41d8ff1c3..7a992b8f9 100644 --- a/crates/socket-patch-core/src/formats/gem/manifest.rs +++ b/crates/socket-patch-core/src/formats/gem/manifest.rs @@ -67,6 +67,9 @@ pub enum GemfileSetting { AppConfig, /// `BUNDLE_GEMFILE:` in the user's global bundler config file. GlobalConfig, + /// A `lockfile ""` call in the loaded manifest itself (bundler 4's + /// Gemfile DSL). Only ever names a lockfile. + GemfileCall, } impl GemfileSetting { @@ -80,6 +83,7 @@ impl GemfileSetting { GemfileSetting::GlobalConfig => { "BUNDLE_GEMFILE in the global bundler config (~/.bundle/config)" } + GemfileSetting::GemfileCall => "the `lockfile` call in the Gemfile", } } } @@ -137,6 +141,7 @@ impl LoadedManifest { "run `bundle config unset --global gemfile`, or point it at the \ project's Gemfile" } + GemfileSetting::GemfileCall => "remove the `lockfile` call from the Gemfile", }; Some(format!( "bundler loads `{value}` ({}), not the project's Gemfile or gems.rb; \ @@ -159,6 +164,10 @@ impl LoadedManifest { "BUNDLE_LOCKFILE in the global bundler config (~/.bundle/config)", "run `bundle config unset --global lockfile`", ), + GemfileSetting::GemfileCall => ( + "the `lockfile` call in the Gemfile", + "remove the `lockfile` call from the Gemfile", + ), }; Some(format!( "bundler reads the lockfile `{value}` ({knob}), not the Gemfile.lock or \ @@ -200,13 +209,22 @@ impl LoadedManifest { /// [`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. + /// lexically, never through this host's directories, and an absolute + /// value never names the stand-in root's lock (bundler opens it as is). pub(crate) fn with_lockfile_lexical( self, root: &Path, lockfile_config: Option<&str>, gems_rb_present: bool, ) -> LoadedManifest { + if let Some(value) = lockfile_config.filter(|v| Path::new(v).has_root()) { + if self.pair(gems_rb_present).is_some() { + return LoadedManifest::UnsupportedLockfile { + value: value.to_string(), + by: GemfileSetting::AppConfig, + }; + } + } self.with_lockfile_by( root, None, @@ -241,7 +259,85 @@ impl LoadedManifest { if value.as_os_str().is_empty() { return self; } - let target = resolve_against(root, &value); + self.lockfile_against(root, &value, by, lock, same_dir) + } + + /// Layer a `lockfile` call read from the manifest bundler loads + /// ([`gemfile_lockfile`]) onto `self`, in place of + /// [`LoadedManifest::with_lockfile`]: bundler 4's `Dsl#to_definition` + /// uses the call's value over the lockfile `Bundler::CLI` picked from + /// `BUNDLE_LOCKFILE` and the config files, so the call decides alone. A + /// literal path is read against `root` like the settings; one naming the + /// pair's own lock leaves `self` unchanged. Any other call (another + /// lock, `lockfile false`, a computed value) is + /// [`LoadedManifest::UnsupportedLockfile`] (#1477). + pub fn with_gemfile_lockfile( + self, + root: &Path, + call: &GemfileLockfile, + gems_rb_present: bool, + ) -> LoadedManifest { + self.with_gemfile_lockfile_by(root, call, gems_rb_present, same_dir_on_disk) + } + + /// [`LoadedManifest::with_gemfile_lockfile`] for a `root` that only + /// stands in for a project not on this disk, as + /// [`LoadedManifest::with_lockfile_lexical`] compares. + pub(crate) fn with_gemfile_lockfile_lexical( + self, + root: &Path, + call: &GemfileLockfile, + gems_rb_present: bool, + ) -> LoadedManifest { + if let GemfileLockfile::Path(value) = call { + if Path::new(value).has_root() && self.pair(gems_rb_present).is_some() { + return LoadedManifest::UnsupportedLockfile { + value: value.clone(), + by: GemfileSetting::GemfileCall, + }; + } + } + self.with_gemfile_lockfile_by(root, call, gems_rb_present, |a, b| a == b) + } + + fn with_gemfile_lockfile_by( + self, + root: &Path, + call: &GemfileLockfile, + gems_rb_present: bool, + same_dir: SameDir, + ) -> LoadedManifest { + let Some((_, lock)) = self.pair(gems_rb_present) else { + return self; + }; + match call { + GemfileLockfile::Path(value) if !value.is_empty() => self.lockfile_against( + root, + Path::new(value), + GemfileSetting::GemfileCall, + lock, + same_dir, + ), + GemfileLockfile::Path(value) | GemfileLockfile::Other(value) => { + LoadedManifest::UnsupportedLockfile { + value: value.clone(), + by: GemfileSetting::GemfileCall, + } + } + } + } + + /// `self` when `value` (read against `root`) names `lock`, the pair's + /// own lock, else [`LoadedManifest::UnsupportedLockfile`]. + fn lockfile_against( + self, + root: &Path, + value: &Path, + by: GemfileSetting, + lock: &str, + same_dir: SameDir, + ) -> 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, same_dir) { @@ -255,6 +351,248 @@ impl LoadedManifest { } } +/// A `lockfile` call in a Gemfile / `gems.rb` (bundler 4's DSL), as +/// [`gemfile_lockfile`] reads it. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum GemfileLockfile { + /// `lockfile ""`: one plain string literal. + Path(String), + /// Any call whose lock cannot be read statically: `lockfile false` or + /// `lockfile nil` (bundler then reads and writes no lock), a computed + /// or interpolated value, a modifier (`lockfile "x" if …`), a call + /// that is not the first thing on its line, or two calls that + /// disagree. The call's line. + Other(String), +} + +impl GemfileLockfile { + /// The answer for two calls bundler evaluates in turn: one when they + /// agree, else [`GemfileLockfile::Other`] (which one wins depends on + /// control flow a line scan cannot follow). + pub fn merge(self, later: GemfileLockfile) -> GemfileLockfile { + match (self, later) { + (a, b) if a == b => a, + (_, GemfileLockfile::Other(line)) | (GemfileLockfile::Other(line), _) => { + GemfileLockfile::Other(line) + } + (_, GemfileLockfile::Path(value)) => { + GemfileLockfile::Other(format!("lockfile \"{value}\"")) + } + } + } +} + +/// The `lockfile` call of a Gemfile / `gems.rb`, if it makes one. Bundler +/// evaluates the DSL anywhere in the file (inside `group`/`platforms` +/// blocks and conditionals too), so every `lockfile` call in code counts. +/// One that starts its statement with a plain string literal is a +/// [`GemfileLockfile::Path`]; every other call is +/// [`GemfileLockfile::Other`], so callers fail closed rather than pin a +/// lock bundler may not read. Comments, strings, heredoc bodies, +/// `=begin`/`=end` blocks, the `__END__` data section, a longer name, a +/// symbol or method of that name, and a local variable used at the start +/// of a line (`lockfile = …`, `lockfile ||= …`, `lockfile.foo`) are not +/// calls. `eval_gemfile`d files are the caller's to read +/// ([`eval_gemfile_paths`]). +pub fn gemfile_lockfile(contents: &str) -> Option { + let mut found: Option = None; + for_each_code_line(contents, |line, code| { + let lead = code.len() - code.trim_start().len(); + for at in ident_positions(code, "lockfile") { + let this = if at == lead { + match statement_call(&code[at + "lockfile".len()..]) { + Some(this) => this, + None => continue, + } + } else { + GemfileLockfile::Other(line.trim().to_string()) + }; + found = Some(match found.take() { + None => this, + Some(prev) => prev.merge(this), + }); + } + }); + found +} + +/// The literal paths a Gemfile hands to `eval_gemfile`, as written +/// (bundler reads a relative one against the evaluating file's directory). +/// A computed path is left out. +pub fn eval_gemfile_paths(contents: &str) -> Vec { + let mut out = Vec::new(); + for_each_code_line(contents, |_, code| { + for at in ident_positions(code, "eval_gemfile") { + let arg = code[at + "eval_gemfile".len()..].trim_start(); + let arg = arg.strip_prefix('(').map(str::trim_start).unwrap_or(arg); + if let Some((value, _)) = plain_literal(arg) { + out.push(value.to_string()); + } + } + }); + out +} + +/// What follows a `lockfile` that starts its statement: the call it makes, +/// or `None` when the name is a local variable here (assigned, compared, +/// or a method receiver). +fn statement_call(rest: &str) -> Option { + let arg = rest.trim_start(); + let spaced = arg.len() != rest.len(); + let operator = |c: char| "=|&+-*/<>!%?^~.".contains(c); + if arg.starts_with(['.', '=']) || (spaced && arg.starts_with(operator)) { + return None; + } + let call = format!("lockfile{}", rest.trim_end()); + let inner = arg + .strip_prefix('(') + .and_then(|c| c.strip_suffix(')')) + .map(str::trim) + .unwrap_or(arg); + Some(match plain_literal(inner) { + Some((value, after)) if after.trim().is_empty() => GemfileLockfile::Path(value.to_string()), + _ => GemfileLockfile::Other(call), + }) +} + +/// A leading string literal of `s` whose value is its text as written (no +/// escapes, no `#{…}` interpolation in a double-quoted one): `(value, rest +/// after the closing quote)`. +fn plain_literal(s: &str) -> Option<(&str, &str)> { + use crate::formats::gem::gemfile::leading_quoted; + let (value, rest) = leading_quoted(s)?; + let interpolated = s.starts_with('"') && value.contains("#{"); + (!value.contains('\\') && !interpolated).then_some((value, rest)) +} + +/// Run `f(line, code)` over each line of Ruby source that holds code: +/// `code` is the line before any `#` comment. Skips `=begin`/`=end` +/// blocks and heredoc bodies, and stops at `__END__`. A leading BOM is +/// dropped, as Ruby does. +fn for_each_code_line(contents: &str, mut f: impl FnMut(&str, &str)) { + use crate::formats::gem::gemfile::code_of; + let contents = crate::formats::text::strip_bom(contents); + let mut in_comment_block = false; + let mut heredocs: Vec = Vec::new(); + for line in contents.lines() { + if let Some(id) = heredocs.first() { + if line.trim() == id { + heredocs.remove(0); + } + continue; + } + if in_comment_block { + in_comment_block = !line.starts_with("=end"); + continue; + } + if line.starts_with("=begin") { + in_comment_block = true; + continue; + } + if line.trim_end() == "__END__" { + break; + } + let code = code_of(line).unwrap_or(line); + heredocs.extend(heredoc_ids(code)); + f(line, code); + } +} + +/// The byte offsets in `code` where `name` appears as a bare identifier +/// outside strings: not part of a longer name, a symbol (`:name`), an +/// instance/global variable, or a method called on a receiver +/// (`x.name`). +fn ident_positions(code: &str, name: &str) -> Vec { + let bytes = code.as_bytes(); + let ident = |b: u8| b.is_ascii_alphanumeric() || b == b'_'; + let mut out = Vec::new(); + let mut quote: Option = None; + let mut escaped = false; + let mut i = 0; + while i < bytes.len() { + let b = bytes[i]; + if let Some(q) = quote { + if escaped { + escaped = false; + } else if b == b'\\' { + escaped = true; + } else if b == q { + quote = None; + } + i += 1; + continue; + } + if b == b'"' || b == b'\'' { + quote = Some(b); + i += 1; + continue; + } + if ident(b) { + let start = i; + while i < bytes.len() && ident(bytes[i]) { + i += 1; + } + let prev = start.checked_sub(1).map(|p| bytes[p]); + let next = bytes.get(i).copied(); + if &code[start..i] == name + && !matches!(prev, Some(b'.' | b':' | b'@' | b'$')) + && !matches!(next, Some(b'?' | b'!' | b':')) + { + out.push(start); + } + continue; + } + i += 1; + } + out +} + +/// The terminators of the heredocs a line of code opens (`<<~ID`, `<<-ID`, +/// `< Vec { + let mut out = Vec::new(); + let mut quote: Option = None; + let mut escaped = false; + let mut chars = code.char_indices().peekable(); + while let Some((i, c)) = chars.next() { + if let Some(q) = quote { + if escaped { + escaped = false; + } else if c == '\\' { + escaped = true; + } else if c == q { + quote = None; + } + continue; + } + match c { + '"' | '\'' => quote = Some(c), + '<' if code[i..].starts_with("<<") => { + let rest = &code[i + 2..]; + let rest = rest.strip_prefix(['~', '-']).unwrap_or(rest); + let (rest, close) = match rest.chars().next() { + Some(q @ ('"' | '\'' | '`')) => (&rest[1..], Some(q)), + _ => (rest, None), + }; + let len = rest + .find(|c: char| !(c.is_ascii_alphanumeric() || c == '_')) + .unwrap_or(rest.len()); + let id = &rest[..len]; + let starts_ident = id + .chars() + .next() + .is_some_and(|c| c.is_ascii_alphabetic() || c == '_'); + if starts_ident && close.is_none_or(|q| rest[len..].starts_with(q)) { + out.push(id.to_string()); + } + chars.next(); + } + _ => {} + } + } + out +} + /// The `BUNDLE_LOCKFILE:` value of a bundler config file (bundler 4's /// `bundle config set lockfile `). An empty value is still returned: /// it shadows the tiers below it ([`LoadedManifest::with_lockfile`]). @@ -398,6 +736,175 @@ fn classify_by( mod tests { use super::*; + /// #1477: bundler 4's Gemfile `lockfile` call, in every spelling a + /// Gemfile uses; anything not a plain literal fails closed. + #[test] + fn gemfile_lockfile_reads_the_dsl_call() { + let path = |v: &str| Some(GemfileLockfile::Path(v.into())); + let other = |v: &str| Some(GemfileLockfile::Other(v.into())); + assert_eq!( + gemfile_lockfile("source \"https://rubygems.org\"\ngem \"rack\"\n"), + None + ); + assert_eq!( + gemfile_lockfile("lockfile \"custom.lock\"\nsource \"https://rubygems.org\"\n"), + path("custom.lock") + ); + assert_eq!( + gemfile_lockfile("lockfile 'custom.lock' # why\n"), + path("custom.lock") + ); + assert_eq!( + gemfile_lockfile("lockfile(\"custom.lock\")\r\n"), + path("custom.lock") + ); + assert_eq!( + gemfile_lockfile("lockfile( 'a/b.lock' )\n"), + path("a/b.lock") + ); + assert_eq!( + gemfile_lockfile("group :dev do\n lockfile \"x.lock\"\nend\n"), + path("x.lock") + ); + // Two identical calls are one answer; disagreeing ones are not. + assert_eq!( + gemfile_lockfile("lockfile \"a\"\nlockfile \"a\"\n"), + path("a") + ); + assert_eq!( + gemfile_lockfile("lockfile \"a\"\nlockfile \"b\"\n"), + other("lockfile \"b\"") + ); + // Bundler reads and writes no lock, or the value is not static. + assert_eq!( + gemfile_lockfile("lockfile false\n"), + other("lockfile false") + ); + assert_eq!( + gemfile_lockfile("lockfile ENV.fetch(\"L\")\n"), + other("lockfile ENV.fetch(\"L\")") + ); + assert_eq!( + gemfile_lockfile("lockfile \"#{x}.lock\"\n"), + other("lockfile \"#{x}.lock\"") + ); + assert_eq!( + gemfile_lockfile("lockfile \"x.lock\" if ENV[\"CI\"]\n"), + other("lockfile \"x.lock\" if ENV[\"CI\"]") + ); + // `lockfile nil` replaces the CLI's lock with none at all (bundler + // 4's `Dsl.evaluate` seeds `@lockfile` with it first). + assert_eq!(gemfile_lockfile("lockfile nil\n"), other("lockfile nil")); + // A call later in a line, which a statement read cannot pin down. + for text in [ + "group(:ci) { lockfile \"ci.lock\" }\n", + "source \"https://rubygems.org\"; lockfile \"custom.lock\"\n", + "ENV[\"CI\"] && lockfile(\"ci.lock\")\n", + "if ENV[\"CI\"] then lockfile \"a\" end\n", + ] { + assert!( + matches!(gemfile_lockfile(text), Some(GemfileLockfile::Other(_))), + "{text:?}" + ); + } + // Ruby skips a leading BOM. + assert_eq!( + gemfile_lockfile("\u{feff}lockfile \"custom.lock\"\n"), + path("custom.lock") + ); + // Not calls: comments, strings, heredoc bodies, the data section, a + // variable (assigned, compared, a receiver), a symbol or method of + // that name, a longer name. + for text in [ + "# lockfile \"x\"\n", + "=begin\nlockfile \"x\"\n=end\n", + "lockfile = \"x\"\n", + "lockfile=\"x\"\n", + "lockfile ||= \"x\"\n", + "lockfile += \"x\"\n", + "lockfile == other\n", + "lockfile.freeze\n", + "lockfiles \"x\"\n", + "gem \"lockfile\"\n", + "gem \"x\", require: \"lockfile\" # lockfile \"y\"\n", + "x = :lockfile\n", + "Bundler.lockfile\n", + "puts <<~TXT\n lockfile \"x\"\nTXT\n", + "puts <<-'TXT'\nlockfile \"x\"\n TXT\n", + "gem \"rack\"\n__END__\nlockfile \"x\"\n", + ] { + assert_eq!(gemfile_lockfile(text), None, "{text:?}"); + } + // Code after a heredoc closes is read again. + assert_eq!( + gemfile_lockfile("puts <<~TXT\n hi\nTXT\nlockfile \"x\"\n"), + path("x") + ); + } + + #[test] + fn eval_gemfile_paths_reads_literal_paths() { + assert_eq!( + eval_gemfile_paths( + "eval_gemfile \"Gemfile.common\"\neval_gemfile('a/b.rb')\n\ + eval_gemfile File.join(x)\n# eval_gemfile \"c\"\n" + ), + vec!["Gemfile.common".to_string(), "a/b.rb".to_string()] + ); + } + + /// #1477: the Gemfile call outranks the `BUNDLE_LOCKFILE` tiers, as + /// bundler 4's `Dsl#to_definition` does; naming the pair's own lock is a + /// no-op and anything else is refused with the call's own remedy. + #[test] + fn with_gemfile_lockfile_accepts_only_the_pairs_own_lock() { + let call = |v: &str| GemfileLockfile::Path(v.into()); + let m = LoadedManifest::Default.with_gemfile_lockfile(&root(), &call("custom.lock"), false); + assert_eq!( + m, + LoadedManifest::UnsupportedLockfile { + value: "custom.lock".into(), + by: GemfileSetting::GemfileCall, + } + ); + assert_eq!(m.pair(false), None); + let detail = m.unsupported_detail().unwrap(); + assert!(detail.contains("custom.lock"), "{detail}"); + assert!(detail.contains("remove the `lockfile` call"), "{detail}"); + for own in ["Gemfile.lock", "./Gemfile.lock", "/proj/Gemfile.lock"] { + assert_eq!( + LoadedManifest::Default.with_gemfile_lockfile(&root(), &call(own), false), + LoadedManifest::Default, + "{own}" + ); + } + // With a gems.rb the pair's lock is gems.locked. + assert!(matches!( + LoadedManifest::Default.with_gemfile_lockfile(&root(), &call("Gemfile.lock"), true), + LoadedManifest::UnsupportedLockfile { .. } + )); + assert_eq!( + LoadedManifest::Default.with_gemfile_lockfile(&root(), &call("gems.locked"), true), + LoadedManifest::Default + ); + let off = GemfileLockfile::Other("lockfile false".into()); + assert_eq!( + LoadedManifest::Default.with_gemfile_lockfile(&root(), &off, false), + LoadedManifest::UnsupportedLockfile { + value: "lockfile false".into(), + by: GemfileSetting::GemfileCall, + } + ); + // An unsupported manifest stays the answer. + let unsupported = classify(&root(), None, Some("Gemfile.next"), None); + assert_eq!( + unsupported + .clone() + .with_gemfile_lockfile(&root(), &call("custom.lock"), false), + unsupported + ); + } + fn root() -> PathBuf { std::path::absolute("/proj").unwrap() } diff --git a/crates/socket-patch-core/src/vendor/gem.rs b/crates/socket-patch-core/src/vendor/gem.rs index 6b159b4bd..51bd83bf2 100644 --- a/crates/socket-patch-core/src/vendor/gem.rs +++ b/crates/socket-patch-core/src/vendor/gem.rs @@ -3021,6 +3021,40 @@ mod tests { assert!(!root.join(".socket/vendor").exists()); } + /// #1477: bundler 4's Gemfile `lockfile "custom.lock"` call makes + /// bundler read `custom.lock` just like the setting does, so it is + /// refused the same way, before any write. + #[tokio::test] + async fn a_bundler4_gemfile_lockfile_call_is_refused() { + let gemfile = format!("lockfile \"custom.lock\"\n{GEMFILE_DIRECT}"); + let (_tmp, root, installed, blobs, record) = fixture(&gemfile, LOCK_DIRECT).await; + tokio::fs::write(root.join("custom.lock"), LOCK_DIRECT) + .await + .unwrap(); + + let (code, detail) = + unwrap_refused(run_vendor(&root, &blobs, &installed, &record, false).await); + assert_eq!(code, "gemfile_not_loaded"); + assert!(detail.contains("custom.lock"), "{detail}"); + assert!( + detail.contains("remove the `lockfile` call from the Gemfile"), + "{detail}" + ); + assert_eq!( + tokio::fs::read_to_string(root.join("Gemfile")) + .await + .unwrap(), + gemfile + ); + assert_eq!( + tokio::fs::read_to_string(root.join(GEMFILE_LOCK)) + .await + .unwrap(), + LOCK_DIRECT + ); + assert!(!root.join(".socket/vendor").exists()); + } + /// `BUNDLE_GEMFILE` naming the project's own Gemfile beside a `gems.rb` /// makes bundler load the Gemfile, so vendoring wires it as usual. #[tokio::test]