Skip to content

Commit 25aabce

Browse files
mikolalysenkoclaude
andcommitted
Skip hosted gem pins when no lock exists
A Gemfile with no Gemfile.lock (a fresh library clone before `bundle install`) skipped the #1060 version-not-locked guard, so a hosted scan pinned whatever version the shared gem home held: `gem "x", "~> 2.0"` was rewritten down to another project's 1.0.0, and a gem the project never declared was appended as a new dependency. Both exited 0, and rollback cannot undo a Gemfile-only pin. Hosted mode now skips every gem in a lockless project with `redirect_gem_no_lockfile`, writing nothing; the detail asks for `bundle lock` (or `bundle install`) and a re-run. Rewriter unit tests that used lockless Gemfiles now carry a CHECKSUMS-less lock. Fixes #1125 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent cdd8aa5 commit 25aabce

4 files changed

Lines changed: 220 additions & 2 deletions

File tree

‎crates/socket-patch-cli/CLI_CONTRACT.md‎

Lines changed: 2 additions & 1 deletion
Large diffs are not rendered by default.

‎crates/socket-patch-cli/tests/e2e_redirect_gem_stale_install.rs‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,53 @@ async fn gem_hosted_scan_never_pins_a_version_the_lock_does_not_resolve() {
438438
);
439439
}
440440

441+
/// #1125: the same shared-home copy, but the project has no lock at all
442+
/// (a fresh library clone before `bundle install`). Nothing says which
443+
/// version the project resolves, so hosted mode must neither rewrite the
444+
/// user's `~> 2.0` down to the installed 1.0.0 nor append the gem to a
445+
/// Gemfile that never declared it: it skips with `redirect_gem_no_lockfile`
446+
/// (whose detail names `bundle lock`) and writes nothing.
447+
#[tokio::test(flavor = "multi_thread")]
448+
async fn gem_hosted_scan_without_a_lock_pins_nothing() {
449+
let server = MockServer::start().await;
450+
mount_api(&server, None).await;
451+
for gemfile in [
452+
format!("source \"https://rubygems.org\"\ngem \"{DEP}\", \"~> 2.0\"\n"),
453+
"source \"https://rubygems.org\"\ngem \"tiny-dep\"\n".to_string(),
454+
] {
455+
let tmp = tempfile::tempdir().unwrap();
456+
let proj = tmp.path().join("proj");
457+
std::fs::create_dir_all(&proj).unwrap();
458+
std::fs::write(proj.join("Gemfile"), &gemfile).unwrap();
459+
materialize_installed_gem(&proj, "3.3.0", UPSTREAM_LIB);
460+
461+
let (code, stdout, stderr) = hosted_scan_json(&proj, &server.uri());
462+
let env = common::parse_json_envelope(&stdout);
463+
assert_eq!(code, 0, "{env}\nstderr:\n{stderr}");
464+
assert_eq!(
465+
env["redirect"]["redirected"], 0,
466+
"nothing may be redirected: {env}\nstderr:\n{stderr}"
467+
);
468+
let hit: Vec<&serde_json::Value> = env["redirect"]["warnings"]
469+
.as_array()
470+
.expect("redirect.warnings")
471+
.iter()
472+
.filter(|w| w["code"] == "redirect_gem_no_lockfile")
473+
.collect();
474+
assert_eq!(hit.len(), 1, "the missing lock must be reported: {env}");
475+
assert!(
476+
hit[0]["detail"].as_str().unwrap().contains("bundle lock"),
477+
"the remedy names `bundle lock`: {env}"
478+
);
479+
assert_eq!(
480+
std::fs::read_to_string(proj.join("Gemfile")).unwrap(),
481+
gemfile,
482+
"the Gemfile must stay byte-identical"
483+
);
484+
assert!(!proj.join("Gemfile.lock").exists());
485+
}
486+
}
487+
441488
/// TWO gem homes (two ruby versions under vendor/bundle) both stale: one
442489
/// warning per home, each naming its own home's paths — multiplicity is
443490
/// per materialization, not per purl.

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

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6424,6 +6424,26 @@ fn rewrite_gem(
64246424
}
64256425
continue;
64266426
}
6427+
// A manifest with no lock (a fresh library clone before `bundle
6428+
// install`): nothing records which version the project resolves,
6429+
// and the crawl may hand in another project's copy from the shared
6430+
// gem home. Pinning it would overwrite the user's constraint or
6431+
// append a gem the project never declared, and a Gemfile-only pin
6432+
// is no reference `vex` / `rollback` / `remove` can see (#1125).
6433+
// Hosted mode re-points the version a lock resolves, so ask for one.
6434+
if locked.is_none() {
6435+
result.warnings.push(RewriteWarning {
6436+
code: "redirect_gem_no_lockfile".into(),
6437+
detail: format!(
6438+
"{gemfile_name} has no {lock_name}, so nothing says which version of {} \
6439+
this project resolves ({} {} may be another project's copy in the \
6440+
shared gem home); redirect skipped — run `bundle lock` (or `bundle \
6441+
install`), commit {lock_name}, and re-run the hosted scan",
6442+
dep.name, dep.name, dep.version
6443+
),
6444+
});
6445+
continue;
6446+
}
64276447

