From 44275bdf61374550e4bd5f57128729406bfeebd9 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 11:30:15 +0000 Subject: [PATCH 1/6] Start fix for #482, #548 Assisted-by: Claude Code:claude-opus-5-5 From 94270b48a6e12e2e0545e465b862ee81ada6a6a0 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 11:47:52 +0000 Subject: [PATCH 2/6] Stop Gemfile rewrites declaring a gem twice Hosted and vendored modes could leave a Gemfile that Bundler refuses on every install ("You cannot specify the same gem twice"): - Hosted mode rewrote only the first of several declarations of a gem (for example one in each of two `group` blocks), leaving an exact pin next to the original requirement (#548). It now refuses with redirect_gem_declared_more_than_once, as vendored mode already did. - Both modes treated a gem they could not see declared in the root Gemfile as transitive and appended a second declaration, even when the lock lists it under DEPENDENCIES because the Gemfile declares it through eval_gemfile or a loop (#482). Both now refuse instead. Fixes #482 Fixes #548 Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-core/src/formats/gem/mod.rs | 20 +++ .../src/patch/redirect/mod.rs | 147 ++++++++++++++++-- crates/socket-patch-core/src/vendor/gem.rs | 57 ++++++- 3 files changed, 213 insertions(+), 11 deletions(-) diff --git a/crates/socket-patch-core/src/formats/gem/mod.rs b/crates/socket-patch-core/src/formats/gem/mod.rs index 627b5125f..3c345cda8 100644 --- a/crates/socket-patch-core/src/formats/gem/mod.rs +++ b/crates/socket-patch-core/src/formats/gem/mod.rs @@ -114,6 +114,10 @@ pub(crate) struct GemfileLock<'t> { pub(crate) checksums: Option>>, /// `DEPENDENCIES` entries bundler marks source-pinned (`name …!`). pub(crate) pinned: BTreeSet<&'t str>, + /// Every `DEPENDENCIES` entry: the gems bundler treats as DIRECT + /// dependencies, however the Gemfile declares them (`eval_gemfile`, a + /// loop, a `gemspec` development dependency, a plain `gem` line). + pub(crate) direct: BTreeSet<&'t str>, /// Why bundler would refuse this lock, first problem first. pub(crate) problems: Vec, } @@ -222,6 +226,7 @@ pub(crate) fn parse(text: &str) -> GemfileLock<'_> { let mut in_checksums = false; let mut in_dependencies = false; let mut pinned: BTreeSet<&str> = BTreeSet::new(); + let mut direct: BTreeSet<&str> = BTreeSet::new(); let mut seen_header = false; let mut bundler_shaped = false; @@ -281,6 +286,10 @@ pub(crate) fn parse(text: &str) -> GemfileLock<'_> { _ => {} } } else if in_dependencies && indent == 2 { + let name = trimmed.split([' ', '(', '!']).next().unwrap_or_default(); + if !name.is_empty() { + direct.insert(name); + } if let Some(entry) = trimmed.strip_suffix('!') { let name = entry.split([' ', '(']).next().unwrap_or_default(); if !name.is_empty() { @@ -306,10 +315,21 @@ pub(crate) fn parse(text: &str) -> GemfileLock<'_> { sections, checksums, pinned, + direct, problems, } } +/// Whether the Bundler lock `lock` lists `name` under `DEPENDENCIES`, i.e. +/// bundler resolved it as a DIRECT dependency of the Gemfile. The Gemfile +/// rewriters consult it before treating a gem they cannot see declared as +/// transitive: appending a declaration for a gem the Gemfile already +/// declares out of sight (`eval_gemfile`, a loop) leaves it declared twice, +/// and bundler refuses every install (#482). +pub(crate) fn lock_lists_direct_dependency(lock: &str, name: &str) -> bool { + parse(lock).direct.contains(name) +} + /// The plain gem-token charset (letters, digits, `.`, `_`, `-`). The vendor /// backend applies it before embedding coordinates into Ruby source and lock /// line grammar (see the SECURITY note in `crate::vendor::gem`'s diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index 3fdda3257..7b292ee18 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -51,6 +51,7 @@ use crate::formats::pnpm::plan_hosted; use crate::formats::cargo::CargoLock; use crate::formats::composer::hosted::rewrite_composer_lock; use crate::formats::gem::hosted::{checksum_entry_span, converge_gem_lock_source}; +use crate::formats::gem::lock_lists_direct_dependency; pub(crate) use crate::formats::yarn::is_berry_lock; use crate::formats::cargo::hosted::CargoLockPlan; #[cfg(test)] @@ -4945,6 +4946,31 @@ fn rewrite_gem( // written or already present) — the lock pin below is gated on it. let mut source_placed = false; if let Some(gf) = gemfile.as_mut() { + // Looser "declared at all?" probe: counts every `gem` call that + // names the gem, in any form (indented in a group, parenthesized, + // our own source block). It gates the append branch (appending + // next to a declaration the recognizer below cannot parse would + // leave the gem declared twice) and catches a gem declared more + // than once: rewriting only one of them leaves `= x.y.z` next to + // the other requirement, and bundler refuses the Gemfile (#548). + let declared_re = Regex::new( + &(String::from(r#"(?m)^[ \t]*gem\b[^\n]*["']"#) + + ®ex::escape(&dep.name) + + r#"["']"#), + ) + .expect("declaration probe regex from the escaped gem name is valid"); + if declared_re.find_iter(gf).nth(1).is_some() { + result.warnings.push(RewriteWarning { + code: "redirect_gem_declared_more_than_once".into(), + detail: format!( + "`gem \"{}\"` is declared more than once in {gemfile_name}; \ + rewriting one declaration would leave conflicting requirements \ + bundler refuses — merge them into one declaration and re-run", + dep.name + ), + }); + continue; + } // Grant-agnostic idempotency guard: the grant-token (and patch // uuid) segments of the index URL rotate per request, so an // exact-URL check misses the block a previous run wrote and this @@ -4994,16 +5020,6 @@ fn rewrite_gem( + r#"["']([^\n]*)$"#), ) .expect("gem-line regex from the escaped gem name is valid"); - // Looser "declared at all?" probe: gates the append branch — - // appending next to a declaration the recognizer above cannot - // parse would leave the gem declared twice (bundler - // hard-fails on the duplicate). - let declared_re = Regex::new( - &(String::from(r#"(?m)^[ \t]*gem\b[^\n]*["']"#) - + ®ex::escape(&dep.name) - + r#"["']"#), - ) - .expect("declaration probe regex from the escaped gem name is valid"); if let Some(m) = gem_line_re.captures(gf) { let range = m.get(0).expect("group 0 is the whole match").range(); let original = m @@ -5116,6 +5132,26 @@ fn rewrite_gem( ), }); continue; + } else if files + .get(lock_name) + .is_some_and(|lk| lock_lists_direct_dependency(lk, &dep.name)) + { + // Not declared where the rewriter can see it, yet bundler + // resolved it as a DIRECT dependency: the Gemfile declares + // it out of sight (`eval_gemfile`, a loop, a gemspec). + // Appending a block would declare it twice (#482). + result.warnings.push(RewriteWarning { + code: "redirect_gem_declaration_not_visible".into(), + detail: format!( + "{lock_name} lists {} as a direct dependency, but {gemfile_name} \ + declares it somewhere the rewriter cannot edit (an \ + `eval_gemfile`d file, a loop, a gemspec); appending a second \ + declaration would make bundler refuse the Gemfile — redirect \ + skipped", + dep.name + ), + }); + continue; } else { // Genuinely undeclared (a transitive dep): append a block. let block = format!( @@ -11042,6 +11078,97 @@ mod tests { ); } + /// #548: bundler accepts a gem declared more than once with the same + /// requirement (two `group` blocks, or top level plus a group). + /// Rewriting only the first declaration leaves `= 1.0.0` next to + /// `>= 0`, which bundler refuses on every install. Fail closed before + /// any write, like vendored mode's `gemfile_declaration_not_editable`. + #[test] + fn gemfile_gem_declared_twice_fails_closed() { + let lock = "GEM\n remote: https://rubygems.org/\n specs:\n vuln-gem (1.0.0)\n\n\ + PLATFORMS\n ruby\n\nDEPENDENCIES\n vuln-gem\n\n\ + CHECKSUMS\n vuln-gem (1.0.0) sha256=" + .to_string() + + &"2".repeat(64) + + "\n\nBUNDLED WITH\n 4.0.17\n"; + for gemfile in [ + "source \"https://rubygems.org\"\n\ngroup :development do\n gem \"vuln-gem\"\nend\n\n\ + group :test do\n gem \"vuln-gem\"\nend\n", + "source \"https://rubygems.org\"\n\ngem \"vuln-gem\"\n\n\ + group :test do\n gem \"vuln-gem\"\nend\n", + "source \"https://rubygems.org\"\n\ngem \"vuln-gem\"\ngem(\"vuln-gem\")\n", + ] { + let mut files = BTreeMap::new(); + files.insert("Gemfile".to_string(), gemfile.to_string()); + files.insert("Gemfile.lock".to_string(), lock.clone()); + let r = rewrite_registry_redirect(&files, &[gem_override("vuln-gem", "1.0.0")]); + assert!( + r.files.is_empty() && r.edits.is_empty(), + "a gem declared twice must not be half-rewritten: {gemfile}\nfiles={:?} edits={:?}", + r.files, + r.edits + ); + assert_eq!( + warning_codes(&r), + vec!["redirect_gem_declared_more_than_once"], + "{gemfile}: {:?}", + r.warnings + ); + } + } + + /// #482: a DIRECT dependency the root Gemfile declares out of the + /// rewriter's sight (`eval_gemfile`, a loop) is listed under the lock's + /// DEPENDENCIES. Appending a source block for it declares it twice and + /// bundler refuses every install, so fail closed instead. + #[test] + fn gemfile_direct_dependency_declared_out_of_sight_is_not_appended() { + let lock = "GEM\n remote: https://rubygems.org/\n specs:\n rack (3.1.8)\n\n\ + PLATFORMS\n ruby\n\nDEPENDENCIES\n rack (~> 3.1)\n\n\ + BUNDLED WITH\n 4.0.17\n"; + for gemfile in [ + "source \"https://rubygems.org\"\neval_gemfile \"Gemfile.common\"\n", + "source \"https://rubygems.org\"\n%w[rack].each { |g| gem g, \"~> 3.1\" }\n", + ] { + let mut files = BTreeMap::new(); + files.insert("Gemfile".to_string(), gemfile.to_string()); + files.insert("Gemfile.lock".to_string(), lock.to_string()); + let r = rewrite_registry_redirect(&files, &[gem_override("rack", "3.1.8")]); + assert!( + r.files.is_empty() && r.edits.is_empty(), + "a direct dep declared out of sight must not be appended: {gemfile}\n\ + files={:?} edits={:?}", + r.files, + r.edits + ); + assert_eq!( + warning_codes(&r), + vec!["redirect_gem_declaration_not_visible"], + "{gemfile}: {:?}", + r.warnings + ); + } + // Control: a genuinely transitive gem (absent from DEPENDENCIES) is + // still appended. + let mut files = BTreeMap::new(); + files.insert( + "Gemfile".to_string(), + "source \"https://rubygems.org\"\ngem \"rails\"\n".to_string(), + ); + files.insert( + "Gemfile.lock".to_string(), + lock.replace("DEPENDENCIES\n rack (~> 3.1)", "DEPENDENCIES\n rails"), + ); + let r = rewrite_registry_redirect(&files, &[gem_override("rack", "3.1.8")]); + let out = r.files.get("Gemfile").expect("transitive gem appended"); + assert!( + out.ends_with( + "source \"https://patch.test/gem/tok/uuid/\" do\n gem \"rack\", \"3.1.8\"\nend\n" + ), + "{out}" + ); + } + /// The CHECKSUMS pin is gated on the Gemfile source redirect being in /// place: with no Gemfile in the candidate map, pinning the patched sha /// while the gem still resolves upstream guarantees a checksum failure. diff --git a/crates/socket-patch-core/src/vendor/gem.rs b/crates/socket-patch-core/src/vendor/gem.rs index 2aaa4e3d0..99fc41219 100644 --- a/crates/socket-patch-core/src/vendor/gem.rs +++ b/crates/socket-patch-core/src/vendor/gem.rs @@ -415,7 +415,9 @@ fn gem_edits( ) -> Result<(GemfilePlan, LockEdit), Box> { let (name, version) = (prelude.name.as_str(), prelude.version.as_str()); // ── Gemfile edit plan (refusals before any write) ──────────────────── - let plan = match plan_gemfile_edit(&prelude.gemfile_text, name, version, &prelude.copy_rel) { + let plan = match plan_gemfile_edit(&prelude.gemfile_text, name, version, &prelude.copy_rel) + .and_then(|plan| refuse_append_of_direct_dependency(plan, &prelude.lock_text, name)) + { Ok(p) => p, Err(detail) => { return Err(Box::new(refused( @@ -1428,6 +1430,29 @@ fn plan_gemfile_edit( }) } +/// Refuse an [`GemfilePlan::Append`] for a gem the lock lists under +/// `DEPENDENCIES`: bundler resolved it as a DIRECT dependency, so the Gemfile +/// declares it somewhere the line grammar cannot see (`eval_gemfile`, a +/// loop, a gemspec). The managed block would declare it a second time and +/// bundler refuses every install (#482). Every other plan passes through. +fn refuse_append_of_direct_dependency( + plan: GemfilePlan, + lock_text: &str, + name: &str, +) -> Result { + if matches!(plan, GemfilePlan::Append { .. }) + && crate::formats::gem::lock_lists_direct_dependency(lock_text, name) + { + return Err(format!( + "Gemfile.lock lists \"{name}\" as a direct dependency, but the Gemfile declares \ + it somewhere the line grammar cannot edit (an `eval_gemfile`d file, a loop, a \ + gemspec); refusing to append a second declaration (bundler hard-fails on \ + duplicates)" + )); + } + Ok(plan) +} + /// Looser "declared at all?" probe — the redirect rewriter's `declared_re` /// twin. True when a non-comment line is a `gem` call (the keyword followed /// by anything but an identifier character) whose arguments quote the exact @@ -5488,6 +5513,36 @@ mod tests { ); } + /// #482: a DIRECT dependency declared where the line grammar cannot see + /// it (`eval_gemfile`, a loop) is listed under the lock's DEPENDENCIES. + /// Appending the managed block would declare it twice and bundler + /// refuses every install, so the plan must refuse before any write. + #[tokio::test] + async fn direct_dependency_declared_out_of_sight_refuses_instead_of_appending() { + for gemfile in [ + "source \"https://rubygems.org\"\n\ngem \"puma\"\neval_gemfile \"Gemfile.common\"\n", + "source \"https://rubygems.org\"\n\ngem \"puma\"\n%w[rack].each { |g| gem g, \"~> 3.1\" }\n", + ] { + let (_tmp, root, installed, blobs, record) = fixture(gemfile, LOCK_DIRECT).await; + let (code, detail) = + unwrap_refused(run_vendor(&root, &blobs, &installed, &record, false).await); + assert_eq!(code, "gemfile_declaration_not_editable", "{gemfile}"); + assert!(detail.contains("direct dependency"), "{detail}"); + assert!(!root.join(".socket").exists()); + assert_eq!( + tokio::fs::read_to_string(root.join(GEMFILE)).await.unwrap(), + gemfile, + "refusal must write nothing: {detail}" + ); + assert_eq!( + tokio::fs::read_to_string(root.join(GEMFILE_LOCK)) + .await + .unwrap(), + LOCK_DIRECT + ); + } + } + /// Re-vendor (new uuid) over a lock whose CHECKSUMS entry was ALREADY /// bare pre-vendor: the first run recorded no checksum wiring, so the /// re-vendor's `original: None` checksum record has nothing to From 5d814c79aed348e0847dfec493ae94037800e202 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 11:51:10 +0000 Subject: [PATCH 3/6] Test hosted gem refusals with real Bundler Covers a gem declared in two group blocks (#548) and a direct dependency declared through eval_gemfile (#482): the hosted scan must leave the Gemfile pair untouched, attest nothing, and Bundler must still install the project. Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/e2e_redirect_gem_build.rs | 137 +++++++++++++++++- 1 file changed, 130 insertions(+), 7 deletions(-) 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 fe874c013..af8ef70a2 100644 --- a/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs +++ b/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs @@ -405,6 +405,17 @@ enum Driver { /// `Gemfile.next`, so the run must redirect nothing and attest nothing. /// The fixture asserts that contract itself and yields `None`. ScanVexDualBoot, + /// [`Driver::ScanVex`] on a Gemfile that declares the gem in two `group` + /// blocks (#548): bundler accepts the duplicate, but rewriting only one + /// declaration would leave conflicting requirements. The run must + /// refuse, write nothing and attest nothing; the fixture asserts that + /// and yields `None`. + ScanVexDuplicateDeclaration, + /// [`Driver::ScanVex`] on a Gemfile that declares the gem through + /// `eval_gemfile` (#482): the lock lists it as a direct dependency, so + /// appending a source block would declare it twice. Same contract as + /// [`Driver::ScanVexDuplicateDeclaration`]. + ScanVexEvalGemfile, } impl Driver { @@ -413,6 +424,8 @@ impl Driver { Driver::ScanVex => "scan --mode hosted", Driver::GetUuid => "get --mode hosted", Driver::ScanVexDualBoot => "scan --mode hosted (BUNDLE_GEMFILE=Gemfile.next)", + Driver::ScanVexDuplicateDeclaration => "scan --mode hosted (gem in two groups)", + Driver::ScanVexEvalGemfile => "scan --mode hosted (gem via eval_gemfile)", } } } @@ -675,11 +688,22 @@ async fn redirect_scanned_project( // 3. The fixture project, installed from the MOCK upstream (hermetic). let proj = tmp.path().join("proj"); std::fs::create_dir_all(&proj).unwrap(); - std::fs::write( - proj.join(gemfile_name), - format!("source \"{}/upstream\"\n\ngem \"{DEP}\"\n", server.uri()), - ) - .unwrap(); + let gemfile_body = match driver { + Driver::ScanVexDuplicateDeclaration => format!( + "source \"{}/upstream\"\n\ngroup :development do\n gem \"{DEP}\"\nend\n\n\ + group :test do\n gem \"{DEP}\"\nend\n", + server.uri() + ), + Driver::ScanVexEvalGemfile => { + std::fs::write(proj.join("Gemfile.common"), format!("gem \"{DEP}\"\n")).unwrap(); + format!( + "source \"{}/upstream\"\n\neval_gemfile \"Gemfile.common\"\n", + server.uri() + ) + } + _ => format!("source \"{}/upstream\"\n\ngem \"{DEP}\"\n", server.uri()), + }; + std::fs::write(proj.join(gemfile_name), gemfile_body).unwrap(); let config_args = bundler.config_local_args("path", "vendor/bundle"); let config_args: Vec<&str> = config_args.iter().map(String::as_str).collect(); let config = bundle(&proj, &config_args); @@ -775,7 +799,10 @@ async fn redirect_scanned_project( ); } let argv: Vec<&str> = match driver { - Driver::ScanVex | Driver::ScanVexDualBoot => vec![ + Driver::ScanVex + | Driver::ScanVexDualBoot + | Driver::ScanVexDuplicateDeclaration + | Driver::ScanVexEvalGemfile => vec![ "scan", "--mode", "hosted", @@ -825,6 +852,21 @@ async fn redirect_scanned_project( assert_dual_boot_redirects_nothing(&env, &proj, &pristine_gemfile, &pristine_lock); return None; } + if let Some(warning) = match driver { + Driver::ScanVexDuplicateDeclaration => Some("redirect_gem_declared_more_than_once"), + Driver::ScanVexEvalGemfile => Some("redirect_gem_declaration_not_visible"), + _ => None, + } { + assert_unwirable_declaration_redirects_nothing( + &proj, + &bundler, + warning, + (code, &stdout, &stderr), + &pristine_gemfile, + &pristine_lock, + ); + return None; + } assert_eq!( code, 0, @@ -898,7 +940,9 @@ async fn redirect_scanned_project( "in-run hosted VEX is attested from this run's fetched record, not hash-verified: {env}" ); } - Driver::ScanVexDualBoot => unreachable!("asserted and returned above"), + Driver::ScanVexDualBoot + | Driver::ScanVexDuplicateDeclaration + | Driver::ScanVexEvalGemfile => unreachable!("asserted and returned above"), Driver::GetUuid => { // get's hosted envelope (CLI_CONTRACT.md "get --mode and // installed narrowing"): `found` counts the resolved patch; @@ -952,6 +996,49 @@ async fn redirect_scanned_project( }) } +/// #482 / #548: a Gemfile whose declarations of the gem the rewriter cannot +/// edit as one (two `group` blocks, an `eval_gemfile`d file). The scan names +/// the refusal, leaves the pair byte-identical and attests nothing, and the +/// real bundler still installs the project (before the fix the Gemfile was +/// left declaring the gem twice and every install exited 4). +fn assert_unwirable_declaration_redirects_nothing( + proj: &Path, + bundler: &bundler_e2e::Bundler, + warning: &str, + (code, stdout, stderr): (i32, &str, &str), + pristine_gemfile: &[u8], + pristine_lock: &[u8], +) { + let env: serde_json::Value = serde_json::from_str(stdout) + .unwrap_or_else(|e| panic!("not JSON: {e}\nstdout:\n{stdout}\nstderr:\n{stderr}")); + assert!( + stdout.contains(warning), + "the refusal must be named ({warning}): {env}" + ); + assert_ne!(code, 0, "nothing was patched or attested: {env}"); + assert!( + env["vex"]["statements"].as_u64().unwrap_or(0) == 0, + "nothing may be attested: {env}" + ); + assert_eq!( + std::fs::read(proj.join("Gemfile")).unwrap(), + pristine_gemfile, + "the Gemfile must be byte-identical" + ); + assert_eq!( + std::fs::read(proj.join("Gemfile.lock")).unwrap(), + pristine_lock, + "the lock must be byte-identical" + ); + let install = bundle(proj, &["install"]); + assert!( + install.status.success(), + "bundler {} must still install the untouched project:\n{}", + bundler.version, + String::from_utf8_lossy(&install.stderr) + ); +} + /// #390's contract on a `BUNDLE_GEMFILE: Gemfile.next` project: the hosted /// scan names the setting, rewrites neither the `Gemfile` pair (which /// bundler ignores) nor `Gemfile.next`, and its in-run VEX attests nothing. @@ -1549,6 +1636,42 @@ async fn gem_hosted_bundle_gemfile_dual_boot_redirects_nothing() { assert!(fx.is_none(), "the dual-boot driver asserts in place"); } +/// #548: a gem declared in two `group` blocks must not be half-rewritten +/// (bundler refuses `= 1.0.0` next to `>= 0` on every install). +#[tokio::test(flavor = "multi_thread")] +#[ignore = "host capstone: shells out to a real ruby/gem/bundler (>= 1.17; CHECKSUMS arm >= 2.6); \ + the unpinned `test` job skips it, an e2e job with a pinned toolchain runs it via --ignored"] +async fn gem_hosted_gem_declared_in_two_groups_is_refused_and_still_installs() { + let fx = redirect_scanned_project( + "two-groups", + Spelling::Gemfile, + false, + true, + None, + Driver::ScanVexDuplicateDeclaration, + ) + .await; + assert!(fx.is_none(), "the duplicate-declaration driver asserts in place"); +} + +/// #482: a direct dependency declared through `eval_gemfile` must not get a +/// second, appended declaration. +#[tokio::test(flavor = "multi_thread")] +#[ignore = "host capstone: shells out to a real ruby/gem/bundler (>= 1.17; CHECKSUMS arm >= 2.6); \ + the unpinned `test` job skips it, an e2e job with a pinned toolchain runs it via --ignored"] +async fn gem_hosted_eval_gemfile_direct_dep_is_refused_and_still_installs() { + let fx = redirect_scanned_project( + "eval-gemfile", + Spelling::Gemfile, + false, + true, + None, + Driver::ScanVexEvalGemfile, + ) + .await; + assert!(fx.is_none(), "the eval_gemfile driver asserts in place"); +} + /// The compact-index DEPENDENCY contract, pinned from the red side: a patch /// registry whose `/info` omits the gem's runtime deps (production's /// HISTORICAL behavior until the 2026-08-18 republish fixed the served index) From 5ca20fac9eaf93e05852975a248676bc73d06652 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 11:52:42 +0000 Subject: [PATCH 4/6] Test vendored eval_gemfile refusal with Bundler A direct dependency declared through eval_gemfile must be refused before any write (#482), and Bundler must still install the project frozen and unfrozen. Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/e2e_vendor_gem_build.rs | 75 +++++++++++++++++++ 1 file changed, 75 insertions(+) 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 fa81746f8..cd6e29cd7 100644 --- a/crates/socket-patch-cli/tests/e2e_vendor_gem_build.rs +++ b/crates/socket-patch-cli/tests/e2e_vendor_gem_build.rs @@ -1562,3 +1562,78 @@ fn gem_vendor_refuses_a_bundle_gemfile_dual_boot() { ], ); } + +/// #482: a direct dependency declared through `eval_gemfile` is invisible to +/// the Gemfile line grammar. Vendor used to treat it as transitive, append a +/// managed `path:` declaration, exit 0, and leave every `bundle install` +/// failing on the duplicate; it must refuse before any write. +#[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_refuses_an_eval_gemfile_direct_dep() { + let Some((_tmp, proj, _bundler, purl)) = staged_rack_project("eval_gemfile") else { + return; + }; + std::fs::write(proj.join("Gemfile.common"), "gem \"rack\", \"~> 3.1\"\n").unwrap(); + std::fs::write( + proj.join("Gemfile"), + "source \"https://rubygems.org\"\n\neval_gemfile \"Gemfile.common\"\n", + ) + .unwrap(); + let relock = bundle(&proj, &["install"], false); + assert!( + relock.status.success(), + "the eval_gemfile layout installs (test premise):\n{}", + String::from_utf8_lossy(&relock.stderr) + ); + let files = ["Gemfile", "Gemfile.common", "Gemfile.lock"]; + let before: Vec> = files + .iter() + .map(|f| std::fs::read(proj.join(f)).unwrap()) + .collect(); + let (code, stdout, stderr) = run_socket( + &proj, + &[ + "vendor", + "--json", + "--offline", + "--cwd", + proj.to_str().unwrap(), + ], + ); + assert_ne!( + code, 0, + "vendor must not succeed.\nstdout:\n{stdout}\nstderr:\n{stderr}" + ); + let env = parse_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}" + ); + for (file, before) in files.iter().zip(before) { + assert_eq!( + std::fs::read(proj.join(file)).unwrap(), + before, + "{file} must be byte-untouched" + ); + } + assert!( + !proj.join(".socket/vendor/gem").exists(), + "no vendored copy is written" + ); + for frozen in [false, true] { + let install = bundle(&proj, &["install"], frozen); + assert!( + install.status.success(), + "bundle install (frozen: {frozen}) must still succeed:\n{}", + String::from_utf8_lossy(&install.stderr) + ); + } +} From b82c8be91233885a18397eb2f350fb33b7343042 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 13:58:29 +0000 Subject: [PATCH 5/6] Format the duplicate-declaration e2e assert Wraps one long assert in the new Bundler capstone so rustfmt leaves the file unchanged. No behavior change. Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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 af8ef70a2..7f26b8fc4 100644 --- a/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs +++ b/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs @@ -1651,7 +1651,10 @@ async fn gem_hosted_gem_declared_in_two_groups_is_refused_and_still_installs() { Driver::ScanVexDuplicateDeclaration, ) .await; - assert!(fx.is_none(), "the duplicate-declaration driver asserts in place"); + assert!( + fx.is_none(), + "the duplicate-declaration driver asserts in place" + ); } /// #482: a direct dependency declared through `eval_gemfile` must not get a From e13a087d9048b26a9fed6c051f2e5fa8963b50d3 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 14:18:53 +0000 Subject: [PATCH 6/6] Count only real gem declarations as duplicates Hosted redirect refused a Gemfile with one editable declaration when another gem call quoted the same name, e.g. `gem "other", require: "rack"` or a trailing comment. Only `gem` calls whose first argument is the gem now count toward the more-than-once refusal. The looser probe still gates appending a source block. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/patch/redirect/mod.rs | 62 ++++++++++++++++--- 1 file changed, 54 insertions(+), 8 deletions(-) diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index 7b292ee18..5f1b72222 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -4946,20 +4946,30 @@ fn rewrite_gem( // written or already present) — the lock pin below is gated on it. let mut source_placed = false; if let Some(gf) = gemfile.as_mut() { - // Looser "declared at all?" probe: counts every `gem` call that - // names the gem, in any form (indented in a group, parenthesized, - // our own source block). It gates the append branch (appending - // next to a declaration the recognizer below cannot parse would - // leave the gem declared twice) and catches a gem declared more - // than once: rewriting only one of them leaves `= x.y.z` next to - // the other requirement, and bundler refuses the Gemfile (#548). + // Looser "declared at all?" probe: matches every `gem` call that + // names the gem anywhere in its arguments, in any form. It gates + // the append branch: appending next to a declaration the + // recognizer below cannot parse would leave the gem declared + // twice. Too loose to count declarations with, since it also + // matches `gem "rails", require: "rack"` or a trailing comment. let declared_re = Regex::new( &(String::from(r#"(?m)^[ \t]*gem\b[^\n]*["']"#) + ®ex::escape(&dep.name) + r#"["']"#), ) .expect("declaration probe regex from the escaped gem name is valid"); - if declared_re.find_iter(gf).nth(1).is_some() { + // Declarations proper: `gem` calls whose FIRST argument is the + // gem (indented in a group, parenthesized, `gem"x"`, our own + // source block). More than one means rewriting only one leaves + // `= x.y.z` next to the other requirement, and bundler refuses + // the Gemfile (#548). + let declaration_re = Regex::new( + &(String::from(r#"(?m)^[ \t]*gem[ \t]*\(?[ \t]*["']"#) + + ®ex::escape(&dep.name) + + r#"["']"#), + ) + .expect("declaration regex from the escaped gem name is valid"); + if declaration_re.find_iter(gf).nth(1).is_some() { result.warnings.push(RewriteWarning { code: "redirect_gem_declared_more_than_once".into(), detail: format!( @@ -11097,6 +11107,7 @@ mod tests { "source \"https://rubygems.org\"\n\ngem \"vuln-gem\"\n\n\ group :test do\n gem \"vuln-gem\"\nend\n", "source \"https://rubygems.org\"\n\ngem \"vuln-gem\"\ngem(\"vuln-gem\")\n", + "source \"https://rubygems.org\"\n\ngem \"vuln-gem\"\ngem\"vuln-gem\"\n", ] { let mut files = BTreeMap::new(); files.insert("Gemfile".to_string(), gemfile.to_string()); @@ -11117,6 +11128,41 @@ mod tests { } } + /// #548 follow-up: only `gem` calls whose FIRST argument is the gem + /// count as declarations. A different gem's `require:` option or a + /// trailing comment that quotes the name must not turn one editable + /// declaration into a refusal. + #[test] + fn gemfile_name_quoted_by_another_gem_call_still_rewrites() { + let lock = "GEM\n remote: https://rubygems.org/\n specs:\n vuln-gem (1.0.0)\n\n\ + PLATFORMS\n ruby\n\nDEPENDENCIES\n vuln-gem\n\n\ + BUNDLED WITH\n 4.0.17\n"; + for gemfile in [ + "source \"https://rubygems.org\"\n\ngem \"vuln-gem\"\ngem \"other\", require: \"vuln-gem\"\n", + "source \"https://rubygems.org\"\n\ngem \"vuln-gem\"\ngem \"other\" # wraps \"vuln-gem\"\n", + ] { + let mut files = BTreeMap::new(); + files.insert("Gemfile".to_string(), gemfile.to_string()); + files.insert("Gemfile.lock".to_string(), lock.to_string()); + let r = rewrite_registry_redirect(&files, &[gem_override("vuln-gem", "1.0.0")]); + assert!( + !warning_codes(&r).contains(&"redirect_gem_declared_more_than_once"), + "{gemfile}: {:?}", + r.warnings + ); + let out = r.files.get("Gemfile").expect("declaration rewritten"); + assert!( + out.contains("gem \"other\""), + "the other gem call is left alone: {out}" + ); + assert_eq!( + out.matches("gem \"vuln-gem\"").count(), + 1, + "the one declaration is rewritten, nothing appended: {out}" + ); + } + } + /// #482: a DIRECT dependency the root Gemfile declares out of the /// rewriter's sight (`eval_gemfile`, a loop) is listed under the lock's /// DEPENDENCIES. Appending a source block for it declares it twice and