Skip to content

Commit cb98e0e

Browse files
committed
Keep Bundler override targets transitive on undo
With Bundler 4.1, `override "other", with: "gem", in: "parent"` records the replacement only in the lock's OVERRIDES section and keeps parent's original edge in the specs. Hosted rollback and remove then treated the gem as directly declared and wrote a new exact `gem "gem", "x.y.z"` pin into the Gemfile, which blocked the project from updating past the vulnerable version. The restore now counts an OVERRIDES `with:` target as a dependency of another spec, so it drops the appended Socket block and its DEPENDENCIES entry and gives back the original pair byte for byte. Fixes #1989 Assisted-by: Claude Code:claude-opus-5-5
1 parent f577be4 commit cb98e0e

1 file changed

Lines changed: 119 additions & 1 deletion

File tree

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

‎crates/socket-patch-core/src/patch/redirect/upstream/gem.rs‎

Lines changed: 119 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,11 +173,30 @@ fn dependency_line<'l>(lines: &[&'l str], name: &str) -> Option<(usize, &'l str)
173173
})
174174
}
175175

176-
/// Whether another locked spec depends on `name` (a 6-space sub-line).
176+
/// Whether another locked spec depends on `name`: a 6-space sub-line, or
177+
/// the `with:` target of a Bundler 4.1 `OVERRIDES` entry (#1989). An
178+
/// `override "other", with: "name", in: "parent"` keeps `parent`'s
179+
/// original edge to `other` in the specs, so no sub-line names `name`.
177180
fn is_subdependency(lines: &[&str], name: &str) -> bool {
178181
lines
179182
.iter()
180183
.any(|l| indented(l, 6).is_some_and(|d| d.split([' ', '(']).next() == Some(name)))
184+
|| override_targets(lines).any(|t| t == name)
185+
}
186+
187+
/// The `with:` values of the lock's `OVERRIDES` entries
188+
/// (` "other", with: "name", in: "parent"`).
189+
fn override_targets<'l>(lines: &'l [&'l str]) -> impl Iterator<Item = &'l str> + 'l {
190+
let range = named_section(lines, "OVERRIDES").unwrap_or((0, 0));
191+
lines[range.0..range.1].iter().filter_map(|l| {
192+
let entry = indented(l, 2)?.trim_end();
193+
// Gem names hold no `"` or `,`, so each `, ` starts a new argument.
194+
entry.split(", ").find_map(|arg| {
195+
arg.strip_prefix("with: \"")?
196+
.strip_suffix('"')
197+
.filter(|v| !v.contains('"'))
198+
})
199+
})
181200
}
182201

183202
/// How the lock wires patch `uuid`.
@@ -1863,6 +1882,40 @@ mod tests {
18631882
assert!(!is_subdependency(&[" rails (7.0.0)"], "rails"));
18641883
}
18651884

1885+
/// #1989: Bundler 4.1's `override "other", with: "rails", in: "puma"`
1886+
/// keeps `puma`'s original edge to `other` in the specs and records
1887+
/// the replacement only in `OVERRIDES`, so its `with:` target is a
1888+
/// dependency of another spec although no sub-line names it.
1889+
#[test]
1890+
fn an_overrides_with_target_is_a_subdependency() {
1891+
let lock = |overrides: &str| {
1892+
format!(
1893+
"GEM\n remote: https://rubygems.org/\n specs:\n puma (6.0.0)\n \
1894+
other (>= 1)\n rails (7.0.0)\n\nDEPENDENCIES\n puma\n\nOVERRIDES\n\
1895+
{overrides}\nBUNDLED WITH\n 4.1.0.beta3\n"
1896+
)
1897+
};
1898+
let has = |overrides: &str| {
1899+
let text = lock(overrides);
1900+
let lines: Vec<&str> = text.split('\n').collect();
1901+
is_subdependency(&lines, "rails")
1902+
};
1903+
assert!(has(" \"other\", with: \"rails\", in: \"puma\"\n"));
1904+
assert!(has(" \"other\", in: \"puma\", with: \"rails\"\n"));
1905+
assert!(has(
1906+
" \"x\", with: \"y\", in: \"z\"\n \"other\", with: \"rails\", in: \"puma\"\n"
1907+
));
1908+
// `rails` as the overridden name, the parent, or a prefix is not
1909+
// a `with:` target.
1910+
assert!(!has(" \"rails\", with: \"other\", in: \"puma\"\n"));
1911+
assert!(!has(" \"other\", with: \"x\", in: \"rails\"\n"));
1912+
assert!(!has(" \"other\", with: \"rails-html\", in: \"puma\"\n"));
1913+
assert!(!has(""));
1914+
// A `with:` entry outside the OVERRIDES section is not read.
1915+
let lines = ["DEPENDENCIES", " \"other\", with: \"rails\", in: \"puma\""];
1916+
assert!(!is_subdependency(&lines, "rails"));
1917+
}
1918+
18661919
/// #457: what the hosted rewriter itself writes for a TRANSITIVE gem
18671920
/// (an appended block, converged CHECKSUMS lock) must be recognized as
18681921
/// its append, and undoing it must give the manifest back byte for
@@ -2055,6 +2108,71 @@ mod tests {
20552108
assert_eq!(next_lock.as_deref(), Some(lock.as_str()));
20562109
}
20572110

