From a7cb889cd56e2228cf656db5914841d5b649a8b8 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 20:23:58 +0000 Subject: [PATCH 1/5] Start fix for #985 Assisted-by: Claude Code:claude-opus-5-5 From df4f3a54e130892e65dec32cf066704a5928c002 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 20:50:12 +0000 Subject: [PATCH 2/5] Pin gemspec-merged gem requirements in locks On Bundler 2.7+, a Gemfile `gemspec` directive folds the gemspec's development dependency on a gem into a later `gem` line for the same gem, and the lock records the union, e.g. `vuln-gem (>= 0, = 1.0.0)!`. Both the hosted and the vendored lock writers wrote the bare `(= 1.0.0)!`, so every frozen or deployment install of the committed pair failed (exit 16) while the scan reported success. Both writers now read the gemspec the directive loads and pin the requirement Bundler derives: the gemspec's requirements plus the exact version, sorted the way Bundler writes them. Bundler < 2.7 and a `gem` line before the directive keep the exact pin, as Bundler does. When the gemspec cannot be read, hosted mode keeps the exact pin and warns (`redirect_gem_gemspec_dependency_unreadable`); vendored mode refuses before writing anything. Fixes #985 Assisted-by: Claude Code:claude-opus-5-5 --- .../src/formats/gem/gemfile.rs | 2 +- .../src/formats/gem/gemspec.rs | 579 ++++++++++++++++++ .../src/formats/gem/hosted.rs | 12 +- .../socket-patch-core/src/formats/gem/mod.rs | 1 + crates/socket-patch-core/src/hosted/engine.rs | 59 ++ .../src/patch/redirect/mod.rs | 112 ++++ crates/socket-patch-core/src/vendor/gem.rs | 165 ++++- 7 files changed, 909 insertions(+), 21 deletions(-) create mode 100644 crates/socket-patch-core/src/formats/gem/gemspec.rs diff --git a/crates/socket-patch-core/src/formats/gem/gemfile.rs b/crates/socket-patch-core/src/formats/gem/gemfile.rs index 8e9f6aba2..bf8ce17b1 100644 --- a/crates/socket-patch-core/src/formats/gem/gemfile.rs +++ b/crates/socket-patch-core/src/formats/gem/gemfile.rs @@ -84,7 +84,7 @@ pub(crate) fn code_of(tail: &str) -> Option<&str> { /// Split `code` at its top-level commas (outside strings and brackets). /// Each piece keeps its byte offset into `code`. `None` on unbalanced /// brackets. -fn split_args(code: &str) -> Option> { +pub(crate) fn split_args(code: &str) -> Option> { let mut out = Vec::new(); let mut quote: Option = None; let mut escaped = false; diff --git a/crates/socket-patch-core/src/formats/gem/gemspec.rs b/crates/socket-patch-core/src/formats/gem/gemspec.rs new file mode 100644 index 000000000..29d16c692 --- /dev/null +++ b/crates/socket-patch-core/src/formats/gem/gemspec.rs @@ -0,0 +1,579 @@ +//! The `DEPENDENCIES` requirement bundler locks for a gem a Gemfile pins +//! exactly, once the Gemfile's `gemspec` directive is taken into account. +//! +//! `gemspec` adds the gemspec's development dependencies to the Gemfile. +//! Bundler 2.7+ merges such a development dependency with a `gem` line for +//! the same gem that FOLLOWS the directive (`Dsl#add_dependency`): the +//! Gemfile dependency carries both requirement lists, and the lock writes +//! the union, sorted the way `Gem::Dependency#to_lock` sorts it +//! (`vuln-gem (>= 0, = 1.0.0)!`). A `gem` line BEFORE the directive keeps +//! its own requirement (the later development dependency is dropped), and +//! bundler < 2.7 never merges. Both pin writers (hosted +//! [`super::hosted::converge_gem_lock_source`], vendored `edit_lock`) lock +//! what bundler derives here, or every frozen install of the pair fails +//! (#985). +//! +//! The gemspec is Ruby; only literal `add_development_dependency` calls +//! are read. A call naming the gem that cannot be read, or a directive +//! whose gemspec cannot be found or read, is reported as an error: the +//! caller cannot know what bundler would lock. + +use super::gemfile::{code_of, leading_quoted, split_args}; + +/// The `gemspec` files of one directory: `(file name, text)`, the text +/// `None` when the file exists but could not be read. `Err` when the +/// directory itself could not be listed. +pub(crate) type GemspecListing = Result)>, String>; + +/// The synthetic candidate key the hosted planner reads the `gemspec` +/// directives' files from (`hosted::engine::read_candidate_files`): a JSON +/// object mapping each directive's root-relative directory to either its +/// `*.gemspec` files (`{"name.gemspec": text | null}`, `null` when +/// unreadable) or why it could not be listed. Never a path. +pub(crate) const GEMSPECS_KEY: &str = ""; + +/// [`GemspecListing`] of `dir` from the candidate files' [`GEMSPECS_KEY`]. +pub(crate) fn candidate_listing( + files: &std::collections::BTreeMap, + dir: &str, +) -> GemspecListing { + let raw = files + .get(GEMSPECS_KEY) + .ok_or_else(|| "the project's gemspecs were not read".to_string())?; + let listing: serde_json::Value = serde_json::from_str(raw).map_err(|e| e.to_string())?; + match listing.get(dir) { + Some(serde_json::Value::Object(entries)) => Ok(entries + .iter() + .map(|(file, text)| (file.clone(), text.as_str().map(str::to_string))) + .collect()), + Some(serde_json::Value::String(why)) => Err(why.clone()), + _ => Err("the directory was not listed".to_string()), + } +} + +/// Whether a directory entry is a gemspec bundler's `gemspec` directive +/// finds (`{,*}.gemspec`: a dotfile only when it is `.gemspec` itself). +pub(crate) fn is_gemspec_file(name: &str) -> bool { + name == ".gemspec" || (!name.starts_with('.') && name.ends_with(".gemspec")) +} + +/// Whether the bundler that wrote `lock` (its `BUNDLED WITH` version) merges +/// a `gemspec` development dependency into a later `gem` line: 2.7+. A lock +/// without the section (bundler < 1.10) or with an unreadable version does +/// not. +pub(crate) fn bundler_merges_dev_dependencies(lock: &str) -> bool { + let mut lines = lock.lines().map(|l| l.trim_end_matches('\r')); + let Some(version) = lines + .by_ref() + .find(|l| *l == "BUNDLED WITH") + .and_then(|_| lines.next()) + .map(str::trim) + else { + return false; + }; + let mut parts = version.split('.').map(|p| p.parse::()); + match (parts.next(), parts.next()) { + (Some(Ok(major)), Some(Ok(minor))) => (major, minor) >= (2, 7), + _ => false, + } +} + +/// One `gemspec` directive of a Gemfile. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct GemspecDirective { + /// 0-based line index. + pub(crate) line: usize, + /// The `path:` option, root-relative with `/` separators (`""` for the + /// Gemfile's own directory). + pub(crate) dir: String, + /// The `name:` option. + pub(crate) name: Option, +} + +/// The `gemspec` directives of `gemfile`, in file order. `Err` names the +/// first one whose options cannot be read statically (a non-literal +/// `path:` / `name:`, a `glob:`, an absolute or escaping path). +pub(crate) fn gemspec_directives(gemfile: &str) -> Result, String> { + let mut out = Vec::new(); + for (line, text) in gemfile.split('\n').enumerate() { + let trimmed = text.trim_start(); + let Some(rest) = trimmed.strip_prefix("gemspec") else { + continue; + }; + // `gemspec`, `gemspec path: "x"`, `gemspec(path: "x")`; never + // `gemspec_foo` or `gemspecs`. + if rest + .chars() + .next() + .is_some_and(|c| c.is_ascii_alphanumeric() || c == '_') + { + continue; + } + let unreadable = || format!("line {}: `{}`", line + 1, text.trim()); + let code = code_of(rest).ok_or_else(unreadable)?.trim(); + let code = code.trim_end_matches(';').trim_end(); + let code = match code.strip_prefix('(') { + Some(inner) => inner.strip_suffix(')').ok_or_else(unreadable)?, + None => code, + }; + let mut directive = GemspecDirective { + line, + dir: String::new(), + name: None, + }; + if !code.trim().is_empty() { + for (_, arg) in split_args(code).ok_or_else(unreadable)? { + let (key, value) = option(arg).ok_or_else(unreadable)?; + match key { + "path" => { + let lit = literal(value).ok_or_else(unreadable)?; + directive.dir = relative_dir(&lit).ok_or_else(unreadable)?; + } + "name" => directive.name = Some(literal(value).ok_or_else(unreadable)?), + // Only picks the group the dependencies join. + "development_group" => {} + // `glob:` changes which gemspec files bundler finds. + _ => return Err(unreadable()), + } + } + } + out.push(directive); + } + Ok(out) +} + +/// `key: value` / `:key => value` / `"key" => value`: `(key, value text)`. +fn option(arg: &str) -> Option<(&str, &str)> { + let a = arg.trim(); + if let Some((key, rest)) = leading_quoted(a) { + let value = rest.trim_start().strip_prefix("=>")?; + return Some((key, value)); + } + if let Some(sym) = a.strip_prefix(':') { + let (key, value) = sym.split_once("=>")?; + return Some((key.trim(), value)); + } + let (key, value) = a.split_once(':')?; + let key = key.trim(); + (!key.is_empty() && key.bytes().all(|b| b.is_ascii_alphanumeric() || b == b'_')) + .then_some((key, value)) +} + +/// A Ruby string literal (`"x"`, `'x'`, `%q`, each optionally +/// `.freeze`d) with no escapes or interpolation: its content. +fn literal(value: &str) -> Option { + let v = value.trim(); + let v = v.strip_suffix(".freeze").unwrap_or(v).trim_end(); + let content = if let Some((content, rest)) = leading_quoted(v) { + if !rest.trim().is_empty() || (v.starts_with('"') && content.contains("#{")) { + return None; + } + content + } else { + let body = v.strip_prefix("%q").or_else(|| v.strip_prefix("%Q"))?; + let open = body.chars().next()?; + let close = match open { + '<' => '>', + '{' => '}', + '(' => ')', + '[' => ']', + _ => return None, + }; + let inner = body[1..].strip_suffix(close)?; + if inner.contains([open, close]) || inner.contains("#{") { + return None; + } + inner + }; + (!content.contains('\\')).then(|| content.to_string()) +} + +/// `path` as a normalized root-relative directory: `""` for `.`, `None` +/// for an absolute path or one that leaves the root. +fn relative_dir(path: &str) -> Option { + if path.starts_with('/') || path.starts_with('~') || path.contains('\\') { + return None; + } + let mut parts: Vec<&str> = Vec::new(); + for part in path.split('/') { + match part { + "" | "." => {} + ".." => { + parts.pop()?; + } + p => parts.push(p), + } + } + Some(parts.join("/")) +} + +/// A requirement in the spelling `Gem::Requirement#as_list` gives it: +/// `op version`, the operator defaulting to `=`. `None` when it is not a +/// requirement rubygems parses. +fn normalize_requirement(req: &str) -> Option { + let r = req.trim(); + let (op, version) = ["~>", ">=", "<=", "!=", "=", ">", "<"] + .iter() + .find_map(|op| r.strip_prefix(op).map(|v| (*op, v.trim()))) + .unwrap_or(("=", r)); + let valid = !version.is_empty() + && version.starts_with(|c: char| c.is_ascii_digit()) + && version + .chars() + .all(|c| c.is_ascii_alphanumeric() || c == '.' || c == '-'); + valid.then(|| format!("{op} {version}")) +} + +/// The requirements the gemspec's `add_development_dependency` call on +/// `gem` declares, as `Gem::Requirement#as_list` spells them (`[">= 0"]` +/// for none). `Ok(None)` when no such call names it. `Err` when a call +/// names it but cannot be read (non-literal arguments, a call spanning +/// lines), or when several calls disagree. +pub(crate) fn dev_dependency_requirements( + gemspec: &str, + gem: &str, +) -> Result>, String> { + let mut found: Option> = None; + for (i, text) in gemspec.split('\n').enumerate() { + let trimmed = text.trim(); + if trimmed.starts_with('#') { + continue; + } + let Some(at) = trimmed.find("add_development_dependency") else { + continue; + }; + let after = &trimmed[at + "add_development_dependency".len()..]; + let mentions = after.contains(&format!("\"{gem}\"")) + || after.contains(&format!("'{gem}'")) + || after.contains(&format!("<{gem}>")) + || after.contains(&format!("{{{gem}}}")); + let unreadable = || format!("line {}: `{trimmed}`", i + 1); + let Some(reqs) = read_dev_call(after) else { + if mentions { + return Err(unreadable()); + } + continue; + }; + let (name, reqs) = reqs; + if name != gem { + continue; + } + let reqs = if reqs.is_empty() { + vec![">= 0".to_string()] + } else { + reqs + }; + match &found { + Some(prev) if *prev != reqs => return Err(unreadable()), + _ => found = Some(reqs), + } + } + Ok(found) +} + +/// The arguments of one `add_development_dependency` call written on one +/// line (`"x", "~> 1"`, `("x", [">= 1", "< 2"])`, `(%q.freeze, [">= 0"])`): +/// `(gem name, normalized requirements)`. +fn read_dev_call(after: &str) -> Option<(String, Vec)> { + let code = code_of(after)?.trim(); + let code = code.trim_end_matches(';').trim_end(); + let code = match code.strip_prefix('(') { + Some(inner) => inner.strip_suffix(')')?, + None => code, + }; + let args = split_args(code)?; + let mut args = args.into_iter().map(|(_, a)| a.trim()); + let name = literal(args.next()?)?; + let mut reqs = Vec::new(); + for arg in args { + if arg.is_empty() { + // A trailing comma: the call continues on the next line. + return None; + } + match arg.strip_prefix('[') { + Some(list) => { + let list = list.strip_suffix(']')?; + for (_, item) in split_args(list)? { + if item.trim().is_empty() { + continue; + } + reqs.push(normalize_requirement(&literal(item)?)?); + } + } + None => reqs.push(normalize_requirement(&literal(arg)?)?), + } + } + Some((name, reqs)) +} + +/// The requirement list bundler locks, inside the parentheses, for `name` +/// once the Gemfile (as the pin writer leaves it) declares it pinned to +/// exactly `version`: `= 1.0.0`, or `>= 0, = 1.0.0` when bundler 2.7+ +/// merges a gemspec development dependency into it. `gemspecs` lists a +/// root-relative directory's `*.gemspec` files. `Err` says why the merge +/// cannot be decided (the caller cannot know what bundler would lock). +pub(crate) fn locked_exact_requirement( + gemfile: &str, + lock: &str, + name: &str, + version: &str, + gemspecs: &mut dyn FnMut(&str) -> GemspecListing, +) -> Result { + let exact = format!("= {version}"); + if !bundler_merges_dev_dependencies(lock) { + return Ok(exact); + } + let Some(decl) = gemfile.split('\n').position(|line| { + let trimmed = line.trim_start(); + !trimmed.starts_with('#') + && crate::vendor::gem::gem_declaration_any(trimmed).is_some_and(|d| d.name == name) + }) else { + return Ok(exact); + }; + let directives = gemspec_directives(gemfile) + .map_err(|why| format!("cannot read the Gemfile's `gemspec` directive ({why})"))?; + let mut merged: Option> = None; + for directive in directives.iter().filter(|d| d.line < decl) { + let shown = if directive.dir.is_empty() { + ".".to_string() + } else { + directive.dir.clone() + }; + let files = gemspecs(&directive.dir) + .map_err(|why| format!("cannot list the gemspecs in {shown} ({why})"))?; + let file = match &directive.name { + Some(n) => { + let want = format!("{n}.gemspec"); + files.iter().find(|(f, _)| *f == want) + } + None if files.len() == 1 => files.first(), + None => None, + } + .ok_or_else(|| { + format!( + "the Gemfile's `gemspec` directive does not name exactly one gemspec in {shown}" + ) + })?; + let text = file + .1 + .as_deref() + .ok_or_else(|| format!("cannot read {shown}/{}", file.0))?; + let reqs = dev_dependency_requirements(text, name).map_err(|why| { + format!( + "cannot read the development dependency on {name} in {shown}/{} ({why})", + file.0 + ) + })?; + if let Some(reqs) = reqs { + if merged.is_some() { + return Err(format!( + "more than one gemspec declares a development dependency on {name}" + )); + } + merged = Some(reqs); + } + } + let Some(mut reqs) = merged else { + return Ok(exact); + }; + // `Dependency.new(name, gemspec.as_list + gemfile.as_list)`: duplicates + // dropped, then `to_lock` sorts the spellings and reverses them. + reqs.push(exact); + reqs.sort(); + reqs.dedup(); + reqs.reverse(); + Ok(reqs.join(", ")) +} + +#[cfg(test)] +mod tests { + use super::*; + + const LOCK_27: &str = "DEPENDENCIES\n mylib!\n vuln-gem\n\nBUNDLED WITH\n 2.7.2\n"; + + fn one(text: &'static str) -> impl FnMut(&str) -> GemspecListing { + move |dir: &str| { + assert_eq!(dir, ""); + Ok(vec![("mylib.gemspec".to_string(), Some(text.to_string()))]) + } + } + + const GEMSPEC: &str = "Gem::Specification.new do |s|\n s.name = \"mylib\"\n \ + s.add_development_dependency \"vuln-gem\"\n \ + s.add_development_dependency \"rack\", \">= 3\"\nend\n"; + + #[test] + fn bundler_2_7_and_later_merge() { + for (version, want) in [ + ("2.6.9", false), + ("2.7.0", true), + ("2.7.2", true), + ("4.0.22", true), + ("1.17.3", false), + ("garbage", false), + ] { + let lock = format!("DEPENDENCIES\n rails\n\nBUNDLED WITH\n {version}\n"); + assert_eq!(bundler_merges_dev_dependencies(&lock), want, "{version}"); + assert_eq!( + bundler_merges_dev_dependencies(&lock.replace('\n', "\r\n")), + want, + "{version} CRLF" + ); + } + assert!(!bundler_merges_dev_dependencies("DEPENDENCIES\n rails\n")); + } + + /// #985: a gemspec development dependency merges into a later `gem` + /// line, sorted as bundler's `to_lock` writes it. + #[test] + fn merges_a_dev_dependency_declared_before_the_gem_line() { + let gemfile = "source \"https://rubygems.org\"\n\ngemspec\n\ngroup :development do\n \ + source \"https://patch.example/x/\" do\n gem \"vuln-gem\", \"1.0.0\"\n \ + end\nend\n"; + let got = + locked_exact_requirement(gemfile, LOCK_27, "vuln-gem", "1.0.0", &mut one(GEMSPEC)); + assert_eq!(got.as_deref(), Ok(">= 0, = 1.0.0")); + let gemfile = + "source \"https://rubygems.org\"\ngemspec\ngem \"rack\", \"3.2.7\", path: \"v\"\n"; + let got = locked_exact_requirement(gemfile, LOCK_27, "rack", "3.2.7", &mut one(GEMSPEC)); + assert_eq!(got.as_deref(), Ok(">= 3, = 3.2.7")); + } + + #[test] + fn keeps_the_exact_pin_when_bundler_does_not_merge() { + let mut never = |_: &str| -> GemspecListing { panic!("not listed") }; + // Bundler < 2.7. + let gemfile = "gemspec\ngem \"vuln-gem\", \"1.0.0\"\n"; + let lock = LOCK_27.replace("2.7.2", "2.6.9"); + assert_eq!( + locked_exact_requirement(gemfile, &lock, "vuln-gem", "1.0.0", &mut never).as_deref(), + Ok("= 1.0.0") + ); + // The `gem` line comes first: bundler keeps it unmerged. + let gemfile = "gem \"vuln-gem\", \"1.0.0\"\ngemspec\n"; + assert_eq!( + locked_exact_requirement(gemfile, LOCK_27, "vuln-gem", "1.0.0", &mut never).as_deref(), + Ok("= 1.0.0") + ); + // No `gemspec` directive at all. + let gemfile = "gem \"vuln-gem\", \"1.0.0\"\n# gemspec\n"; + assert_eq!( + locked_exact_requirement(gemfile, LOCK_27, "vuln-gem", "1.0.0", &mut never).as_deref(), + Ok("= 1.0.0") + ); + // The gemspec has no development dependency on it (a runtime one is + // never merged). + let gemfile = "gemspec\ngem \"puma\", \"6.0.0\"\n"; + let runtime = "Gem::Specification.new do |s|\n s.add_dependency \"puma\"\nend\n"; + assert_eq!( + locked_exact_requirement(gemfile, LOCK_27, "puma", "6.0.0", &mut one(runtime)) + .as_deref(), + Ok("= 6.0.0") + ); + } + + #[test] + fn a_dev_dependency_pinned_to_the_same_version_is_not_repeated() { + let gemfile = "gemspec\ngem \"vuln-gem\", \"1.0.0\"\n"; + let spec = "s.add_development_dependency(\"vuln-gem\", \"1.0.0\")\n"; + assert_eq!( + locked_exact_requirement(gemfile, LOCK_27, "vuln-gem", "1.0.0", &mut one(spec)) + .as_deref(), + Ok("= 1.0.0") + ); + } + + #[test] + fn reads_the_directive_options() { + let gemfile = "gemspec path: \"gems/core\", name: \"core\"\ngem \"vuln-gem\", \"1.0.0\"\n"; + let mut listing = |dir: &str| -> GemspecListing { + assert_eq!(dir, "gems/core"); + Ok(vec![ + ("other.gemspec".to_string(), Some(String::new())), + ( + "core.gemspec".to_string(), + Some("spec.add_development_dependency 'vuln-gem', '~> 1.0', '< 2'\n".into()), + ), + ]) + }; + assert_eq!( + locked_exact_requirement(gemfile, LOCK_27, "vuln-gem", "1.0.0", &mut listing) + .as_deref(), + Ok("~> 1.0, = 1.0.0, < 2") + ); + assert_eq!( + gemspec_directives( + "gemspec(:path => './x/../y', \"name\" => 'n', development_group: :dev)\n" + ), + Ok(vec![GemspecDirective { + line: 0, + dir: "y".into(), + name: Some("n".into()) + }]) + ); + assert_eq!(gemspec_directives("gemspec_helper\ngemspecs\n"), Ok(vec![])); + for bad in [ + "gemspec path: ROOT\n", + "gemspec path: \"../up\"\n", + "gemspec path: \"/abs\"\n", + "gemspec glob: \"*.spec\"\n", + "gemspec path: \"#{x}\"\n", + ] { + assert!(gemspec_directives(bad).is_err(), "{bad}"); + } + } + + #[test] + fn reads_generated_and_bracketed_dev_dependencies() { + let spec = "s.add_development_dependency(%q.freeze, [\">= 0.5\", \"< 2\"])\n"; + assert_eq!( + dev_dependency_requirements(spec, "vuln-gem"), + Ok(Some(vec![">= 0.5".into(), "< 2".into()])) + ); + // `if respond_to?` branches repeat the same call. + let spec = "if x\n s.add_development_dependency(%q, [\"1.0\"])\nelse\n \ + s.add_development_dependency(%q, [\"= 1.0\"])\nend\n"; + assert_eq!( + dev_dependency_requirements(spec, "v"), + Ok(Some(vec!["= 1.0".into()])) + ); + assert_eq!(dev_dependency_requirements(spec, "other"), Ok(None)); + // Commented out. + assert_eq!( + dev_dependency_requirements("# s.add_development_dependency \"v\", \"x\"\n", "v"), + Ok(None) + ); + } + + #[test] + fn an_unreadable_dev_dependency_or_gemspec_is_an_error() { + let gemfile = "gemspec\ngem \"vuln-gem\", \"1.0.0\"\n"; + for spec in [ + "s.add_development_dependency \"vuln-gem\", VERSION\n", + "s.add_development_dependency \"vuln-gem\",\n \"~> 1.0\"\n", + "s.add_development_dependency \"vuln-gem\", \"~> 1\"\ns.add_development_dependency \"vuln-gem\", \"~> 2\"\n", + ] { + let text: &'static str = Box::leak(spec.to_string().into_boxed_str()); + assert!( + locked_exact_requirement(gemfile, LOCK_27, "vuln-gem", "1.0.0", &mut one(text)) + .is_err(), + "{spec}" + ); + } + for listing in [ + Ok(vec![]), + Ok(vec![ + ("a.gemspec".to_string(), Some(String::new())), + ("b.gemspec".to_string(), Some(String::new())), + ]), + Ok(vec![("a.gemspec".to_string(), None)]), + Err("denied".to_string()), + ] { + let mut f = |_: &str| listing.clone(); + assert!( + locked_exact_requirement(gemfile, LOCK_27, "vuln-gem", "1.0.0", &mut f).is_err(), + "{listing:?}" + ); + } + } +} diff --git a/crates/socket-patch-core/src/formats/gem/hosted.rs b/crates/socket-patch-core/src/formats/gem/hosted.rs index 224d69c1e..e64ada032 100644 --- a/crates/socket-patch-core/src/formats/gem/hosted.rs +++ b/crates/socket-patch-core/src/formats/gem/hosted.rs @@ -70,9 +70,11 @@ fn place_gem_section_sorted<'a>( /// from the redirected Gemfile (verified frozen-installable on bundler 4): /// the dep's spec entry (+ its dependency sublines) moves out of the /// upstream `GEM` section into a patch-registry `GEM` section -/// (`remote: `), and DEPENDENCIES pins ` (= )!` -/// (bundler's source-pin spelling for a block-scoped exact-version gem) — -/// added in sorted position when the dep was transitive. Without this the +/// (`remote: `), and DEPENDENCIES pins ` ()!` +/// (bundler's source-pin spelling for a block-scoped exact-version gem: +/// `requirement` is `= `, plus any gemspec development dependency +/// bundler 2.7+ merges in, see [`super::gemspec`]) — added in sorted +/// position when the dep was transitive. Without this the /// CHECKSUMS pin leaves a MIXED state bundler refuses: the lock still /// attributes the gem to the upstream remote, so the prescribed unfrozen /// install exits 37 "mismatched checksums" and a frozen install exits 16. @@ -94,6 +96,7 @@ pub(crate) fn converge_gem_lock_source( dep: &DepOverride, index_url: &str, lock_name: &str, + requirement: &str, lock_changed: &mut bool, result: &mut RewriteResult, ) -> bool { @@ -140,7 +143,7 @@ pub(crate) fn converge_gem_lock_source( // DEPENDENCIES pin first — its lines sit AFTER the GEM sections, so the // spec move below never invalidates these indices (and vice versa would). - let target = format!(" {} (= {})!", dep.name, dep.version); + let target = format!(" {} ({requirement})!", dep.name); let entry_idx = deps .entries .iter() @@ -470,6 +473,7 @@ mod tests { dep, INDEX_URL, "Gemfile.lock", + &format!("= {}", dep.version), &mut changed, &mut result, ); diff --git a/crates/socket-patch-core/src/formats/gem/mod.rs b/crates/socket-patch-core/src/formats/gem/mod.rs index 122c02b18..e041581ba 100644 --- a/crates/socket-patch-core/src/formats/gem/mod.rs +++ b/crates/socket-patch-core/src/formats/gem/mod.rs @@ -19,6 +19,7 @@ //! readers that must refuse such a lock. pub(crate) mod gemfile; +pub(crate) mod gemspec; pub(crate) mod hosted; pub(crate) mod manifest; pub(crate) mod mirror; diff --git a/crates/socket-patch-core/src/hosted/engine.rs b/crates/socket-patch-core/src/hosted/engine.rs index 85f235c54..4984a5dec 100644 --- a/crates/socket-patch-core/src/hosted/engine.rs +++ b/crates/socket-patch-core/src/hosted/engine.rs @@ -790,6 +790,7 @@ pub async fn read_candidate_files( } if candidates.iter().any(|c| c.dep.ecosystem == "gem") { keep_bundler_loaded_gem_files(view, candidates, &mut out).await; + read_gemspecs(view, unreadable, &mut out).await; } // A Gradle build: every script, catalog and lock file its script graph // reaches, for the hosted Gradle planner. @@ -1173,6 +1174,64 @@ async fn keep_bundler_loaded_gem_files( })); } +/// The gemspecs the Gemfile's `gemspec` directives load: bundler 2.7+ +/// folds their development dependencies into the requirement the lock +/// pins (#985). Advisory: never rewritten, carried in the +/// [`GEMSPECS_KEY`](crate::formats::gem::gemspec::GEMSPECS_KEY) entry. A +/// directive the rewriter cannot read is left for it to report. +async fn read_gemspecs( + view: &ProjectView<'_>, + unreadable: &BTreeSet, + out: &mut CandidateFiles, +) { + use crate::formats::gem::gemspec::{gemspec_directives, is_gemspec_file, GEMSPECS_KEY}; + let Some(gemfile) = ["gems.rb", "Gemfile"] + .iter() + .find_map(|rel| out.files.get(*rel)) + else { + return; + }; + let dirs: BTreeSet = gemspec_directives(gemfile) + .unwrap_or_default() + .into_iter() + .map(|d| d.dir) + .collect(); + if dirs.is_empty() { + return; + } + let mut listing = serde_json::Map::new(); + for dir in dirs { + let entries = match view.list_dir(&dir).await { + Ok(entries) => entries, + Err(e) => { + listing.insert(dir, serde_json::Value::String(e.to_string())); + continue; + } + }; + let mut specs = serde_json::Map::new(); + for entry in entries + .into_iter() + .filter(|e| !e.is_dir && is_gemspec_file(&e.name)) + { + let rel = if dir.is_empty() { + entry.name.clone() + } else { + format!("{dir}/{}", entry.name) + }; + let text = read_advisory(view, unreadable, &rel).await; + specs.insert( + entry.name, + text.map_or(serde_json::Value::Null, serde_json::Value::String), + ); + } + listing.insert(dir, serde_json::Value::Object(specs)); + } + out.files.insert( + GEMSPECS_KEY.to_string(), + serde_json::Value::Object(listing).to_string(), + ); +} + /// The pypi wheels whose metadata a native lock rewrite needs, in /// candidate order: `(override, sha256)` of every `.whl` artifact some /// `uv.lock` / PEP 723 script lock would rewrite. Each native lock is diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index f31ba9fc3..e9086c0c1 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -7800,12 +7800,48 @@ fn rewrite_gem( // lock bundler < 2.2 wrote is proven frozen-installable after // that refresh (e2e_redirect_gem_build); a later bundler keeping // a merged lock keeps the caveat. + // Bundler 2.7+ folds a `gemspec` development dependency on the + // gem into its Gemfile requirement, so the lock must pin the + // union or every frozen install rejects the pair (#985). When + // that cannot be read, the plain exact pin still installs + // unfrozen; say so. + let requirement = gemfile + .as_deref() + .map(|gf| { + crate::formats::gem::gemspec::locked_exact_requirement( + gf, + lk, + &dep.name, + &dep.version, + &mut |dir| crate::formats::gem::gemspec::candidate_listing(files, dir), + ) + }) + .unwrap_or_else(|| Ok(format!("= {}", dep.version))) + .unwrap_or_else(|why| { + if checksums_era { + result.warnings.push(RewriteWarning { + code: "redirect_gem_gemspec_dependency_unreadable".into(), + detail: format!( + "{lock_name} pins {name} (= {version})!, but bundler 2.7+ merges \ + a gemspec development dependency on {name} into that \ + requirement and {why}; if the gemspec declares one, \ + frozen/deployment installs reject the pair — run `bundle \ + install` (unfrozen) once to record bundler's requirement in \ + {lock_name}", + name = dep.name, + version = dep.version, + ), + }); + } + format!("= {}", dep.version) + }); let converged = checksums_era && converge_gem_lock_source( lk, dep, &ov.index_url, lock_name, + &requirement, &mut lock_changed, result, ); @@ -17921,6 +17957,82 @@ mod tests { ); } + /// #985: on bundler 2.7+ a `gemspec` development dependency on the gem + /// merges into the redirected `gem` line that follows the directive, so + /// the DEPENDENCIES pin carries the union (`>= 0, = 7.0.0`) bundler + /// writes; a bare `(= 7.0.0)!` fails every frozen install. Bundler + /// 2.6, a `gem` line before the directive, and an unreadable gemspec + /// keep the exact pin (the last one with a warning). + #[test] + fn gem_pin_carries_the_gemspec_dev_dependency_bundler_merges() { + let gemfile = "source \"https://rubygems.org\"\n\ngemspec\n\ngroup :development do\n \ + gem \"rails\"\nend\n"; + let gemspec = "Gem::Specification.new do |s|\n s.name = \"mylib\"\n \ + s.add_development_dependency \"rails\", \">= 6\"\nend\n"; + let listing = serde_json::json!({ "": { "mylib.gemspec": gemspec } }).to_string(); + let lock = |bundler: &str| { + gem_lock(&format!(" rails (7.0.0) sha256={}", "2".repeat(64))) + .replace("rails (= 7.0.0)", "rails (>= 6)") + .replace("2.6.2", bundler) + }; + let pin = |files: &BTreeMap| { + let r = rewrite_registry_redirect(files, &[gem_override("rails", "7.0.0")]); + let lock = r.files.get("Gemfile.lock").expect("lock rewritten").clone(); + let pin = lock + .lines() + .skip_while(|l| *l != "DEPENDENCIES") + .nth(1) + .unwrap() + .to_string(); + ( + pin, + warning_codes(&r) + .iter() + .map(|c| c.to_string()) + .collect::>(), + ) + }; + let mut files = BTreeMap::new(); + files.insert("Gemfile".to_string(), gemfile.to_string()); + files.insert("Gemfile.lock".to_string(), lock("2.7.2")); + files.insert( + crate::formats::gem::gemspec::GEMSPECS_KEY.to_string(), + listing.clone(), + ); + let (merged, warnings) = pin(&files); + assert_eq!(merged, " rails (>= 6, = 7.0.0)!"); + assert!(warnings.is_empty(), "{warnings:?}"); + // Bundler 4 merges too. + files.insert("Gemfile.lock".to_string(), lock("4.0.22")); + assert_eq!(pin(&files).0, " rails (>= 6, = 7.0.0)!"); + // Re-running over the converged pair changes nothing. + let r = rewrite_registry_redirect(&files, &[gem_override("rails", "7.0.0")]); + let mut again = files.clone(); + again.extend(r.files.clone()); + let r2 = rewrite_registry_redirect(&again, &[gem_override("rails", "7.0.0")]); + assert!(r2.edits.is_empty(), "{:?}", r2.edits); + + // Bundler 2.6 never merges. + files.insert("Gemfile.lock".to_string(), lock("2.6.9")); + assert_eq!(pin(&files).0, " rails (= 7.0.0)!"); + // A `gem` line before the directive keeps its own requirement. + files.insert("Gemfile.lock".to_string(), lock("2.7.2")); + files.insert( + "Gemfile".to_string(), + "source \"https://rubygems.org\"\n\ngem \"rails\"\ngemspec\n".to_string(), + ); + assert_eq!(pin(&files).0, " rails (= 7.0.0)!"); + // The gemspec cannot be read: the exact pin, and a warning. + files.insert("Gemfile".to_string(), gemfile.to_string()); + files.insert( + crate::formats::gem::gemspec::GEMSPECS_KEY.to_string(), + serde_json::json!({ "": { "mylib.gemspec": null } }).to_string(), + ); + let (exact, warnings) = pin(&files); + assert_eq!(exact, " rails (= 7.0.0)!"); + assert_eq!(warnings, ["redirect_gem_gemspec_dependency_unreadable"]); + } + /// CHECKSUMS-era locks (bundler >= 4 writes the section by default) must /// come out FULLY CONVERGED, not mixed-state: the dep's spec entry moves /// out of the upstream GEM section into a patch-registry GEM section diff --git a/crates/socket-patch-core/src/vendor/gem.rs b/crates/socket-patch-core/src/vendor/gem.rs index 04fc21e60..881c9f557 100644 --- a/crates/socket-patch-core/src/vendor/gem.rs +++ b/crates/socket-patch-core/src/vendor/gem.rs @@ -485,10 +485,41 @@ fn gem_edits( ))) } }; + // ── DEPENDENCIES requirement ────────────────────────────────────────── + // Bundler 2.7+ folds a `gemspec` development dependency on the gem into + // the vendored `gem` line's requirement; the lock must pin that union or + // the frozen install vendored mode promises fails (#985). Read on the + // Gemfile as the plan leaves it. + let project_root = prelude.gemfile_path.parent().unwrap_or(Path::new(".")); + let requirement = match crate::formats::gem::gemspec::locked_exact_requirement( + &apply_gemfile_plan(&prelude.gemfile_text, &plan), + &prelude.lock_text, + name, + version, + &mut |dir| gemspec_listing(project_root, dir), + ) { + Ok(requirement) => requirement, + Err(why) => { + return Err(Box::new(refused( + "gemfile_declaration_not_editable", + format!( + "bundler 2.7+ merges a gemspec development dependency on `{name}` into the \ + vendored requirement, and {why}; refusing to write a Gemfile.lock frozen \ + installs could reject" + ), + ))) + } + }; // ── Gemfile.lock edit (pure text surgery, computed before any write) ── // A lock-shape failure therefore costs no download / copy / patch and no // Gemfile write. - let lock_edit = match edit_lock(&prelude.lock_text, name, version, &prelude.copy_rel) { + let lock_edit = match edit_lock( + &prelude.lock_text, + name, + version, + &requirement, + &prelude.copy_rel, + ) { Ok(edit) => edit, Err(e) => { return Err(Box::new(done( @@ -1655,11 +1686,42 @@ fn lock_record_lines(edit: &LockEdit) -> (Vec, Vec) { (original, new) } +/// The `*.gemspec` files of the root-relative directory `dir` under +/// `project_root`, for [`crate::formats::gem::gemspec::locked_exact_requirement`]. +fn gemspec_listing(project_root: &Path, dir: &str) -> crate::formats::gem::gemspec::GemspecListing { + let path = project_root.join(dir); + let entries = std::fs::read_dir(&path).map_err(|e| e.to_string())?; + let mut out = Vec::new(); + for entry in entries { + let entry = entry.map_err(|e| e.to_string())?; + let Some(file) = entry.file_name().to_str().map(str::to_string) else { + continue; + }; + if !crate::formats::gem::gemspec::is_gemspec_file(&file) + || entry.file_type().is_ok_and(|t| t.is_dir()) + { + continue; + } + let text = crate::utils::fs::read_regular_to_string_sync(&path.join(&file)).ok(); + out.push((file, text)); + } + out.sort(); + Ok(out) +} + /// Produce the pair-edited lock text (see the module doc for the canonical /// form). Pure string surgery on exact line spans — every byte not /// deliberately changed is preserved, which is what keeps the result -/// byte-identical to what bundler regenerates. -fn edit_lock(text: &str, name: &str, version: &str, rel: &str) -> Result { +/// byte-identical to what bundler regenerates. `requirement` is what the +/// DEPENDENCIES pin carries inside its parentheses (`= `, or the +/// gemspec-merged union). +fn edit_lock( + text: &str, + name: &str, + version: &str, + requirement: &str, + rel: &str, +) -> Result { let mut lines: Vec = text.split('\n').map(str::to_string).collect(); // 1. Lift the gem's spec block out of GEM/specs — or, on a re-vendor to @@ -1787,7 +1849,7 @@ fn edit_lock(text: &str, name: &str, version: &str, rel: &str) -> Result = None; let mut insert_at = dep_start + 1; let mut existing_idx: Option = None; @@ -3090,6 +3152,77 @@ mod tests { ); } + /// #985: on bundler 2.7+ a `gemspec` development dependency on the gem + /// merges into the vendored `gem` line after the directive, so the lock + /// pins the union bundler writes (`rack (>= 3, = 3.2.6)!`); the bare + /// exact pin fails the frozen install vendored mode promises. A gemspec + /// that cannot be read refuses before any write. + #[tokio::test] + async fn gemspec_dev_dependency_merges_into_the_lock_pin() { + let gemfile = "source \"https://rubygems.org\"\n\ngemspec\n\ngem \"puma\"\ngem \"rack\", \"~> 3.1\"\n"; + let lock = LOCK_DIRECT + .replace( + " puma\n rack (~> 3.1)\n", + " mylib!\n puma\n rack (~> 3.1, >= 3)\n", + ) + .replace("2.5.22", "2.7.2"); + let spec = "Gem::Specification.new do |s|\n s.name = \"mylib\"\n \ + s.add_development_dependency \"rack\", \">= 3\"\nend\n"; + + let (_tmp, root, installed, blobs, record) = fixture(gemfile, &lock).await; + tokio::fs::write(root.join("mylib.gemspec"), spec) + .await + .unwrap(); + let (result, _entry, _w) = + unwrap_done(run_vendor(&root, &blobs, &installed, &record, false).await); + assert!(result.success, "vendor failed: {:?}", result.error); + let wired = tokio::fs::read_to_string(root.join(GEMFILE_LOCK)) + .await + .unwrap(); + assert!( + wired.contains("\nDEPENDENCIES\n mylib!\n puma\n rack (>= 3, = 3.2.6)!\n"), + "{wired}" + ); + + // Bundler < 2.7 keeps the exact pin. + let (_tmp, root, installed, blobs, record) = + fixture(gemfile, &lock.replace("2.7.2", "2.6.9")).await; + tokio::fs::write(root.join("mylib.gemspec"), spec) + .await + .unwrap(); + let (result, _entry, _w) = + unwrap_done(run_vendor(&root, &blobs, &installed, &record, false).await); + assert!(result.success, "vendor failed: {:?}", result.error); + let wired = tokio::fs::read_to_string(root.join(GEMFILE_LOCK)) + .await + .unwrap(); + assert!(wired.contains("\n rack (= 3.2.6)!\n"), "{wired}"); + + // Two gemspecs and no `name:`: bundler's choice is unknown, refuse. + let (_tmp, root, installed, blobs, record) = fixture(gemfile, &lock).await; + tokio::fs::write(root.join("mylib.gemspec"), spec) + .await + .unwrap(); + tokio::fs::write(root.join("other.gemspec"), spec) + .await + .unwrap(); + let outcome = run_vendor(&root, &blobs, &installed, &record, false).await; + assert!( + matches!(&outcome, VendorOutcome::Refused { code, .. } if *code == "gemfile_declaration_not_editable"), + "{outcome:?}" + ); + assert_eq!( + tokio::fs::read_to_string(root.join(GEMFILE_LOCK)) + .await + .unwrap(), + lock + ); + assert_eq!( + tokio::fs::read_to_string(root.join(GEMFILE)).await.unwrap(), + gemfile + ); + } + #[tokio::test] async fn test_direct_dep_happy_path() { let (_tmp, root, installed, blobs, record) = fixture(GEMFILE_DIRECT, LOCK_DIRECT).await; @@ -4488,7 +4621,7 @@ mod tests { SPIKE_RACK_SHA_LINE, &format!("{SPIKE_RACK_SHA_LINE}\n{SPIKE_RACK_SHA_LINE}"), ); - let err = match edit_lock(&lock, "rack", "3.1.8", ©_rel_318()) { + let err = match edit_lock(&lock, "rack", "3.1.8", "= 3.1.8", ©_rel_318()) { Err(e) => e, Ok(_) => panic!("duplicate CHECKSUMS entries must fail closed"), }; @@ -4499,7 +4632,7 @@ mod tests { fn test_no_checksums_lock_records_no_checksum_wiring() { // A lock WITHOUT a CHECKSUMS section must keep producing // the exact pre-CHECKSUMS output and no checksum record. - let edit = edit_lock(LOCK_DIRECT, "rack", "3.2.6", ©_rel()).unwrap(); + let edit = edit_lock(LOCK_DIRECT, "rack", "3.2.6", "= 3.2.6", ©_rel()).unwrap(); assert!(edit.checksum_rewrite.is_none()); assert_eq!(edit.text, expected_lock_direct()); } @@ -4956,7 +5089,7 @@ mod tests { fn test_gem_specs_platform_sibling_fails_closed() { let lock = "GEM\n remote: https://rubygems.org/\n specs:\n nokogiri (1.16.0)\n racc (~> 1.4)\n nokogiri (1.16.0-arm64-darwin)\n racc (~> 1.4)\n\nPLATFORMS\n arm64-darwin\n ruby\n\nDEPENDENCIES\n nokogiri\n\nBUNDLED WITH\n 2.5.22\n"; let rel = format!(".socket/vendor/gem/{UUID}/nokogiri-1.16.0"); - let err = match edit_lock(lock, "nokogiri", "1.16.0", &rel) { + let err = match edit_lock(lock, "nokogiri", "1.16.0", "= 1.16.0", &rel) { Err(e) => e, Ok(_) => panic!("a platform-suffixed GEM specs sibling must fail closed"), }; @@ -7493,7 +7626,7 @@ mod tests { fn edit_lock_missing_specs_stanza_fails_closed() { let lock = LOCK_DIRECT.replace(" specs:\n", ""); assert_ne!(lock, LOCK_DIRECT); - let err = edit_lock(&lock, "rack", "3.2.6", ©_rel()) + let err = edit_lock(&lock, "rack", "3.2.6", "= 3.2.6", ©_rel()) .err() .expect("a specs:-less GEM section must fail closed"); assert!(err.contains("no specs: stanza"), "{err}"); @@ -7510,7 +7643,7 @@ mod tests { "PATH\n remote: {rel}\n specs:\n other (1.0)\n{tail}", rel = copy_rel() ); - let err = edit_lock(&lost, "rack", "3.2.6", ©_rel()) + let err = edit_lock(&lost, "rack", "3.2.6", "= 3.2.6", ©_rel()) .err() .expect("a PATH section without its spec entry must fail closed"); assert!(err.contains("lost its"), "{err}"); @@ -7519,7 +7652,7 @@ mod tests { "PATH\n remote: {rel}\n specs:\n rack (3.2.6)\n hand: edit\n{tail}", rel = copy_rel() ); - let err = edit_lock(&edited, "rack", "3.2.6", ©_rel()) + let err = edit_lock(&edited, "rack", "3.2.6", "= 3.2.6", ©_rel()) .err() .expect("a hand-edited PATH section must fail closed"); assert!(err.contains("not the shape vendor wrote"), "{err}"); @@ -7533,7 +7666,7 @@ mod tests { let git = "GIT\n remote: https://example.com/dep.git\n revision: abc123\n specs:\n dep (1.0)\n\n"; let lock = format!("{git}{LOCK_DIRECT}"); - let edit = edit_lock(&lock, "rack", "3.2.6", ©_rel()).unwrap(); + let edit = edit_lock(&lock, "rack", "3.2.6", "= 3.2.6", ©_rel()).unwrap(); assert_eq!(edit.text, format!("{git}{}", expected_lock_direct())); } @@ -8461,7 +8594,7 @@ mod tests { /// "GEM specs has no entry" although the lock lists it. #[test] fn edit_lock_lifts_spec_from_second_gem_section() { - let edit = edit_lock(LOCK_TWO_SOURCES, "rack", "3.2.6", ©_rel()).unwrap(); + let edit = edit_lock(LOCK_TWO_SOURCES, "rack", "3.2.6", "= 3.2.6", ©_rel()).unwrap(); assert_eq!(edit.text, expected_lock_two_sources()); assert!(!edit.rewired_ours); assert_eq!( @@ -8478,7 +8611,7 @@ mod tests { /// recorded, so ledgers written before #779 and after it agree. #[test] fn edit_lock_first_gem_section_records_no_source_remote() { - let edit = edit_lock(LOCK_DIRECT, "rack", "3.2.6", ©_rel()).unwrap(); + let edit = edit_lock(LOCK_DIRECT, "rack", "3.2.6", "= 3.2.6", ©_rel()).unwrap(); assert_eq!(edit.source_remote, None); let (original, _new) = lock_record_lines(&edit); assert!(original.iter().all(|l| !l.starts_with(" remote: "))); @@ -8493,7 +8626,7 @@ mod tests { " aaa-internal (1.0.0)\n", " aaa-internal (1.0.0)\n rack (3.2.6)\n", ); - let err = edit_lock(&lock, "rack", "3.2.6", ©_rel()) + let err = edit_lock(&lock, "rack", "3.2.6", "= 3.2.6", ©_rel()) .err() .expect("a spec listed in two GEM sections must fail closed"); assert!(err.contains("more than one GEM section"), "{err}"); @@ -8507,7 +8640,7 @@ mod tests { " aaa-internal (1.0.0)\n", " aaa-internal (1.0.0)\n rack (3.2.6-x86_64-linux)\n", ); - let err = edit_lock(&lock, "rack", "3.2.6", ©_rel()) + let err = edit_lock(&lock, "rack", "3.2.6", "= 3.2.6", ©_rel()) .err() .expect("a platform sibling in any GEM section must fail closed"); assert!(err.contains("platform-suffixed"), "{err}"); @@ -8570,7 +8703,7 @@ mod tests { /// later revert still targets the second GEM section. #[test] fn revert_lock_text_targets_recorded_gem_section() { - let edit = edit_lock(LOCK_TWO_SOURCES, "rack", "3.2.6", ©_rel()).unwrap(); + let edit = edit_lock(LOCK_TWO_SOURCES, "rack", "3.2.6", "= 3.2.6", ©_rel()).unwrap(); let (original, new) = lock_record_lines(&edit); assert_eq!( revert_lock_text(&edit.text, &original, &new).as_deref(), From 32d42766fa4cb93973dd4283a39e97c1646e9f30 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 21:02:42 +0000 Subject: [PATCH 3/5] Refuse vendoring a gem merged with a gemspec dep Real Bundler (2.7.2 and 4.0.18) rejects even its own lock under frozen mode when a path gem's `gem` line follows a `gemspec` directive whose gemspec declares the same gem as a development dependency ("rack from `` to ``"). So no lock vendored mode can write keeps its frozen-install promise in that shape. Vendoring now refuses it before any write (`gemfile_declaration_not_editable`) and names the fix: declare the gem above `gemspec`, where Bundler keeps its own requirement and the vendored pair installs frozen, or use hosted mode. Bundler < 2.7 and a `gem` line before the directive vendor as before. Adds real-Bundler e2e coverage for both modes: the hosted pair installs frozen with the merged pin, and the vendored refusal and its remedy behave as described. Documents both in CLI_CONTRACT.md. Refs #985 Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 2 +- .../tests/e2e_redirect_gem_build.rs | 87 ++++++++- .../tests/e2e_vendor_gem_build.rs | 166 +++++++++++++++++ crates/socket-patch-core/src/vendor/gem.rs | 171 +++++++++--------- 4 files changed, 342 insertions(+), 84 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index e2bc9b3d9..f0e0c473f 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -934,7 +934,7 @@ to **six flavors**. | cargo | crate dir `-/` (no `.cargo-checksum.json`) | (v5.0) `[patch.crates-io]` path entry in the **workspace-root `Cargo.toml`** (the manifest beside the `Cargo.lock` it detaches — never `.cargo/config*`) **+** Cargo.lock surgery (the `[[package]]` entry's `source`/`checksum` removed and its `version` set to the copy's TAGGED version `+socket.` — `+.socket.` when the version already has build metadata — with every lock reference that spells the old version rewritten, formats v1–v4; the copy's own `Cargo.toml` version carries the same tag, so the patched crate sees it in `CARGO_PKG_VERSION`; revert restores the lock byte for byte). Key: always the Socket-owned `-socket-` with `package = ""` (the full uuid hex when that key is taken), never the bare crate name — cargo lets a config-file `[patch]` item (project, ancestor directory or `$CARGO_HOME`) replace the manifest item with the same key whatever its version, so keys any of those configs use are avoided and a re-run moves an entry off a now-shadowed key; two versions of one crate are wired side by side. Pre-v5 wiring in `.cargo/config.toml` / `.cargo/config` is moved into `Cargo.toml` by a re-run (`vendor`, `scan`/`get --mode vendored`) or `repair` (`cargo_wiring_migrated` note; the ledger's `cargo_patch_entry` record then names `Cargo.toml`); a detached lock entry left unwired by the pre-v5 multi-version overwrite is re-wired the same way (`cargo_wiring_restored`); every revert removes both spellings | `cargo build --locked --offline` on a fresh checkout — single-version manifest `[patch]` also builds with no network on cargo older than 1.56 (the old config-file wiring's floor); two vendored versions of ONE crate need cargo 1.45 or newer (`--offline` from an empty CARGO_HOME is enough there); older cargo fails closed whatever the index state, and a project that does not pin cargo ≥ 1.45 (`rust-version`, a `rust-version.workspace = true` inheritor's `[workspace.package]` value, or toolchain file) gets the `cargo_multi_version_old_cargo` warning. Note: path deps build **without** `--cap-lints allow` | | golang | module dir `@/` | `go.mod` `replace => ./.socket/vendor/golang//@` | `go build` with `GOPROXY=off` + empty `GOMODCACHE` (directory replaces bypass go.sum entirely; survives `go mod tidy`) | | composer | package dir `/@/`; the copy's `.gitignore` / `.hgignore` are emptied and its `.gitattributes` `export-ignore` rules dropped, because Composer's path mirror skips the files they match (`vendor_composer_mirror_filters_neutralized`; a patch that rewrites one of them is refused `vendor_composer_mirror_filter_conflict`). Re-runs heal copies vendored before this | `composer.lock` only: entry's `dist` → `{type: "path", url, reference: ""}`, `source` removed, `transport-options: {symlink: false}` added. `content-hash` unaffected; `composer.json` untouched | `composer install` (from the lock alone, real copy not symlink, works under `--network none`). Composer 1 does not reinstall an already-installed package whose dist changed: remove `//` first (the hint names the project's resolved vendor dir: `COMPOSER_VENDOR_DIR`, else `config.vendor-dir`, else `vendor`). `composer update ` reverts it. See `docs/testing/composer-compatibility.md` | -| gem | gem dir `-/` + gemspec materialized from `specifications/` | **Gemfile + Gemfile.lock pair**: the `gem` line gains `path:` (or a managed block for transitive deps); the lock's spec block moves GEM→PATH and the DEPENDENCIES entry becomes ` (= )!`, in bundler's exact canonical form | `bundle install` (normal **and** `BUNDLE_FROZEN=true`), byte-stable lock. Lock-only edits are a silent unpatch — hence the mandatory pair | +| gem | gem dir `-/` + gemspec materialized from `specifications/` | **Gemfile + Gemfile.lock pair**: the `gem` line gains `path:` (or a managed block for transitive deps); the lock's spec block moves GEM→PATH and the DEPENDENCIES entry becomes ` (= )!`, in bundler's exact canonical form. On bundler 2.7+ (the lock's `BUNDLED WITH`), a `gem` line that follows a `gemspec` directive whose gemspec declares a development dependency on the same gem is refused `gemfile_declaration_not_editable` before any write: bundler merges the two requirements and then rejects even its own lock for a path gem under frozen mode, so the refusal names the remedy (declare the gem above `gemspec`, or use hosted mode). Hosted mode instead pins the merged requirement bundler writes (` (>= 0, = )!`), and warns `redirect_gem_gemspec_dependency_unreadable` when the gemspec cannot be read (#985) | `bundle install` (normal **and** `BUNDLE_FROZEN=true`), byte-stable lock. Lock-only edits are a silent unpatch — hence the mandatory pair | | pypi / uv (uv.lock) | rebuilt wheel (canonical PEP 427 filename; RECORD regenerated) | `[tool.uv.sources] = {path}` in pyproject + surgical uv.lock rewrite; transitive deps via `[tool.uv] override-dependencies` | `uv sync --locked` / `--frozen --offline` (hash-verified, byte-stable lock) | | pypi / poetry (poetry.lock: legacy `[metadata.hashes]`, lock 1.0/1.1 `[metadata.files]`, 2.x `files`) | (rebuilt wheel) | lock-only: the target `[[package]]` gets `[package.source] type="file"` (+ `reference = ""` on the 0.12/1.0 layouts, which read it unconditionally) and the single `{file, hash: sha256-of-our-wheel}` entry in whichever table the generation keeps it. pyproject + `metadata.content-hash` untouched; CRLF locks keep their line endings. A lock written by Poetry < 1.4 emits `pypi_poetry_integrity_unverified` (that installer verifies no local hashes and skips an already-installed version) | `poetry check --lock && poetry sync`, cold cache (hash fail-closed from Poetry 1.4; byte-stable lock) — see `docs/testing/poetry-compatibility.md` | | pypi / pdm (pdm.lock) | (rebuilt wheel) | lock-only: the `[[package]]` gains the local-file `path` + `files[]` hash. pyproject + `content_hash` untouched. Non-fixture `[metadata] strategy` / hash-less locks refused | `pdm sync` (+ `pdm install --check`), cold cache | 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 64d7229ba..d8d082b53 100644 --- a/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs +++ b/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs @@ -131,6 +131,19 @@ const PURL: &str = "pkg:gem/vuln-gem@1.0.0"; /// The runtime probe constant baked into the PATCHED lib — observable at /// `require` time, carries the patch uuid so the assert can't pass on any /// other content. +/// The gem library a [`Driver::ScanVexGemspecDevDependency`] project +/// builds: its gemspec declares the vulnerable gem as a development +/// dependency. +const GEMSPEC_LIB: &str = "mylib.gemspec"; + +fn gemspec_lib() -> String { + format!( + "Gem::Specification.new do |s|\n s.name = \"mylib\"\n s.version = \"0.1.0\"\n \ + s.summary = \"x\"\n s.authors = [\"x\"]\n s.files = [\"lib/mylib.rb\"]\n \ + s.add_development_dependency \"{DEP}\"\nend\n" + ) +} + fn patched_marker() -> String { format!("PATCHED-{UUID}") } @@ -458,6 +471,11 @@ enum Driver { /// [`Driver::ScanVex`] on a declaration ending in a bare `;` and a /// comment (#826): a complete declaration, so it is redirected. ScanVexTrailingSemicolonDeclaration, + /// [`Driver::ScanVex`] on a gem library whose Gemfile's `gemspec` + /// directive also declares the gem as a development dependency (#985): + /// bundler 2.7+ merges the two requirements, and the lock must pin the + /// union for frozen installs to accept it. + ScanVexGemspecDevDependency, /// [`Driver::ScanVexDualBoot`] with `BUNDLE_GEMFILE=Gemfile` exported to /// socket-patch too (#507): bundler's local app config outranks the /// environment, so bundler still loads `Gemfile.next` and the run must @@ -555,6 +573,9 @@ impl Driver { Driver::ScanVexTrailingSemicolonDeclaration => { "scan --mode hosted (gem line ending in `;`)" } + Driver::ScanVexGemspecDevDependency => { + "scan --mode hosted (gem also a gemspec development dependency)" + } Driver::ScanVexCustomGitSource => "scan --mode hosted (gem from a custom git_source)", } } @@ -946,6 +967,15 @@ async fn redirect_scanned_project( "source \"{}/upstream\"\n\ngem \"{DEP}\"; # the vulnerable one\n", server.uri() ), + Driver::ScanVexGemspecDevDependency => { + std::fs::create_dir_all(proj.join("lib")).unwrap(); + std::fs::write(proj.join("lib/mylib.rb"), "module Mylib; end\n").unwrap(); + std::fs::write(proj.join(GEMSPEC_LIB), gemspec_lib()).unwrap(); + format!( + "source \"{}/upstream\"\n\ngemspec\n\ngroup :development do\n gem \"{DEP}\"\nend\n", + server.uri() + ) + } _ => format!("source \"{}/upstream\"\n\ngem \"{DEP}\"\n", server.uri()), }; std::fs::write(proj.join(gemfile_name), gemfile_body).unwrap(); @@ -1139,7 +1169,8 @@ async fn redirect_scanned_project( | Driver::ScanVexHeredocDeclaration | Driver::ScanVexInterpolatedHeredocDeclaration | Driver::ScanVexSemicolonJoinedDeclaration - | Driver::ScanVexTrailingSemicolonDeclaration => vec![ + | Driver::ScanVexTrailingSemicolonDeclaration + | Driver::ScanVexGemspecDevDependency => vec![ "scan", "--mode", "hosted", @@ -1340,7 +1371,8 @@ async fn redirect_scanned_project( | Driver::ScanVexSourceBlockMultiLineOpener | Driver::ScanVexSourceBlockBraces | Driver::ScanVexSourceBlockAfterSemicolon - | Driver::ScanVexTrailingSemicolonDeclaration => { + | Driver::ScanVexTrailingSemicolonDeclaration + | Driver::ScanVexGemspecDevDependency => { assert_eq!(env["vex"]["statements"], 1, "vex block: {env}"); assert!( env["vex"]["warnings"] @@ -1570,6 +1602,11 @@ fn stage_fresh_checkout(fx: &RedirectFixture, name: &str) -> PathBuf { std::fs::create_dir_all(&fresh).unwrap(); std::fs::copy(fx.proj.join(fx.gemfile_name), fresh.join(fx.gemfile_name)).unwrap(); std::fs::copy(fx.proj.join(fx.lock_name), fresh.join(fx.lock_name)).unwrap(); + // A gem library's own sources (Driver::ScanVexGemspecDevDependency). + if fx.proj.join(GEMSPEC_LIB).exists() { + std::fs::copy(fx.proj.join(GEMSPEC_LIB), fresh.join(GEMSPEC_LIB)).unwrap(); + copy_dir_recursive(&fx.proj.join("lib"), &fresh.join("lib")); + } copy_dir_recursive(&fx.proj.join(".socket"), &fresh.join(".socket")); copy_dir_recursive(&fx.proj.join(".bundle"), &fresh.join(".bundle")); assert!( @@ -3024,6 +3061,52 @@ async fn gem_hosted_trailing_semicolon_declaration_redirects_and_installs() { assert_patched_install(&fx, &fresh); } +/// #985: a gem library whose Gemfile's `gemspec` directive declares the +/// vulnerable gem as a development dependency AND whose Gemfile declares it +/// too. Bundler 2.7+ merges the two into one Gemfile dependency, so the +/// redirected lock must pin the union (`(>= 0, = 1.0.0)!`): a fresh +/// checkout's FROZEN install accepts the pair unchanged and installs the +/// patched bytes (before the fix it exited 16). Older bundlers keep the +/// plain exact pin, which they accept frozen too. +#[tokio::test(flavor = "multi_thread")] +#[ignore = "host capstone: shells out to a real ruby/gem/bundler (>= 2.6); \ + run with a pinned toolchain via --ignored"] +async fn gem_hosted_gemspec_dev_dependency_frozen_install_accepts_the_pin() { + let Some(fx) = redirect_scanned_project( + "gemspec-dev-dependency", + Spelling::Gemfile, + true, + true, + None, + Driver::ScanVexGemspecDevDependency, + ) + .await + else { + return; + }; + let lock = std::fs::read_to_string(fx.proj.join(fx.lock_name)).unwrap(); + let want = if fx.bundler.at_least(2, 7) { + format!(" {DEP} (>= 0, = {DEP_VERSION})!\n") + } else { + format!(" {DEP} (= {DEP_VERSION})!\n") + }; + assert!(lock.contains(&want), "lock must pin {want:?}:\n{lock}"); + let fresh = stage_fresh_checkout(&fx, "fresh-frozen"); + let install = bundle_env(&fresh, &["install"], &[("BUNDLE_FROZEN", "true")]); + assert!( + install.status.success(), + "fresh-checkout frozen `bundle install` must accept the redirected pair.\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&install.stdout), + String::from_utf8_lossy(&install.stderr), + ); + assert_eq!( + std::fs::read_to_string(fresh.join(fx.lock_name)).unwrap(), + lock, + "the frozen install must leave the lock byte-identical" + ); + assert_patched_install(&fx, &fresh); +} + /// #340: a `gem` declaration with an `if` modifier must not be rewritten /// (the rewrite dropped the condition and declared the gem unconditionally). #[tokio::test(flavor = "multi_thread")] diff --git a/crates/socket-patch-cli/tests/e2e_vendor_gem_build.rs b/crates/socket-patch-cli/tests/e2e_vendor_gem_build.rs index d5dc62056..8acc9c297 100644 --- a/crates/socket-patch-cli/tests/e2e_vendor_gem_build.rs +++ b/crates/socket-patch-cli/tests/e2e_vendor_gem_build.rs @@ -1940,3 +1940,169 @@ fn gem_vendor_drops_positional_constraints() { ); } } + +/// The gem library a #985 project builds: its gemspec declares rack as a +/// development dependency. Writes `Gemfile` (the given body), the gemspec +/// and its lib, then installs unfrozen. +fn gemspec_library(proj: &Path, gemfile: &str) { + std::fs::write(proj.join("Gemfile"), gemfile).unwrap(); + std::fs::create_dir_all(proj.join("lib")).unwrap(); + std::fs::write(proj.join("lib/mylib.rb"), "module Mylib; end\n").unwrap(); + std::fs::write( + proj.join("mylib.gemspec"), + "Gem::Specification.new do |s|\n s.name = \"mylib\"\n s.version = \"0.1.0\"\n \ + s.summary = \"x\"\n s.authors = [\"x\"]\n s.files = [\"lib/mylib.rb\"]\n \ + s.add_development_dependency \"rack\", \">= 3\"\nend\n", + ) + .unwrap(); + let relock = bundle(proj, &["install"], false); + assert!( + relock.status.success(), + "the gem library installs before vendor (test premise):\n{}", + String::from_utf8_lossy(&relock.stderr) + ); +} + +/// #985: a gem library whose Gemfile's `gemspec` directive declares rack as +/// a development dependency, and whose Gemfile declares rack AFTER the +/// directive. Bundler 2.7+ merges the two requirements, and for a path gem +/// it then rejects even the lock it writes itself under frozen mode, so +/// vendoring refuses before any write (before the fix it wrote a pair whose +/// frozen install exited 16) and names the remedy. With the `gem` line moved +/// above `gemspec`, the vendored pair installs frozen and reverts cleanly. +/// Bundler < 2.7 never merges and vendors either order. +#[test] +#[ignore = "host capstone: shells out to a real bundler >= 1.17; the unpinned `test` job \ + skips it, the e2e job runs it with a pinned toolchain via --ignored"] +fn gem_vendor_gemspec_dev_dependency_refuses_or_installs_frozen() { + let Some((_tmp, proj, bundler, purl)) = staged_rack_project("gemspec dev dependency") else { + return; + }; + let after = "source \"https://rubygems.org\"\n\ngemspec\n\ngem \"rack\", \"~> 3.1\"\n"; + let before = "source \"https://rubygems.org\"\n\ngem \"rack\", \"~> 3.1\"\n\ngemspec\n"; + let vendor = |proj: &Path| { + run_socket( + proj, + &[ + "vendor", + "--json", + "--offline", + "--cwd", + proj.to_str().unwrap(), + ], + ) + }; + + if bundler.at_least(2, 7) { + gemspec_library(&proj, after); + let lock_before = std::fs::read(proj.join("Gemfile.lock")).unwrap(); + let (code, stdout, stderr) = vendor(&proj); + assert_ne!( + code, 0, + "vendor must refuse.\nstdout:\n{stdout}\nstderr:\n{stderr}" + ); + let env = parse_json_envelope(&stdout); + assert_eq!(env["summary"]["applied"], 0, "nothing vendored: {env}"); + let event = env["events"] + .as_array() + .unwrap() + .iter() + .find(|e| e["purl"] == purl) + .unwrap_or_else(|| panic!("an event for {purl}: {env}")); + assert_eq!( + event["errorCode"], "gemfile_declaration_not_editable", + "event: {event}" + ); + assert!( + event["error"] + .as_str() + .is_some_and(|e| e.contains("above `gemspec`")), + "the refusal names the remedy: {event}" + ); + assert_eq!( + std::fs::read_to_string(proj.join("Gemfile")).unwrap(), + after + ); + assert_eq!( + std::fs::read(proj.join("Gemfile.lock")).unwrap(), + lock_before + ); + assert!( + !proj.join(".socket/vendor/gem").exists(), + "no vendored copy is written" + ); + } + + // The remedy (and, on bundler < 2.7, either order): vendored, frozen. + let gemfile = if bundler.at_least(2, 7) { + before + } else { + after + }; + gemspec_library(&proj, gemfile); + let lock_before = std::fs::read(proj.join("Gemfile.lock")).unwrap(); + let (code, stdout, stderr) = vendor(&proj); + assert_eq!( + code, 0, + "vendor failed.\nstdout:\n{stdout}\nstderr:\n{stderr}" + ); + let env = parse_json_envelope(&stdout); + assert_eq!(env["summary"]["applied"], 1, "{env}"); + let wired = std::fs::read_to_string(proj.join("Gemfile.lock")).unwrap(); + let version = purl.rsplit('@').next().unwrap(); + assert!( + wired.contains(&format!(" rack (= {version})!\n")), + "exact pin:\n{wired}" + ); + let install = bundle(&proj, &["install"], true); + assert!( + install.status.success(), + "frozen install of the vendored pair failed:\n{wired}\n{}", + String::from_utf8_lossy(&install.stderr) + ); + assert_eq!( + std::fs::read_to_string(proj.join("Gemfile.lock")).unwrap(), + wired, + "the frozen install keeps the lock byte-identical" + ); + let probe = bundle( + &proj, + &[ + "exec", + "ruby", + "-e", + "print Gem.loaded_specs.fetch(\"rack\").full_gem_path", + ], + true, + ); + let loaded = String::from_utf8_lossy(&probe.stdout); + assert!( + probe.status.success() && loaded.contains(".socket/vendor/gem/"), + "rack must load from the vendored path ({purl}), got {loaded:?}:\n{}", + String::from_utf8_lossy(&probe.stderr) + ); + let (code, stdout, stderr) = run_socket( + &proj, + &[ + "vendor", + "--revert", + "--json", + "--cwd", + proj.to_str().unwrap(), + ], + ); + assert_eq!( + code, 0, + "revert failed.\nstdout:\n{stdout}\nstderr:\n{stderr}" + ); + assert_eq!( + std::fs::read_to_string(proj.join("Gemfile")).unwrap(), + gemfile, + "revert restores the Gemfile byte-identical" + ); + assert_eq!( + std::fs::read(proj.join("Gemfile.lock")).unwrap(), + lock_before, + "revert restores Gemfile.lock byte-identical" + ); +} diff --git a/crates/socket-patch-core/src/vendor/gem.rs b/crates/socket-patch-core/src/vendor/gem.rs index 881c9f557..39634af91 100644 --- a/crates/socket-patch-core/src/vendor/gem.rs +++ b/crates/socket-patch-core/src/vendor/gem.rs @@ -485,41 +485,49 @@ fn gem_edits( ))) } }; - // ── DEPENDENCIES requirement ────────────────────────────────────────── + // ── gemspec development dependency (#985) ──────────────────────────── // Bundler 2.7+ folds a `gemspec` development dependency on the gem into - // the vendored `gem` line's requirement; the lock must pin that union or - // the frozen install vendored mode promises fails (#985). Read on the + // a `gem` line that follows the directive. For a path gem bundler then + // rejects even the lock it writes itself under frozen/deployment mode + // ("rack from `` to ``", verified on 2.7.2 and 4.0.18), so + // no lock this backend could write keeps vendored mode's frozen-install + // promise. Refuse with the remedy: declared before the directive, the + // gem keeps its own requirement and installs frozen. Read on the // Gemfile as the plan leaves it. let project_root = prelude.gemfile_path.parent().unwrap_or(Path::new(".")); - let requirement = match crate::formats::gem::gemspec::locked_exact_requirement( + let merged = crate::formats::gem::gemspec::locked_exact_requirement( &apply_gemfile_plan(&prelude.gemfile_text, &plan), &prelude.lock_text, name, version, &mut |dir| gemspec_listing(project_root, dir), - ) { - Ok(requirement) => requirement, - Err(why) => { - return Err(Box::new(refused( - "gemfile_declaration_not_editable", - format!( - "bundler 2.7+ merges a gemspec development dependency on `{name}` into the \ - vendored requirement, and {why}; refusing to write a Gemfile.lock frozen \ - installs could reject" - ), - ))) - } + ); + let refusal = match merged { + Ok(requirement) if requirement == format!("= {version}") => None, + Ok(requirement) => Some(format!( + "the gemspec the Gemfile's `gemspec` directive loads declares a development \ + dependency on `{name}`, so bundler 2.7+ locks the vendored gem as `{name} \ + ({requirement})!` and then rejects that lock under frozen/deployment mode (a \ + bundler limitation for path gems)" + )), + Err(why) => Some(format!( + "bundler 2.7+ merges a gemspec development dependency on `{name}` into the \ + vendored gem's requirement, and {why}" + )), }; + if let Some(why) = refusal { + return Err(Box::new(refused( + "gemfile_declaration_not_editable", + format!( + "{why}; move the `gem \"{name}\"` declaration above `gemspec` in the Gemfile \ + (bundler then keeps its own requirement), or use hosted mode" + ), + ))); + } // ── Gemfile.lock edit (pure text surgery, computed before any write) ── // A lock-shape failure therefore costs no download / copy / patch and no // Gemfile write. - let lock_edit = match edit_lock( - &prelude.lock_text, - name, - version, - &requirement, - &prelude.copy_rel, - ) { + let lock_edit = match edit_lock(&prelude.lock_text, name, version, &prelude.copy_rel) { Ok(edit) => edit, Err(e) => { return Err(Box::new(done( @@ -1712,16 +1720,8 @@ fn gemspec_listing(project_root: &Path, dir: &str) -> crate::formats::gem::gemsp /// Produce the pair-edited lock text (see the module doc for the canonical /// form). Pure string surgery on exact line spans — every byte not /// deliberately changed is preserved, which is what keeps the result -/// byte-identical to what bundler regenerates. `requirement` is what the -/// DEPENDENCIES pin carries inside its parentheses (`= `, or the -/// gemspec-merged union). -fn edit_lock( - text: &str, - name: &str, - version: &str, - requirement: &str, - rel: &str, -) -> Result { +/// byte-identical to what bundler regenerates. +fn edit_lock(text: &str, name: &str, version: &str, rel: &str) -> Result { let mut lines: Vec = text.split('\n').map(str::to_string).collect(); // 1. Lift the gem's spec block out of GEM/specs — or, on a re-vendor to @@ -1849,7 +1849,7 @@ fn edit_lock( // Gemfile dependency now. let (dep_start, dep_end) = section_span(&lines, "DEPENDENCIES") .ok_or_else(|| "Gemfile.lock has no DEPENDENCIES section".to_string())?; - let new_dep_line = format!(" {name} ({requirement})!"); + let new_dep_line = format!(" {name} (= {version})!"); let mut old_dep_line: Option = None; let mut insert_at = dep_start + 1; let mut existing_idx: Option = None; @@ -3153,12 +3153,13 @@ mod tests { } /// #985: on bundler 2.7+ a `gemspec` development dependency on the gem - /// merges into the vendored `gem` line after the directive, so the lock - /// pins the union bundler writes (`rack (>= 3, = 3.2.6)!`); the bare - /// exact pin fails the frozen install vendored mode promises. A gemspec - /// that cannot be read refuses before any write. + /// merges into a vendored `gem` line after the directive, and bundler + /// then rejects even its own lock for the path gem under frozen mode, so + /// vendoring refuses before any write and names the remedy. A `gem` + /// line before the directive, or bundler < 2.7, keeps the exact pin + /// and vendors. #[tokio::test] - async fn gemspec_dev_dependency_merges_into_the_lock_pin() { + async fn gemspec_dev_dependency_merge_refuses_before_any_write() { let gemfile = "source \"https://rubygems.org\"\n\ngemspec\n\ngem \"puma\"\ngem \"rack\", \"~> 3.1\"\n"; let lock = LOCK_DIRECT .replace( @@ -3168,8 +3169,43 @@ mod tests { .replace("2.5.22", "2.7.2"); let spec = "Gem::Specification.new do |s|\n s.name = \"mylib\"\n \ s.add_development_dependency \"rack\", \">= 3\"\nend\n"; + let refusal = |outcome: &VendorOutcome| match outcome { + VendorOutcome::Refused { code, detail } => Some((*code, detail.clone())), + _ => None, + }; - let (_tmp, root, installed, blobs, record) = fixture(gemfile, &lock).await; + for extra in [None, Some("other.gemspec")] { + let (_tmp, root, installed, blobs, record) = fixture(gemfile, &lock).await; + tokio::fs::write(root.join("mylib.gemspec"), spec) + .await + .unwrap(); + if let Some(extra) = extra { + // Two gemspecs, no `name:`: what bundler loads is unknown. + tokio::fs::write(root.join(extra), spec).await.unwrap(); + } + let outcome = run_vendor(&root, &blobs, &installed, &record, false).await; + let (code, detail) = refusal(&outcome).unwrap_or_else(|| panic!("{outcome:?}")); + assert_eq!(code, "gemfile_declaration_not_editable"); + assert!(detail.contains("above `gemspec`"), "{detail}"); + assert_eq!( + tokio::fs::read_to_string(root.join(GEMFILE_LOCK)) + .await + .unwrap(), + lock + ); + assert_eq!( + tokio::fs::read_to_string(root.join(GEMFILE)).await.unwrap(), + gemfile + ); + } + + // The remedy: declared before the directive, the exact pin vendors. + let before = "source \"https://rubygems.org\"\n\ngem \"puma\"\ngem \"rack\", \"~> 3.1\"\n\ngemspec\n"; + let (_tmp, root, installed, blobs, record) = fixture( + before, + &lock.replace("rack (~> 3.1, >= 3)", "rack (~> 3.1)"), + ) + .await; tokio::fs::write(root.join("mylib.gemspec"), spec) .await .unwrap(); @@ -3179,12 +3215,9 @@ mod tests { let wired = tokio::fs::read_to_string(root.join(GEMFILE_LOCK)) .await .unwrap(); - assert!( - wired.contains("\nDEPENDENCIES\n mylib!\n puma\n rack (>= 3, = 3.2.6)!\n"), - "{wired}" - ); + assert!(wired.contains("\n rack (= 3.2.6)!\n"), "{wired}"); - // Bundler < 2.7 keeps the exact pin. + // Bundler < 2.7 never merges. let (_tmp, root, installed, blobs, record) = fixture(gemfile, &lock.replace("2.7.2", "2.6.9")).await; tokio::fs::write(root.join("mylib.gemspec"), spec) @@ -3197,30 +3230,6 @@ mod tests { .await .unwrap(); assert!(wired.contains("\n rack (= 3.2.6)!\n"), "{wired}"); - - // Two gemspecs and no `name:`: bundler's choice is unknown, refuse. - let (_tmp, root, installed, blobs, record) = fixture(gemfile, &lock).await; - tokio::fs::write(root.join("mylib.gemspec"), spec) - .await - .unwrap(); - tokio::fs::write(root.join("other.gemspec"), spec) - .await - .unwrap(); - let outcome = run_vendor(&root, &blobs, &installed, &record, false).await; - assert!( - matches!(&outcome, VendorOutcome::Refused { code, .. } if *code == "gemfile_declaration_not_editable"), - "{outcome:?}" - ); - assert_eq!( - tokio::fs::read_to_string(root.join(GEMFILE_LOCK)) - .await - .unwrap(), - lock - ); - assert_eq!( - tokio::fs::read_to_string(root.join(GEMFILE)).await.unwrap(), - gemfile - ); } #[tokio::test] @@ -4621,7 +4630,7 @@ mod tests { SPIKE_RACK_SHA_LINE, &format!("{SPIKE_RACK_SHA_LINE}\n{SPIKE_RACK_SHA_LINE}"), ); - let err = match edit_lock(&lock, "rack", "3.1.8", "= 3.1.8", ©_rel_318()) { + let err = match edit_lock(&lock, "rack", "3.1.8", ©_rel_318()) { Err(e) => e, Ok(_) => panic!("duplicate CHECKSUMS entries must fail closed"), }; @@ -4632,7 +4641,7 @@ mod tests { fn test_no_checksums_lock_records_no_checksum_wiring() { // A lock WITHOUT a CHECKSUMS section must keep producing // the exact pre-CHECKSUMS output and no checksum record. - let edit = edit_lock(LOCK_DIRECT, "rack", "3.2.6", "= 3.2.6", ©_rel()).unwrap(); + let edit = edit_lock(LOCK_DIRECT, "rack", "3.2.6", ©_rel()).unwrap(); assert!(edit.checksum_rewrite.is_none()); assert_eq!(edit.text, expected_lock_direct()); } @@ -5089,7 +5098,7 @@ mod tests { fn test_gem_specs_platform_sibling_fails_closed() { let lock = "GEM\n remote: https://rubygems.org/\n specs:\n nokogiri (1.16.0)\n racc (~> 1.4)\n nokogiri (1.16.0-arm64-darwin)\n racc (~> 1.4)\n\nPLATFORMS\n arm64-darwin\n ruby\n\nDEPENDENCIES\n nokogiri\n\nBUNDLED WITH\n 2.5.22\n"; let rel = format!(".socket/vendor/gem/{UUID}/nokogiri-1.16.0"); - let err = match edit_lock(lock, "nokogiri", "1.16.0", "= 1.16.0", &rel) { + let err = match edit_lock(lock, "nokogiri", "1.16.0", &rel) { Err(e) => e, Ok(_) => panic!("a platform-suffixed GEM specs sibling must fail closed"), }; @@ -7626,7 +7635,7 @@ mod tests { fn edit_lock_missing_specs_stanza_fails_closed() { let lock = LOCK_DIRECT.replace(" specs:\n", ""); assert_ne!(lock, LOCK_DIRECT); - let err = edit_lock(&lock, "rack", "3.2.6", "= 3.2.6", ©_rel()) + let err = edit_lock(&lock, "rack", "3.2.6", ©_rel()) .err() .expect("a specs:-less GEM section must fail closed"); assert!(err.contains("no specs: stanza"), "{err}"); @@ -7643,7 +7652,7 @@ mod tests { "PATH\n remote: {rel}\n specs:\n other (1.0)\n{tail}", rel = copy_rel() ); - let err = edit_lock(&lost, "rack", "3.2.6", "= 3.2.6", ©_rel()) + let err = edit_lock(&lost, "rack", "3.2.6", ©_rel()) .err() .expect("a PATH section without its spec entry must fail closed"); assert!(err.contains("lost its"), "{err}"); @@ -7652,7 +7661,7 @@ mod tests { "PATH\n remote: {rel}\n specs:\n rack (3.2.6)\n hand: edit\n{tail}", rel = copy_rel() ); - let err = edit_lock(&edited, "rack", "3.2.6", "= 3.2.6", ©_rel()) + let err = edit_lock(&edited, "rack", "3.2.6", ©_rel()) .err() .expect("a hand-edited PATH section must fail closed"); assert!(err.contains("not the shape vendor wrote"), "{err}"); @@ -7666,7 +7675,7 @@ mod tests { let git = "GIT\n remote: https://example.com/dep.git\n revision: abc123\n specs:\n dep (1.0)\n\n"; let lock = format!("{git}{LOCK_DIRECT}"); - let edit = edit_lock(&lock, "rack", "3.2.6", "= 3.2.6", ©_rel()).unwrap(); + let edit = edit_lock(&lock, "rack", "3.2.6", ©_rel()).unwrap(); assert_eq!(edit.text, format!("{git}{}", expected_lock_direct())); } @@ -8594,7 +8603,7 @@ mod tests { /// "GEM specs has no entry" although the lock lists it. #[test] fn edit_lock_lifts_spec_from_second_gem_section() { - let edit = edit_lock(LOCK_TWO_SOURCES, "rack", "3.2.6", "= 3.2.6", ©_rel()).unwrap(); + let edit = edit_lock(LOCK_TWO_SOURCES, "rack", "3.2.6", ©_rel()).unwrap(); assert_eq!(edit.text, expected_lock_two_sources()); assert!(!edit.rewired_ours); assert_eq!( @@ -8611,7 +8620,7 @@ mod tests { /// recorded, so ledgers written before #779 and after it agree. #[test] fn edit_lock_first_gem_section_records_no_source_remote() { - let edit = edit_lock(LOCK_DIRECT, "rack", "3.2.6", "= 3.2.6", ©_rel()).unwrap(); + let edit = edit_lock(LOCK_DIRECT, "rack", "3.2.6", ©_rel()).unwrap(); assert_eq!(edit.source_remote, None); let (original, _new) = lock_record_lines(&edit); assert!(original.iter().all(|l| !l.starts_with(" remote: "))); @@ -8626,7 +8635,7 @@ mod tests { " aaa-internal (1.0.0)\n", " aaa-internal (1.0.0)\n rack (3.2.6)\n", ); - let err = edit_lock(&lock, "rack", "3.2.6", "= 3.2.6", ©_rel()) + let err = edit_lock(&lock, "rack", "3.2.6", ©_rel()) .err() .expect("a spec listed in two GEM sections must fail closed"); assert!(err.contains("more than one GEM section"), "{err}"); @@ -8640,7 +8649,7 @@ mod tests { " aaa-internal (1.0.0)\n", " aaa-internal (1.0.0)\n rack (3.2.6-x86_64-linux)\n", ); - let err = edit_lock(&lock, "rack", "3.2.6", "= 3.2.6", ©_rel()) + let err = edit_lock(&lock, "rack", "3.2.6", ©_rel()) .err() .expect("a platform sibling in any GEM section must fail closed"); assert!(err.contains("platform-suffixed"), "{err}"); @@ -8703,7 +8712,7 @@ mod tests { /// later revert still targets the second GEM section. #[test] fn revert_lock_text_targets_recorded_gem_section() { - let edit = edit_lock(LOCK_TWO_SOURCES, "rack", "3.2.6", "= 3.2.6", ©_rel()).unwrap(); + let edit = edit_lock(LOCK_TWO_SOURCES, "rack", "3.2.6", ©_rel()).unwrap(); let (original, new) = lock_record_lines(&edit); assert_eq!( revert_lock_text(&edit.text, &original, &new).as_deref(), From a638242fa1ee4ee9a40c3722258fc397031f0860 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 21:17:31 +0000 Subject: [PATCH 4/5] Refuse the gemspec-merge takeover up front The hosted-to-vendored takeover asks the gem backend's refusals before it restores the hosted pin, so a gem vendored mode cannot wire keeps its hosted wiring. That preflight did not know about the new gemspec development dependency refusal: a takeover in that shape restored the gem upstream and then refused, leaving it unpatched in both modes. The preflight now runs the same check, so the takeover (and its dry-run preview) refuses with the hosted pin untouched. The refusal now names the remedy only when Bundler would merge the requirements; when the gemspec cannot be read it says so instead. Refs #985 Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 2 +- .../tests/e2e_redirect_gem_build.rs | 45 ++++++++-- crates/socket-patch-core/src/vendor/gem.rs | 88 ++++++++++++------- 3 files changed, 98 insertions(+), 37 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index f0e0c473f..fe6de7596 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -934,7 +934,7 @@ to **six flavors**. | cargo | crate dir `-/` (no `.cargo-checksum.json`) | (v5.0) `[patch.crates-io]` path entry in the **workspace-root `Cargo.toml`** (the manifest beside the `Cargo.lock` it detaches — never `.cargo/config*`) **+** Cargo.lock surgery (the `[[package]]` entry's `source`/`checksum` removed and its `version` set to the copy's TAGGED version `+socket.` — `+.socket.` when the version already has build metadata — with every lock reference that spells the old version rewritten, formats v1–v4; the copy's own `Cargo.toml` version carries the same tag, so the patched crate sees it in `CARGO_PKG_VERSION`; revert restores the lock byte for byte). Key: always the Socket-owned `-socket-` with `package = ""` (the full uuid hex when that key is taken), never the bare crate name — cargo lets a config-file `[patch]` item (project, ancestor directory or `$CARGO_HOME`) replace the manifest item with the same key whatever its version, so keys any of those configs use are avoided and a re-run moves an entry off a now-shadowed key; two versions of one crate are wired side by side. Pre-v5 wiring in `.cargo/config.toml` / `.cargo/config` is moved into `Cargo.toml` by a re-run (`vendor`, `scan`/`get --mode vendored`) or `repair` (`cargo_wiring_migrated` note; the ledger's `cargo_patch_entry` record then names `Cargo.toml`); a detached lock entry left unwired by the pre-v5 multi-version overwrite is re-wired the same way (`cargo_wiring_restored`); every revert removes both spellings | `cargo build --locked --offline` on a fresh checkout — single-version manifest `[patch]` also builds with no network on cargo older than 1.56 (the old config-file wiring's floor); two vendored versions of ONE crate need cargo 1.45 or newer (`--offline` from an empty CARGO_HOME is enough there); older cargo fails closed whatever the index state, and a project that does not pin cargo ≥ 1.45 (`rust-version`, a `rust-version.workspace = true` inheritor's `[workspace.package]` value, or toolchain file) gets the `cargo_multi_version_old_cargo` warning. Note: path deps build **without** `--cap-lints allow` | | golang | module dir `@/` | `go.mod` `replace => ./.socket/vendor/golang//@` | `go build` with `GOPROXY=off` + empty `GOMODCACHE` (directory replaces bypass go.sum entirely; survives `go mod tidy`) | | composer | package dir `/@/`; the copy's `.gitignore` / `.hgignore` are emptied and its `.gitattributes` `export-ignore` rules dropped, because Composer's path mirror skips the files they match (`vendor_composer_mirror_filters_neutralized`; a patch that rewrites one of them is refused `vendor_composer_mirror_filter_conflict`). Re-runs heal copies vendored before this | `composer.lock` only: entry's `dist` → `{type: "path", url, reference: ""}`, `source` removed, `transport-options: {symlink: false}` added. `content-hash` unaffected; `composer.json` untouched | `composer install` (from the lock alone, real copy not symlink, works under `--network none`). Composer 1 does not reinstall an already-installed package whose dist changed: remove `//` first (the hint names the project's resolved vendor dir: `COMPOSER_VENDOR_DIR`, else `config.vendor-dir`, else `vendor`). `composer update ` reverts it. See `docs/testing/composer-compatibility.md` | -| gem | gem dir `-/` + gemspec materialized from `specifications/` | **Gemfile + Gemfile.lock pair**: the `gem` line gains `path:` (or a managed block for transitive deps); the lock's spec block moves GEM→PATH and the DEPENDENCIES entry becomes ` (= )!`, in bundler's exact canonical form. On bundler 2.7+ (the lock's `BUNDLED WITH`), a `gem` line that follows a `gemspec` directive whose gemspec declares a development dependency on the same gem is refused `gemfile_declaration_not_editable` before any write: bundler merges the two requirements and then rejects even its own lock for a path gem under frozen mode, so the refusal names the remedy (declare the gem above `gemspec`, or use hosted mode). Hosted mode instead pins the merged requirement bundler writes (` (>= 0, = )!`), and warns `redirect_gem_gemspec_dependency_unreadable` when the gemspec cannot be read (#985) | `bundle install` (normal **and** `BUNDLE_FROZEN=true`), byte-stable lock. Lock-only edits are a silent unpatch — hence the mandatory pair | +| gem | gem dir `-/` + gemspec materialized from `specifications/` | **Gemfile + Gemfile.lock pair**: the `gem` line gains `path:` (or a managed block for transitive deps); the lock's spec block moves GEM→PATH and the DEPENDENCIES entry becomes ` (= )!`, in bundler's exact canonical form. On bundler 2.7+ (the lock's `BUNDLED WITH`), a `gem` line that follows a `gemspec` directive whose gemspec declares a development dependency on the same gem is refused `gemfile_declaration_not_editable` before any write: bundler merges the two requirements and then rejects even its own lock for a path gem under frozen mode, so the refusal names the remedy (declare the gem above `gemspec`, or use hosted mode). Hosted mode instead pins the merged requirement bundler writes: the gemspec's requirements plus `= `, in bundler's order (` (>= 0, = )!` for an unconstrained development dependency), and warns `redirect_gem_gemspec_dependency_unreadable` when the gemspec cannot be read (#985) | `bundle install` (normal **and** `BUNDLE_FROZEN=true`), byte-stable lock. Lock-only edits are a silent unpatch — hence the mandatory pair | | pypi / uv (uv.lock) | rebuilt wheel (canonical PEP 427 filename; RECORD regenerated) | `[tool.uv.sources] = {path}` in pyproject + surgical uv.lock rewrite; transitive deps via `[tool.uv] override-dependencies` | `uv sync --locked` / `--frozen --offline` (hash-verified, byte-stable lock) | | pypi / poetry (poetry.lock: legacy `[metadata.hashes]`, lock 1.0/1.1 `[metadata.files]`, 2.x `files`) | (rebuilt wheel) | lock-only: the target `[[package]]` gets `[package.source] type="file"` (+ `reference = ""` on the 0.12/1.0 layouts, which read it unconditionally) and the single `{file, hash: sha256-of-our-wheel}` entry in whichever table the generation keeps it. pyproject + `metadata.content-hash` untouched; CRLF locks keep their line endings. A lock written by Poetry < 1.4 emits `pypi_poetry_integrity_unverified` (that installer verifies no local hashes and skips an already-installed version) | `poetry check --lock && poetry sync`, cold cache (hash fail-closed from Poetry 1.4; byte-stable lock) — see `docs/testing/poetry-compatibility.md` | | pypi / pdm (pdm.lock) | (rebuilt wheel) | lock-only: the `[[package]]` gains the local-file `path` + `files[]` hash. pyproject + `content_hash` untouched. Non-fixture `[metadata] strategy` / hash-less locks refused | `pdm sync` (+ `pdm install --check`), cold cache | 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 d8d082b53..d7f382808 100644 --- a/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs +++ b/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs @@ -128,9 +128,6 @@ const GHSA: &str = "GHSA-redirect-gem-real"; const PRODUCT: &str = "pkg:gem/app@1.0.0"; const PURL: &str = "pkg:gem/vuln-gem@1.0.0"; -/// The runtime probe constant baked into the PATCHED lib — observable at -/// `require` time, carries the patch uuid so the assert can't pass on any -/// other content. /// The gem library a [`Driver::ScanVexGemspecDevDependency`] project /// builds: its gemspec declares the vulnerable gem as a development /// dependency. @@ -144,6 +141,9 @@ fn gemspec_lib() -> String { ) } +/// The runtime probe constant baked into the PATCHED lib — observable at +/// `require` time, carries the patch uuid so the assert can't pass on any +/// other content. fn patched_marker() -> String { format!("PATCHED-{UUID}") } @@ -972,7 +972,7 @@ async fn redirect_scanned_project( std::fs::write(proj.join("lib/mylib.rb"), "module Mylib; end\n").unwrap(); std::fs::write(proj.join(GEMSPEC_LIB), gemspec_lib()).unwrap(); format!( - "source \"{}/upstream\"\n\ngemspec\n\ngroup :development do\n gem \"{DEP}\"\nend\n", + "source \"{}/upstream\"\n\ngemspec\n\ngem \"{DEP}\"\n", server.uri() ) } @@ -3067,7 +3067,10 @@ async fn gem_hosted_trailing_semicolon_declaration_redirects_and_installs() { /// redirected lock must pin the union (`(>= 0, = 1.0.0)!`): a fresh /// checkout's FROZEN install accepts the pair unchanged and installs the /// patched bytes (before the fix it exited 16). Older bundlers keep the -/// plain exact pin, which they accept frozen too. +/// plain exact pin, which they accept frozen too. A vendored takeover, which +/// bundler 2.7+ cannot frozen-install in this shape (the vendored backend +/// refuses it), must refuse before it restores the hosted pin, in the +/// preview too, or the gem ends up unpatched in both modes. #[tokio::test(flavor = "multi_thread")] #[ignore = "host capstone: shells out to a real ruby/gem/bundler (>= 2.6); \ run with a pinned toolchain via --ignored"] @@ -3105,6 +3108,38 @@ async fn gem_hosted_gemspec_dev_dependency_frozen_install_accepts_the_pin() { "the frozen install must leave the lock byte-identical" ); assert_patched_install(&fx, &fresh); + if fx.bundler.at_least(2, 7) { + // As in the #775 takeover test: the restore re-derives the upstream + // sha256 only for a rubygems.org remote, so spell the mock upstream + // that way (served via `SOCKET_RUBYGEMS_URL`). No bundler runs + // after this point. + let upstream = format!("{}/upstream", fx._server.uri()); + for (file, from, to) in [ + ( + "Gemfile", + format!("\"{upstream}\""), + "\"https://rubygems.org\"", + ), + ( + "Gemfile.lock", + format!("{upstream}/"), + "https://rubygems.org/", + ), + ] { + let path = fx.proj.join(file); + let text = std::fs::read_to_string(&path).unwrap(); + assert!( + text.contains(&from), + "{file} names the mock upstream:\n{text}" + ); + std::fs::write(&path, text.replace(&from, to)).unwrap(); + } + for (command, selector) in [("get", Some(UUID)), ("scan", None)] { + for dry_run in [true, false] { + vendor_takeover_keeps_the_hosted_group_pin(&fx, command, selector, dry_run); + } + } + } } /// #340: a `gem` declaration with an `if` modifier must not be rewritten diff --git a/crates/socket-patch-core/src/vendor/gem.rs b/crates/socket-patch-core/src/vendor/gem.rs index 39634af91..0d46e9929 100644 --- a/crates/socket-patch-core/src/vendor/gem.rs +++ b/crates/socket-patch-core/src/vendor/gem.rs @@ -185,7 +185,8 @@ pub async fn gem_manifest_refusal(project_root: &Path) -> Option<(&'static str, /// The per-gem twin of [`gem_manifest_refusal`] for the hosted→vendored /// takeover: the backend's Gemfile declaration gate ([`plan_gemfile_edit`] /// and [`refuse_append_of_direct_dependency`], refused as -/// `gemfile_declaration_not_editable`) for `purl`. Evaluated on the files +/// `gemfile_declaration_not_editable`, with [`gemspec_merge_refusal`]) for +/// `purl`. Evaluated on the files /// as the takeover's restore of `pin` will leave them: a dry-run /// [`restore_upstream`] supplies the restored `Gemfile` / `Gemfile.lock` /// text, so the hosted `source … do` block socket-patch itself wrote is @@ -235,6 +236,11 @@ pub async fn gem_vendor_target_preflight( }; plan_gemfile_edit(&gemfile_text, &name, &version, ©_rel) .and_then(|plan| refuse_append_of_direct_dependency(plan, &lock_text, &name)) + .and_then(|plan| { + let after = apply_gemfile_plan(&gemfile_text, &plan); + gemspec_merge_refusal(project_root, &after, &lock_text, &name, &version) + .map_or(Ok(()), Err) + }) .err() .map(|detail| ("gemfile_declaration_not_editable", detail)) } @@ -486,42 +492,17 @@ fn gem_edits( } }; // ── gemspec development dependency (#985) ──────────────────────────── - // Bundler 2.7+ folds a `gemspec` development dependency on the gem into - // a `gem` line that follows the directive. For a path gem bundler then - // rejects even the lock it writes itself under frozen/deployment mode - // ("rack from `` to ``", verified on 2.7.2 and 4.0.18), so - // no lock this backend could write keeps vendored mode's frozen-install - // promise. Refuse with the remedy: declared before the directive, the - // gem keeps its own requirement and installs frozen. Read on the - // Gemfile as the plan leaves it. let project_root = prelude.gemfile_path.parent().unwrap_or(Path::new(".")); - let merged = crate::formats::gem::gemspec::locked_exact_requirement( + if let Some(detail) = gemspec_merge_refusal( + project_root, &apply_gemfile_plan(&prelude.gemfile_text, &plan), &prelude.lock_text, name, version, - &mut |dir| gemspec_listing(project_root, dir), - ); - let refusal = match merged { - Ok(requirement) if requirement == format!("= {version}") => None, - Ok(requirement) => Some(format!( - "the gemspec the Gemfile's `gemspec` directive loads declares a development \ - dependency on `{name}`, so bundler 2.7+ locks the vendored gem as `{name} \ - ({requirement})!` and then rejects that lock under frozen/deployment mode (a \ - bundler limitation for path gems)" - )), - Err(why) => Some(format!( - "bundler 2.7+ merges a gemspec development dependency on `{name}` into the \ - vendored gem's requirement, and {why}" - )), - }; - if let Some(why) = refusal { + ) { return Err(Box::new(refused( "gemfile_declaration_not_editable", - format!( - "{why}; move the `gem \"{name}\"` declaration above `gemspec` in the Gemfile \ - (bundler then keeps its own requirement), or use hosted mode" - ), + detail, ))); } // ── Gemfile.lock edit (pure text surgery, computed before any write) ── @@ -1694,6 +1675,46 @@ fn lock_record_lines(edit: &LockEdit) -> (Vec, Vec) { (original, new) } +/// Why vendored mode must not wire `name`@`version` into the Gemfile as +/// `plan` leaves it (`gemfile_after`), if it must not (#985). Bundler 2.7+ +/// folds a `gemspec` development dependency on the gem into a `gem` line +/// that follows the directive. For a path gem bundler then rejects even +/// the lock it writes itself under frozen/deployment mode ("rack from +/// `` to ``", verified on 2.7.2 and 4.0.18), so no lock this +/// backend could write keeps vendored mode's frozen-install promise. +/// Declared before the directive, the gem keeps its own requirement and +/// installs frozen: the refusal names that remedy. +fn gemspec_merge_refusal( + project_root: &Path, + gemfile_after: &str, + lock: &str, + name: &str, + version: &str, +) -> Option { + match crate::formats::gem::gemspec::locked_exact_requirement( + gemfile_after, + lock, + name, + version, + &mut |dir| gemspec_listing(project_root, dir), + ) { + Ok(requirement) if requirement == format!("= {version}") => None, + Ok(requirement) => Some(format!( + "the gemspec the Gemfile's `gemspec` directive loads declares a development \ + dependency on `{name}`, so bundler 2.7+ locks the vendored gem as `{name} \ + ({requirement})!` and then rejects that lock under frozen/deployment mode (a \ + bundler limitation for path gems); move the `gem \"{name}\"` declaration above \ + `gemspec` in the Gemfile (bundler then keeps its own requirement), or use hosted \ + mode" + )), + Err(why) => Some(format!( + "cannot tell whether bundler 2.7+ merges a gemspec development dependency on \ + `{name}` into the vendored requirement ({why}), so a frozen install of the \ + vendored pair is not guaranteed; use hosted mode" + )), + } +} + /// The `*.gemspec` files of the root-relative directory `dir` under /// `project_root`, for [`crate::formats::gem::gemspec::locked_exact_requirement`]. fn gemspec_listing(project_root: &Path, dir: &str) -> crate::formats::gem::gemspec::GemspecListing { @@ -3186,7 +3207,12 @@ mod tests { let outcome = run_vendor(&root, &blobs, &installed, &record, false).await; let (code, detail) = refusal(&outcome).unwrap_or_else(|| panic!("{outcome:?}")); assert_eq!(code, "gemfile_declaration_not_editable"); - assert!(detail.contains("above `gemspec`"), "{detail}"); + let want = if extra.is_some() { + "cannot tell" + } else { + "above `gemspec`" + }; + assert!(detail.contains(want), "{detail}"); assert_eq!( tokio::fs::read_to_string(root.join(GEMFILE_LOCK)) .await From fb0db48f843b43b816ad092514b23defcc7ff270 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 21:55:35 +0000 Subject: [PATCH 5/5] Read %q() and %q[] gem names as mentions A gemspec development dependency spelled `%q(name)` or `%q[name]` with arguments the reader cannot parse was treated as absent rather than unreadable, so hosted mode wrote the bare exact pin without a warning and vendored mode skipped its refusal. Every bracket pair the literal reader accepts now counts as a mention. Refs #985 Assisted-by: Claude Code:claude-opus-5-5 --- .../src/formats/gem/gemspec.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/crates/socket-patch-core/src/formats/gem/gemspec.rs b/crates/socket-patch-core/src/formats/gem/gemspec.rs index 29d16c692..0197dc26a 100644 --- a/crates/socket-patch-core/src/formats/gem/gemspec.rs +++ b/crates/socket-patch-core/src/formats/gem/gemspec.rs @@ -243,10 +243,18 @@ pub(crate) fn dev_dependency_requirements( continue; }; let after = &trimmed[at + "add_development_dependency".len()..]; - let mentions = after.contains(&format!("\"{gem}\"")) - || after.contains(&format!("'{gem}'")) - || after.contains(&format!("<{gem}>")) - || after.contains(&format!("{{{gem}}}")); + // Every literal spelling [`literal`] reads: quoted, or `%q` with any + // of its bracket pairs. + let mentions = [ + ('"', '"'), + ('\'', '\''), + ('<', '>'), + ('{', '}'), + ('(', ')'), + ('[', ']'), + ] + .iter() + .any(|(open, close)| after.contains(&format!("{open}{gem}{close}"))); let unreadable = || format!("line {}: `{trimmed}`", i + 1); let Some(reqs) = read_dev_call(after) else { if mentions { @@ -552,6 +560,8 @@ mod tests { "s.add_development_dependency \"vuln-gem\", VERSION\n", "s.add_development_dependency \"vuln-gem\",\n \"~> 1.0\"\n", "s.add_development_dependency \"vuln-gem\", \"~> 1\"\ns.add_development_dependency \"vuln-gem\", \"~> 2\"\n", + "s.add_development_dependency(%q(vuln-gem), VERSION)\n", + "s.add_development_dependency(%q[vuln-gem].freeze, [VERSION])\n", ] { let text: &'static str = Box::leak(spec.to_string().into_boxed_str()); assert!(