Skip to content

Commit e13a087

Browse files
committed
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
1 parent b82c8be commit e13a087

1 file changed

Lines changed: 54 additions & 8 deletions

File tree

  • crates/socket-patch-core/src/patch/redirect

‎crates/socket-patch-core/src/patch/redirect/mod.rs‎

Lines changed: 54 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4946,20 +4946,30 @@ fn rewrite_gem(
49464946
// written or already present) — the lock pin below is gated on it.
49474947
let mut source_placed = false;
49484948
if let Some(gf) = gemfile.as_mut() {
4949-
// Looser "declared at all?" probe: counts every `gem` call that
4950-
// names the gem, in any form (indented in a group, parenthesized,
4951-
// our own source block). It gates the append branch (appending
4952-
// next to a declaration the recognizer below cannot parse would
4953-
// leave the gem declared twice) and catches a gem declared more
4954-
// than once: rewriting only one of them leaves `= x.y.z` next to
4955-
// the other requirement, and bundler refuses the Gemfile (#548).
4949+
// Looser "declared at all?" probe: matches every `gem` call that
4950+
// names the gem anywhere in its arguments, in any form. It gates
4951+
// the append branch: appending next to a declaration the
4952+
// recognizer below cannot parse would leave the gem declared
4953+
// twice. Too loose to count declarations with, since it also
4954+
// matches `gem "rails", require: "rack"` or a trailing comment.
49564955
let declared_re = Regex::new(
49574956
&(String::from(r#"(?m)^[ \t]*gem\b[^\n]*["']"#)
49584957
+ &regex::escape(&dep.name)
49594958
+ r#"["']"#),
49604959
)
49614960
.expect("declaration probe regex from the escaped gem name is valid");
4962-
if declared_re.find_iter(gf).nth(1).is_some() {
4961+
// Declarations proper: `gem` calls whose FIRST argument is the
4962+
// gem (indented in a group, parenthesized, `gem"x"`, our own
4963+
// source block). More than one means rewriting only one leaves
4964+
// `= x.y.z` next to the other requirement, and bundler refuses
4965+
// the Gemfile (#548).
4966+
let declaration_re = Regex::new(
4967+
&(String::from(r#"(?m)^[ \t]*gem[ \t]*\(?[ \t]*["']"#)
4968+
+ &regex::escape(&dep.name)
4969+
+ r#"["']"#),
4970+
)
4971+
.expect("declaration regex from the escaped gem name is valid");
4972+
if declaration_re.find_iter(gf).nth(1).is_some() {
49634973
result.warnings.push(RewriteWarning {
49644974
code: "redirect_gem_declared_more_than_once".into(),
49654975
detail: format!(
@@ -11097,6 +11107,7 @@ mod tests {
1109711107
"source \"https://rubygems.org\"\n\ngem \"vuln-gem\"\n\n\
1109811108
group :test do\n gem \"vuln-gem\"\nend\n",
1109911109
"source \"https://rubygems.org\"\n\ngem \"vuln-gem\"\ngem(\"vuln-gem\")\n",
11110+
"source \"https://rubygems.org\"\n\ngem \"vuln-gem\"\ngem\"vuln-gem\"\n",
1110011111
] {
1110111112
let mut files = BTreeMap::new();
1110211113
files.insert("Gemfile".to_string(), gemfile.to_string());
@@ -11117,6 +11128,41 @@ mod tests {
1111711128
}
1111811129
}
1111911130

11131+
/// #548 follow-up: only `gem` calls whose FIRST argument is the gem
11132+
/// count as declarations. A different gem's `require:` option or a
11133+
/// trailing comment that quotes the name must not turn one editable
11134+
/// declaration into a refusal.
11135+
#[test]
11136+
fn gemfile_name_quoted_by_another_gem_call_still_rewrites() {
11137+
let lock = "GEM\n remote: https://rubygems.org/\n specs:\n vuln-gem (1.0.0)\n\n\
11138+
PLATFORMS\n ruby\n\nDEPENDENCIES\n vuln-gem\n\n\
11139+
BUNDLED WITH\n 4.0.17\n";
11140+
for gemfile in [
11141+
"source \"https://rubygems.org\"\n\ngem \"vuln-gem\"\ngem \"other\", require: \"vuln-gem\"\n",
11142+
"source \"https://rubygems.org\"\n\ngem \"vuln-gem\"\ngem \"other\" # wraps \"vuln-gem\"\n",
11143+
] {
11144+
let mut files = BTreeMap::new();
11145+
files.insert("Gemfile".to_string(), gemfile.to_string());
11146+
files.insert("Gemfile.lock".to_string(), lock.to_string());
11147+
let r = rewrite_registry_redirect(&files, &[gem_override("vuln-gem", "1.0.0")]);
11148+
assert!(
11149+
!warning_codes(&r).contains(&"redirect_gem_declared_more_than_once"),
11150+
"{gemfile}: {:?}",
11151+
r.warnings
11152+
);
11153+
let out = r.files.get("Gemfile").expect("declaration rewritten");
11154+
assert!(
11155+
out.contains("gem \"other\""),
11156+
"the other gem call is left alone: {out}"
11157+
);
11158+
assert_eq!(
11159+
out.matches("gem \"vuln-gem\"").count(),
11160+
1,
11161+
"the one declaration is rewritten, nothing appended: {out}"
11162+
);
11163+
}
11164+
}
11165+
1112011166
/// #482: a DIRECT dependency the root Gemfile declares out of the
1112111167
/// rewriter's sight (`eval_gemfile`, a loop) is listed under the lock's
1112211168
/// DEPENDENCIES. Appending a source block for it declares it twice and

0 commit comments

Comments
 (0)