2111+
/// #1989: a gem reached only through a Bundler 4.1 `override …, with:`
2112+
/// is transitive too. The lock keeps the parent's original edge, so no
2113+
/// spec sub-line names it; the restore must still drop the appended
2114+
/// block and its DEPENDENCIES entry, not write a new exact pin.
2115+
#[tokio::test]
2116+
async fn override_target_redirect_round_trips_through_restore() {
2117+
let client = super::super::UpstreamClient::new(true);
2118+
let upstream_sha = "2".repeat(64);
2119+
client
2120+
.seed_rubygems_sha256("rails", "7.0.0", &upstream_sha)
2121+
.await;
2122+
let ctx = ctx_with(&client);
2123+
let manifest = "source \"https://rubygems.org\"\n\ngem \"puma\"\n\n\
2124+
override \"other\", with: \"rails\", in: \"puma\"\n";
2125+
let lock = format!(
2126+
"GEM\n remote: https://rubygems.org/\n specs:\n puma (6.0.0)\n other\n \
2127+
rails (7.0.0)\n\nPLATFORMS\n ruby\n\nDEPENDENCIES\n puma\n\nOVERRIDES\n \
2128+
\"other\", with: \"rails\", in: \"puma\"\n\nCHECKSUMS\n puma (6.0.0) sha256={}\n \
2129+
rails (7.0.0) sha256={upstream_sha}\n\nBUNDLED WITH\n 4.1.0.beta3\n",
2130+
"a".repeat(64)
2131+
);
2132+
let files = BTreeMap::from([
2133+
("Gemfile".to_string(), manifest.to_string()),
2134+
("Gemfile.lock".to_string(), lock.clone()),
2135+
]);
2136+
let r = crate::patch::redirect::rewrite_registry_redirect(
2137+
&files,
2138+
&[crate::patch::redirect::DepOverride {
2139+
ecosystem: "gem".into(),
2140+
name: "rails".into(),
2141+
namespace: None,
2142+
version: "7.0.0".into(),
2143+
token: "tok".into(),
2144+
patch_uuid: UUID.into(),
2145+
artifact_url: "https://patch.test/rails-7.0.0.gem".into(),
2146+
registry_override: Some(crate::patch::redirect::RegistryOverride {
2147+
kind: "rubygems-compact-index".into(),
2148+
index_url: IDX.into(),
2149+
identifiers: crate::patch::redirect::RegistryOverrideIdentifiers {
2150+
name: "rails".into(),
2151+
version: "7.0.0".into(),
2152+
gem_checksum_sha256: Some("f".repeat(64)),
2153+
..Default::default()
2154+
},
2155+
}),
2156+
integrity: Default::default(),
2157+
}],
2158+
);
2159+
let (hosted_manifest, hosted_lock) = (&r.files["Gemfile"], &r.files["Gemfile.lock"]);
2160+
assert!(hosted_lock.contains(" rails (= 7.0.0)!"), "{hosted_lock}");
2161+
let (next_lock, next_manifest, _) = restore_one(
2162+
&gem(),
2163+
Some(hosted_lock),
2164+
Some(hosted_manifest),
2165+
"Gemfile",
2166+
&global_sources(hosted_manifest),
2167+
&ctx,
2168+
)
2169+
.await
2170+
.unwrap()
2171+
.expect("the redirect is found");
2172+
assert_eq!(next_manifest.as_deref(), Some(manifest));
2173+
assert_eq!(next_lock.as_deref(), Some(lock.as_str()));
2174+
}
2175+
20582176
/// #1056: a gem the user declared inside a `source "…" do` block is
20592177
/// locked with a `!` source pin in `DEPENDENCIES` (bundler writes it
20602178
/// for any dependency with an explicit source, rubygems.org included).

0 commit comments

Comments
 (0)