Fix gem pins breaking frozen installs with gemspec dev deps (#985)

crates/socket-patch-cli/CLI_CONTRACT.md

@@ -934,7 +934,7 @@ to **six flavors**.
 | cargo | crate dir `<name>-<version>/` (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 `<version>+socket.<uuid>` — `<core>+<meta>.socket.<uuid>` 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 `<name>-socket-<first 8 hex of the uuid>` with `package = "<name>"` (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 `<module>@<version>/` | `go.mod` `replace <module> <ver> => ./.socket/vendor/golang/<uuid>/<module>@<ver>` | `go build` with `GOPROXY=off` + empty `GOMODCACHE` (directory replaces bypass go.sum entirely; survives `go mod tidy`) |
 | composer | package dir `<vendor>/<name>@<version>/`; 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: "<patch-uuid>"}`, `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 `<vendor-dir>/<vendor>/<name>` first (the hint names the project's resolved vendor dir: `COMPOSER_VENDOR_DIR`, else `config.vendor-dir`, else `vendor`). `composer update <pkg>` reverts it. See `docs/testing/composer-compatibility.md` |
-| gem | gem dir `<name>-<version>/` + 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 `<name> (= <ver>)!`, 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 `<name>-<version>/` + 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 `<name> (= <ver>)!`, 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 `= <ver>`, in bundler's order (`<name> (>= 0, = <ver>)!` 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] <name> = {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 |

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")]