64286448
// Platform-suffixed CHECKSUMS siblings (`name (version-arm64-darwin)
64296449
// sha256=`) mean bundler resolves platform-specific gems the patch
@@ -14660,6 +14680,73 @@ mod tests {
1466014680
assert!(out.contains(" gem \"colorize\", \"1.1.0\"\nend"), "{out}");
1466114681
}
1466214682

14683+
/// #1125: with no `Gemfile.lock` / `gems.locked` (a fresh library
14684+
/// clone before `bundle install`) nothing says which version the
14685+
/// project resolves, and the crawl may hand in another project's copy
14686+
/// from the shared gem home. Pinning it would downgrade the user's
14687+
/// range or append a gem the project never used. The rewriter skips
14688+
/// every gem with a lock-first remedy, writing nothing.
14689+
#[test]
14690+
fn gem_without_a_lock_is_never_pinned() {
14691+
for (manifest, gemfile) in [
14692+
(
14693+
"Gemfile",
14694+
"source \"https://rubygems.org\"\n\ngem \"colorize\", \"~> 2.0\"\n",
14695+
),
14696+
(
14697+
"Gemfile",
14698+
"source \"https://rubygems.org\"\n\ngem \"tiny-dep\"\n",
14699+
),
14700+
(
14701+
"Gemfile",
14702+
"source \"https://rubygems.org\"\n\ngem \"colorize\", \"0.8.1\"\n",
14703+
),
14704+
(
14705+
"gems.rb",
14706+
"source \"https://rubygems.org\"\n\ngem \"tiny-dep\"\n",
14707+
),
14708+
] {
14709+
let files = BTreeMap::from([(manifest.to_string(), gemfile.to_string())]);
14710+
let r = rewrite_registry_redirect(&files, &[gem_override("colorize", "0.8.1")]);
14711+
assert!(
14712+
r.files.is_empty() && r.edits.is_empty(),
14713+
"{gemfile}: a lockless gem must not be pinned\nfiles={:?}",
14714+
r.files
14715+
);
14716+
assert_eq!(
14717+
warning_codes(&r),
14718+
vec!["redirect_gem_no_lockfile"],
14719+
"{gemfile}: {:?}",
14720+
r.warnings
14721+
);
14722+
let detail = &r.warnings[0].detail;
14723+
assert!(
14724+
detail.contains("bundle lock") && detail.contains("colorize 0.8.1"),
14725+
"{detail}"
14726+
);
14727+
}
14728+
}
14729+
14730+
/// A bundler 2.2–2.5 lock (no CHECKSUMS) resolving `specs` from
14731+
/// rubygems.org, each `(name, version, deps)`, with `direct` in
14732+
/// DEPENDENCIES. Hosted mode only pins a version a lock resolves
14733+
/// (#1055, #1125); without CHECKSUMS it leaves the lock untouched.
14734+
fn gem_lock_resolving(specs: &[(&str, &str, &[&str])], direct: &[&str]) -> String {
14735+
let mut out = String::from("GEM\n remote: https://rubygems.org/\n specs:\n");
14736+
for (name, version, deps) in specs {
14737+
out.push_str(&format!(" {name} ({version})\n"));
14738+
for dep in *deps {
14739+
out.push_str(&format!(" {dep}\n"));
14740+
}
14741+
}
14742+
out.push_str("\nPLATFORMS\n ruby\n\nDEPENDENCIES\n");
14743+
for dep in direct {
14744+
out.push_str(&format!(" {dep}\n"));
14745+
}
14746+
out.push_str("\nBUNDLED WITH\n 2.5.23\n");
14747+
out
14748+
}
14749+
1466314750
fn gem_override(name: &str, version: &str) -> DepOverride {
1466414751
DepOverride {
1466514752
ecosystem: "gem".into(),
@@ -14741,6 +14828,13 @@ mod tests {
1474114828
"source \"https://rubygems.org\"\n\ngem \"rack-mini-profiler\", \"3.1.0\", require: false\n"
1474214829
.to_string(),
1474314830
);
14831+
files.insert(
14832+
"Gemfile.lock".to_string(),
14833+
gem_lock_resolving(
14834+
&[("rack-mini-profiler", "3.1.0", &[])],
14835+
&["rack-mini-profiler (= 3.1.0)"],
14836+
),
14837+
);
1474414838
let r = rewrite_registry_redirect(&files, &[gem_override("rack-mini-profiler", "3.1.0")]);
1474514839
let out = r.files.get("Gemfile").expect("Gemfile rewritten");
1474614840
assert!(
@@ -14854,6 +14948,10 @@ mod tests {
1485414948
gem \"rails\", \"7.0.0\"\n"
1485514949
.to_string(),
1485614950
);
14951+
files.insert(
14952+
"Gemfile.lock".to_string(),
14953+
gem_lock_resolving(&[("rails", "7.0.0", &[])], &["rails (= 7.0.0)"]),
14954+
);
1485714955
let r = rewrite_registry_redirect(&files, &[gem_override("rails", "7.0.0")]);
1485814956
let out = r.files.get("Gemfile").expect("Gemfile rewritten");
1485914957
assert!(
@@ -14887,6 +14985,10 @@ mod tests {
1488714985
"Gemfile".to_string(),
1488814986
"source \"https://rubygems.org\"\n\ngem \"rails\", \"7.0.0\"\n".to_string(),
1488914987
);
14988+
files.insert(
14989+
"Gemfile.lock".to_string(),
14990+
gem_lock_resolving(&[("rails", "7.0.0", &[])], &["rails (= 7.0.0)"]),
14991+
);
1489014992
let first = rewrite_registry_redirect(&files, &[ov("tok-one")]);
1489114993
let redirected = first.files.get("Gemfile").expect("first run rewrites");
1489214994
files.insert("Gemfile".to_string(), redirected.clone());
@@ -14958,6 +15060,10 @@ mod tests {
1495815060
"Gemfile".to_string(),
1495915061
"source \"https://rubygems.org\"\n\ngem \"rails\", \"7.0.0\"\n".to_string(),
1496015062
);
15063+
files.insert(
15064+
"Gemfile.lock".to_string(),
15065+
gem_lock_resolving(&[("rails", "7.0.0", &[])], &["rails (= 7.0.0)"]),
15066+
);
1496115067
let first = rewrite_registry_redirect(&files, &[ov("tok-one")]);
1496215068
files.insert(
1496315069
"Gemfile".to_string(),
@@ -15029,6 +15135,10 @@ mod tests {
1502915135
let mut files = BTreeMap::new();
1503015136
files.insert("gems.rb".to_string(), gemfile.clone());
1503115137
files.insert("Gemfile".to_string(), gemfile);
15138+
files.insert(
15139+
"gems.locked".to_string(),
15140+
gem_lock_resolving(&[("rails", "7.0.0", &[])], &["rails (= 7.0.0)"]),
15141+
);
1503215142
let first = rewrite_registry_redirect(&files, &[ov("tok-one")]);
1503315143
for (name, content) in first.files {
1503415144
files.insert(name, content);
@@ -15075,6 +15185,10 @@ mod tests {
1507515185
gem \"rails\", \"7.0.0\"\r\nend\r\n";
1507615186
let mut files = BTreeMap::new();
1507715187
files.insert("Gemfile".to_string(), crlf_gemfile.to_string());
15188+
files.insert(
15189+
"Gemfile.lock".to_string(),
15190+
gem_lock_resolving(&[("rails", "7.0.0", &[])], &["rails (= 7.0.0)"]),
15191+
);
1507815192

1507915193
// Same grant: recognized in place, a true no-op.
1508015194
let same = rewrite_registry_redirect(&files, &[ov("tok-one")]);
@@ -15279,6 +15393,10 @@ mod tests {
1527915393
#[test]
1528015394
fn gemfile_source_option_refusal_prescribes_vendor_revert_for_own_wiring() {
1528115395
let mut files = BTreeMap::new();
15396+
files.insert(
15397+
"Gemfile.lock".to_string(),
15398+
gem_lock_resolving(&[("rails", "7.0.0", &[])], &["rails (= 7.0.0)"]),
15399+
);
1528215400
files.insert(
1528315401
"Gemfile".to_string(),
1528415402
"source \"https://rubygems.org\"\n\n\
@@ -15479,6 +15597,13 @@ mod tests {
1547915597
gem\t\"puma\", \"6.0.0\"\n"
1548015598
.to_string(),
1548115599
);
15600+
files.insert(
15601+
"Gemfile.lock".to_string(),
15602+
gem_lock_resolving(
15603+
&[("puma", "6.0.0", &[]), ("rails", "7.0.0", &[])],
15604+
&["puma (= 6.0.0)", "rails (= 7.0.0)"],
15605+
),
15606+
);
1548215607
let r = rewrite_registry_redirect(
1548315608
&files,
1548415609
&[
@@ -15516,6 +15641,10 @@ mod tests {
1551615641
"Gemfile".to_string(),
1551715642
"source \"https://rubygems.org\"\n\ngem\"rails\", \"7.0.0\"\n".to_string(),
1551815643
);
15644+
files.insert(
15645+
"Gemfile.lock".to_string(),
15646+
gem_lock_resolving(&[("rails", "7.0.0", &[])], &["rails (= 7.0.0)"]),
15647+
);
1551915648
let r = rewrite_registry_redirect(&files, &[gem_override("rails", "7.0.0")]);
1552015649
assert!(
1552115650
r.files.is_empty() && r.edits.is_empty(),
@@ -16607,6 +16736,10 @@ mod tests {
1660716736
let mut files = BTreeMap::new();
1660816737
files.insert("gems.rb".to_string(), gemfile.clone());
1660916738
files.insert("Gemfile".to_string(), gemfile);
16739+
files.insert(
16740+
"gems.locked".to_string(),
16741+
gem_lock_resolving(&[("rails", "7.0.0", &[])], &["rails (= 7.0.0)"]),
16742+
);
1661016743
let r = rewrite_registry_redirect(&files, &[gem_override("rails", "7.0.0")]);
1661116744
assert!(
1661216745
r.files.contains_key("gems.rb") && !r.files.contains_key("Gemfile"),
@@ -16656,6 +16789,10 @@ mod tests {
1665616789
#[test]
1665716790
fn gems_rb_divergence_only_in_redirected_dep_line_proceeds() {
1665816791
let mut files = BTreeMap::new();
16792+
files.insert(
16793+
"gems.locked".to_string(),
16794+
gem_lock_resolving(&[("rails", "7.0.0", &[])], &["rails (= 7.0.0)"]),
16795+
);
1665916796
files.insert(
1666016797
"gems.rb".to_string(),
1666116798
"source \"https://rubygems.org\"\n\ngem \"rails\", \"7.0.0\"\n".to_string(),
@@ -16742,6 +16879,10 @@ mod tests {
1674216879
let mut files = BTreeMap::new();
1674316880
files.insert("gems.rb".to_string(), gemfile.clone());
1674416881
files.insert("Gemfile".to_string(), gemfile);
16882+
files.insert(
16883+
"gems.locked".to_string(),
16884+
gem_lock_resolving(&[("rails", "7.0.0", &[])], &["rails (= 7.0.0)"]),
16885+
);
1674516886
let first = rewrite_registry_redirect(&files, &[ov("tok-one")]);
1674616887
for (name, content) in first.files {
1674716888
files.insert(name, content);
@@ -16786,6 +16927,16 @@ mod tests {
1678616927
let mut files = BTreeMap::new();
1678716928
files.insert("gems.rb".to_string(), gemfile.clone());
1678816929
files.insert("Gemfile".to_string(), gemfile);
16930+
files.insert(
16931+
"gems.locked".to_string(),
16932+
gem_lock_resolving(
16933+
&[
16934+
("rack", "3.0.0", &["rails (>= 7)"]),
16935+
("rails", "7.0.0", &[]),
16936+
],
16937+
&["rack (= 3.0.0)"],
16938+
),
16939+
);
1678916940
let ovr = gem_override("rails", "7.0.0");
1679016941
let first = rewrite_registry_redirect(&files, std::slice::from_ref(&ovr));
1679116942
assert!(
@@ -16836,6 +16987,10 @@ mod tests {
1683616987
// gems.rb exactly as run 1 wrote it, after a CRLF checkout; the
1683716988
// Gemfile twin got the same CRLF treatment but never had the block.
1683816989
let mut files = BTreeMap::new();
16990+
files.insert(
16991+
"gems.locked".to_string(),
16992+
gem_lock_resolving(&[("rails", "7.0.0", &[])], &["rails (= 7.0.0)"]),
16993+
);
1683916994
files.insert(
1684016995
"gems.rb".to_string(),
1684116996
"source \"https://rubygems.org\"\r\n\r\n\
@@ -22646,6 +22801,10 @@ packages:
2264622801
"Gemfile".to_string(),
2264722802
"source \"https://rubygems.org\"\n\ngem(\"rails\",\n \"7.0.0\")\n".to_string(),
2264822803
);
22804+
files.insert(
22805+
"Gemfile.lock".to_string(),
22806+
gem_lock_resolving(&[("rails", "7.0.0", &[])], &["rails (= 7.0.0)"]),
22807+
);
2264922808
let r = rewrite_registry_redirect(&files, &[gem_override("rails", "7.0.0")]);
2265022809
assert!(
2265122810
r.files.is_empty() && r.edits.is_empty(),

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

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1147,7 +1147,18 @@ mod tests {
11471147
}),
11481148
..ov.clone()
11491149
};
1150-
let files = BTreeMap::from([("Gemfile".to_string(), lf.to_string())]);
1150+
// A CHECKSUMS-less lock resolving both (hosted mode pins only a
1151+
// version a lock resolves, #1125) and left untouched by the rewrite.
1152+
let files = BTreeMap::from([
1153+
("Gemfile".to_string(), lf.to_string()),
1154+
(
1155+
"Gemfile.lock".to_string(),
1156+
"GEM\n remote: https://rubygems.org/\n specs:\n puma (6.0.0)\n \
1157+
rack (>= 3)\n rails (>= 7)\n rack (3.0.0)\n rails (7.0.0)\n\n\
1158+
PLATFORMS\n ruby\n\nDEPENDENCIES\n puma\n\nBUNDLED WITH\n 2.5.23\n"
1159+
.to_string(),
1160+
),
1161+
]);
11511162
let gemfile =
11521163
rewrite_registry_redirect(&files, &[ov.clone(), rack_ov]).files["Gemfile"].clone();
11531164
let rack = || Gem {

0 commit comments

Comments
 (0)