diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index e2bc9b3d9..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 | `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 64d7229ba..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,6 +128,19 @@ 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 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" + ) +} + /// 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. @@ -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\ngem \"{DEP}\"\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,87 @@ 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. 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"] +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); + 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 /// (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/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..0197dc26a --- /dev/null +++ b/crates/socket-patch-core/src/formats/gem/gemspec.rs @@ -0,0 +1,589 @@ +//! 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()..]; + // 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 { + 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", + "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!( + 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..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)) } @@ -485,6 +491,20 @@ fn gem_edits( ))) } }; + // ── gemspec development dependency (#985) ──────────────────────────── + let project_root = prelude.gemfile_path.parent().unwrap_or(Path::new(".")); + if let Some(detail) = gemspec_merge_refusal( + project_root, + &apply_gemfile_plan(&prelude.gemfile_text, &plan), + &prelude.lock_text, + name, + version, + ) { + return Err(Box::new(refused( + "gemfile_declaration_not_editable", + detail, + ))); + } // ── Gemfile.lock edit (pure text surgery, computed before any write) ── // A lock-shape failure therefore costs no download / copy / patch and no // Gemfile write. @@ -1655,6 +1675,69 @@ 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 { + 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 @@ -3090,6 +3173,91 @@ mod tests { ); } + /// #985: on bundler 2.7+ a `gemspec` development dependency on the gem + /// 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_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( + " 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 refusal = |outcome: &VendorOutcome| match outcome { + VendorOutcome::Refused { code, detail } => Some((*code, detail.clone())), + _ => None, + }; + + 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"); + 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 + .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(); + 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}"); + + // 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) + .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}"); + } + #[tokio::test] async fn test_direct_dep_happy_path() { let (_tmp, root, installed, blobs, record) = fixture(GEMFILE_DIRECT, LOCK_DIRECT).await;