From ef3218d4adb1c9fb07233d0d8f94c6e13b3b5aa0 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 16:33:16 +0000 Subject: [PATCH 1/4] Start fix for #1831 Assisted-by: Claude Code:claude-opus-5-5 From 89d5c1dd4850cc691ce193d45f9ed54b3678c3bc Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 16:46:59 +0000 Subject: [PATCH 2/4] Keep CRLF Gemfiles CRLF on hosted gem redirect A hosted scan or get over a CRLF Gemfile (a Windows checkout, or core.autocrlf=true) wrote the Socket `source ... do` block with bare LF endings, so the committed Gemfile ended up with mixed line endings and churned under editors and .gitattributes normalization. The rewritten block now takes the ending of the gem line it replaces (the file's ending for an unterminated last line), and the block appended for a transitive gem takes the file's ending. This matches what vendored mode does since #1822. Fixes #1831 Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/e2e_redirect_gem_build.rs | 90 +++++++++++ .../src/patch/redirect/mod.rs | 152 ++++++++++++++++-- 2 files changed, 231 insertions(+), 11 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 8500a5afc..d2b2fd4a0 100644 --- a/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs +++ b/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs @@ -4769,3 +4769,93 @@ async fn gem_vendored_unwind_names_a_patched_archive_in_vendor_cache() { vendored_unwind_names_the_patched_cached_archive(&fx, command); } } + +/// #1831: a hosted redirect over a CRLF Gemfile (a Windows checkout, or +/// `core.autocrlf=true`) keeps it CRLF throughout, for a plain top-level +/// declaration and for one inside the user's own `source … do` block; a +/// real bundler converges the result, a re-scan is a no-op, and `rollback` +/// hands back a CRLF Gemfile that still declares the gem. +#[tokio::test(flavor = "multi_thread")] +#[ignore = "host capstone: shells out to a real ruby/gem/bundler (>= 1.17); \ + the unpinned `test` job skips it, an e2e job with a pinned toolchain runs it via --ignored"] +async fn gem_hosted_redirect_keeps_a_crlf_gemfile_crlf() { + fn all_crlf(text: &str) -> bool { + let crlf = text.matches("\r\n").count(); + crlf > 0 && crlf == text.matches('\n').count() && crlf == text.matches('\r').count() + } + for (tag, driver) in [ + ("crlf-gemfile", Driver::ScanVex), + ("crlf-gemfile-source-block", Driver::ScanVexSourceBlock), + ] { + let Some(fx) = + redirect_scanned_project(tag, Spelling::Gemfile, false, true, None, driver).await + else { + return; + }; + let api = fx._server.uri(); + let gemfile_path = fx.proj.join(fx.gemfile_name); + let lock_path = fx.proj.join(fx.lock_name); + // Start over from the pristine pair, its Gemfile in CRLF. + let pristine = String::from_utf8(fx.pristine_gemfile.clone()).unwrap(); + let crlf = pristine.replace("\r\n", "\n").replace('\n', "\r\n"); + let _ = std::fs::remove_dir_all(fx.proj.join(".socket")); + std::fs::write(&gemfile_path, &crlf).unwrap(); + std::fs::write(&lock_path, &fx.pristine_lock).unwrap(); + + let (code, out, err) = run_hosted_scan(&fx.proj, &api); + assert_eq!(code, 0, "{tag}: hosted scan: {out}\n{err}"); + let gemfile = std::fs::read_to_string(&gemfile_path).unwrap(); + assert!( + gemfile.contains(&fx.index_url), + "{tag}: the Gemfile is redirected: {gemfile:?}" + ); + assert!( + all_crlf(&gemfile), + "{tag}: the redirected Gemfile mixes line endings: {gemfile:?}" + ); + + let install = bundle(&fx.proj, &["install"]); + assert!( + install.status.success(), + "{tag}: converging install of the CRLF Gemfile:\n{}", + String::from_utf8_lossy(&install.stderr) + ); + let (code, out, err) = run_hosted_scan(&fx.proj, &api); + assert_eq!(code, 0, "{tag}: re-scan: {out}\n{err}"); + assert_eq!( + std::fs::read_to_string(&gemfile_path).unwrap(), + gemfile, + "{tag}: a re-scan leaves the Gemfile byte-identical" + ); + + let upstream = format!("{api}/upstream"); + let cwd = fx.proj.to_str().expect("utf8 tmp path"); + let (code, stdout, stderr) = run_socket_env( + &fx.proj, + &[ + "rollback", + PURL, + "--json", + "--cwd", + cwd, + "--api-url", + &api, + "--org", + ORG, + "--api-token", + "fake", + "--patch-server-url", + &api, + ], + &[("SOCKET_RUBYGEMS_URL", &upstream)], + ); + assert_eq!(code, 0, "{tag}: rollback: {stdout}\nstderr:\n{stderr}"); + let restored = std::fs::read_to_string(&gemfile_path).unwrap(); + assert!( + !restored.contains("patch-registry") + && restored.contains(&format!("gem \"{DEP}\"")) + && all_crlf(&restored), + "{tag}: rollback restores a CRLF Gemfile declaring the gem: {restored:?}" + ); + } +} diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index ccb8bb226..05bb4b1ce 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -25,6 +25,7 @@ use serde_json::{json, Value}; use crate::formats::yarn::berry_gates::{self, Yarnrc}; use crate::utils::digest::is_hex64_lower; +use crate::utils::line_endings; #[cfg(test)] use crate::utils::line_endings::LineEndings; use crate::vendor::common::{parse_json_text, JsonLayout}; @@ -8004,8 +8005,20 @@ fn rewrite_gem( let raw_tail = m .get(2) .expect("gem_line_re always captures group 2 (tail)") - .as_str() - .to_string(); + .as_str(); + // The CR of a CRLF line is its ending, not part of the + // declaration: the block's own lines take that ending + // and the CR goes back at its end (#1831). An + // unterminated last line takes the file's. + let cr = if raw_tail.ends_with('\r') { "\r" } else { "" }; + let raw_tail = raw_tail.strip_suffix('\r').unwrap_or(raw_tail).to_string(); + let nl = if !cr.is_empty() { + "\r\n" + } else if range.end < gf.len() { + "\n" + } else { + line_endings::terminator(gf) + }; // A parenthesized call keeps its closing `)` in the tail: // strip it (dropping any comment with it), or fail closed // when it is absent (the call continues past this line). @@ -8075,12 +8088,12 @@ fn rewrite_gem( let opts = gemfile::trailing_options(&tail); let block = if opts.is_empty() { format!( - "source \"{}\" do\n gem \"{}\", \"{}\"\nend", + "source \"{}\" do{nl} gem \"{}\", \"{}\"{nl}end", ov.index_url, dep.name, dep.version ) } else { format!( - "source \"{}\" do\n gem \"{}\", \"{}\", {opts}\nend", + "source \"{}\" do{nl} gem \"{}\", \"{}\", {opts}{nl}end", ov.index_url, dep.name, dep.version ) }; @@ -8089,7 +8102,7 @@ fn rewrite_gem( // duplicate earlier in the file and corrupt it (and the // block may carry user text a regex replacement would // `$`-expand). - gf.replace_range(range, &block); + gf.replace_range(range, &format!("{block}{cr}")); gemfile_changed = true; result.edits.push(FileEdit { path: gemfile_name.into(), @@ -8131,9 +8144,11 @@ fn rewrite_gem( }); continue; } else { - // Genuinely undeclared (a transitive dep): append a block. + // Genuinely undeclared (a transitive dep): append a block + // in the file's line endings (#1831). + let eol = line_endings::terminator(gf); let block = format!( - "source \"{}\" do\n gem \"{}\", \"{}\"\nend", + "source \"{}\" do{eol} gem \"{}\", \"{}\"{eol}end", ov.index_url, dep.name, dep.version ); // Always one blank line before the block: the in-place @@ -8141,13 +8156,12 @@ fn rewrite_gem( // declaration, so this is what lets `rollback` / // `remove` prove the block is this append and drop it // instead of restoring a direct exact pin (#457). - let eol = if gf.contains("\r\n") { "\r\n" } else { "\n" }; let sep = if gf.is_empty() || gf.ends_with('\n') { eol.to_string() } else { eol.repeat(2) }; - *gf = format!("{gf}{sep}{block}\n"); + *gf = format!("{gf}{sep}{block}{eol}"); gemfile_changed = true; result.edits.push(FileEdit { path: gemfile_name.into(), @@ -18019,6 +18033,120 @@ mod tests { ); } + /// Whether every line break in `text` is CRLF (and no CR stands alone). + fn all_crlf(text: &str) -> bool { + let crlf = text.matches("\r\n").count(); + crlf > 0 && crlf == text.matches('\n').count() && crlf == text.matches('\r').count() + } + + /// #1831: the Socket source block takes the CRLF endings of the Gemfile + /// it is written into, for a rewritten direct declaration (plain, with + /// options, parenthesized, inside a user `source … do` block, on an + /// unterminated last line) and for the block appended for a transitive + /// gem. The re-run over the result is a no-op. + #[test] + fn gemfile_source_block_keeps_crlf_line_endings() { + let direct = gem_lock_resolving(&[("rails", "7.0.0", &[])], &["rails"]); + let transitive = gem_lock_resolving( + &[ + ("puma", "6.0.0", &["rails (>= 0)"]), + ("rails", "7.0.0", &[]), + ], + &["puma"], + ); + let head = "source \"https://rubygems.org\"\r\n\r\n"; + let block = "source \"https://patch.test/gem/tok/uuid/\" do\r\n gem \"rails\", \"7.0.0\""; + for (gemfile, lock, want) in [ + ( + format!("{head}gem \"rails\"\r\n"), + &direct, + format!("{block}\r\nend\r\n"), + ), + ( + format!("{head}gem \"rails\", require: false # boot\r\n"), + &direct, + format!("{block}, require: false # boot\r\nend\r\n"), + ), + ( + format!("{head}gem(\"rails\")\r\n"), + &direct, + format!("{block}\r\nend\r\n"), + ), + ( + format!("{head}source \"https://gems.example\" do\r\n gem \"rails\"\r\nend\r\n"), + &direct, + format!("do\r\n{block}\r\nend\r\nend\r\n"), + ), + ( + format!("{head}gem \"rails\""), + &direct, + format!("{block}\r\nend"), + ), + ( + format!("{head}gem \"puma\"\r\n"), + &transitive, + format!("gem \"puma\"\r\n\r\n{block}\r\nend\r\n"), + ), + ( + format!("{head}gem \"puma\""), + &transitive, + format!("gem \"puma\"\r\n\r\n{block}\r\nend\r\n"), + ), + ] { + let mut files = BTreeMap::new(); + files.insert("Gemfile".to_string(), gemfile.clone()); + files.insert("Gemfile.lock".to_string(), lock.to_string()); + let res = rewrite_registry_redirect(&files, &[gem_override("rails", "7.0.0")]); + let out = res + .files + .get("Gemfile") + .unwrap_or_else(|| panic!("{gemfile:?} is redirected: {:?}", res.warnings)); + assert!(all_crlf(out), "{gemfile:?}: mixed line endings: {out:?}"); + assert!(out.ends_with(&want), "{gemfile:?}: {out:?}"); + + files.insert("Gemfile".to_string(), out.clone()); + files.insert( + "Gemfile.lock".to_string(), + res.files + .get("Gemfile.lock") + .cloned() + .unwrap_or_else(|| lock.to_string()), + ); + let again = rewrite_registry_redirect(&files, &[gem_override("rails", "7.0.0")]); + assert!( + !again.files.contains_key("Gemfile"), + "{gemfile:?}: re-run rewrote the Gemfile: {:?}", + again.files.get("Gemfile") + ); + } + } + + /// #1831: an LF Gemfile still gets an LF block, and a line ending in LF + /// inside a mostly-CRLF file keeps its own LF (the block takes the + /// ending of the line it replaces). + #[test] + fn gemfile_source_block_keeps_lf_line_endings() { + let direct = gem_lock_resolving(&[("rails", "7.0.0", &[])], &["rails"]); + for (gemfile, want) in [ + ( + "source \"https://rubygems.org\"\n\ngem \"rails\"\n", + "source \"https://patch.test/gem/tok/uuid/\" do\n gem \"rails\", \"7.0.0\"\nend\n", + ), + ( + "source \"https://rubygems.org\"\r\n\r\ngem \"rails\"\n", + "source \"https://patch.test/gem/tok/uuid/\" do\n gem \"rails\", \"7.0.0\"\nend\n", + ), + ] { + let mut files = BTreeMap::new(); + files.insert("Gemfile".to_string(), gemfile.to_string()); + files.insert("Gemfile.lock".to_string(), direct.clone()); + let res = rewrite_registry_redirect(&files, &[gem_override("rails", "7.0.0")]); + let out = res.files.get("Gemfile").expect("redirected"); + assert!(out.ends_with(want), "{gemfile:?}: {out:?}"); + assert!(!out.contains("\r\r"), "{out:?}"); + } + } + /// A gem-level source option (`git:` / `path:` / `github:` / `source:`) /// preserved into the Socket source block OVERRIDES it in bundler's DSL, /// leaving the redirect a silent no-op that still gets attested. Fail @@ -18762,10 +18890,12 @@ mod tests { r.warnings ); let out = r.files.get("Gemfile").expect("declaration rewritten"); + // A CRLF declaration's block takes its CRLF (#1831). + let nl = if decl.ends_with('\r') { "\r\n" } else { "\n" }; let want = if opts.is_empty() { - " gem \"vuln-gem\", \"1.0.0\"\nend".to_string() + format!(" gem \"vuln-gem\", \"1.0.0\"{nl}end") } else { - format!(" gem \"vuln-gem\", \"1.0.0\", {opts}\nend") + format!(" gem \"vuln-gem\", \"1.0.0\", {opts}{nl}end") }; assert!(out.contains(&want), "{decl:?}: {out}"); } From 0ea0db35ef90af45c440ac7db8b3ee1d5cb179cc Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 17:05:21 +0000 Subject: [PATCH 3/4] Match gem rollback to the block's line ending Rollback restored a hosted gem declaration with CRLF whenever the Gemfile held any CRLF line. It now takes the ending of the Socket block it replaces, which the redirect writes in the ending of the original line, so a mixed-ending Gemfile round-trips. A gem line that ends in a lone CR at end of file no longer leaves a stray CR, and the recorded edit now shows exactly what was written. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/patch/redirect/mod.rs | 47 ++++++++++--------- .../src/patch/redirect/upstream/gem.rs | 13 +++-- 2 files changed, 36 insertions(+), 24 deletions(-) diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index 05bb4b1ce..18eb2007f 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -7650,7 +7650,8 @@ pub(crate) fn gem_index_url_pattern(dep: &DepOverride, index_url: &str) -> Strin /// divergence the rewriter itself created. Trailing whitespace is trimmed (a /// block appended to a newline-less file adds a final newline the other /// spelling never had). `\r?` mirrors the block recognizer in `rewrite_gem`: -/// a `core.autocrlf` checkout rewrites run 1's LF block to CRLF, and a block +/// a CRLF manifest gets a CRLF block (#1831) and a `core.autocrlf` checkout +/// rewrites run 1's LF block to CRLF, and a block /// the recognizer accepts must also be erased here or the re-run is trapped /// behind the divergence warning before it can reach the recognizer. fn gem_spelling_residue(content: &str, deps: &[&DepOverride]) -> String { @@ -7951,10 +7952,11 @@ fn rewrite_gem( // Wildcard the rotating segments instead (mirrors the CHECKSUMS // at-target guard below). let url_pat = gem_index_url_pattern(dep, &ov.index_url); - // `\r?\n`: the rewriter emits LF, but a `core.autocrlf` checkout - // rewrites the working tree to CRLF — the guard must still - // recognize the block there, or the indented `gem` line inside - // it falls through to `gem_line_re` and gets wrapped again. + // `\r?\n`: the rewriter writes a CRLF Gemfile's block in CRLF + // (#1831), and a `core.autocrlf` checkout rewrites an LF one to + // CRLF — the guard must recognize the block either way, or the + // indented `gem` line inside it falls through to `gem_line_re` + // and gets wrapped again. let block_re = Regex::new( &(String::from(r#"(?m)^source "("#) + &url_pat @@ -8009,15 +8011,19 @@ fn rewrite_gem( // The CR of a CRLF line is its ending, not part of the // declaration: the block's own lines take that ending // and the CR goes back at its end (#1831). An - // unterminated last line takes the file's. - let cr = if raw_tail.ends_with('\r') { "\r" } else { "" }; - let raw_tail = raw_tail.strip_suffix('\r').unwrap_or(raw_tail).to_string(); - let nl = if !cr.is_empty() { - "\r\n" - } else if range.end < gf.len() { - "\n" + // unterminated last line (a lone trailing CR included) + // takes the file's. + let terminated = range.end < gf.len(); + let cr = if terminated && raw_tail.ends_with('\r') { + "\r" } else { - line_endings::terminator(gf) + "" + }; + let raw_tail = raw_tail.strip_suffix('\r').unwrap_or(raw_tail).to_string(); + let nl = match (cr, terminated) { + ("\r", _) => "\r\n", + (_, true) => "\n", + _ => line_endings::terminator(gf), }; // A parenthesized call keeps its closing `)` in the tail: // strip it (dropping any comment with it), or fail closed @@ -8102,7 +8108,8 @@ fn rewrite_gem( // duplicate earlier in the file and corrupt it (and the // block may carry user text a regex replacement would // `$`-expand). - gf.replace_range(range, &format!("{block}{cr}")); + let block = format!("{block}{cr}"); + gf.replace_range(range, &block); gemfile_changed = true; result.edits.push(FileEdit { path: gemfile_name.into(), @@ -18033,12 +18040,6 @@ mod tests { ); } - /// Whether every line break in `text` is CRLF (and no CR stands alone). - fn all_crlf(text: &str) -> bool { - let crlf = text.matches("\r\n").count(); - crlf > 0 && crlf == text.matches('\n').count() && crlf == text.matches('\r').count() - } - /// #1831: the Socket source block takes the CRLF endings of the Gemfile /// it is written into, for a rewritten direct declaration (plain, with /// options, parenthesized, inside a user `source … do` block, on an @@ -18101,7 +18102,11 @@ mod tests { .files .get("Gemfile") .unwrap_or_else(|| panic!("{gemfile:?} is redirected: {:?}", res.warnings)); - assert!(all_crlf(out), "{gemfile:?}: mixed line endings: {out:?}"); + assert_eq!( + LineEndings::of(out), + LineEndings::Crlf, + "{gemfile:?}: mixed line endings: {out:?}" + ); assert!(out.ends_with(&want), "{gemfile:?}: {out:?}"); files.insert("Gemfile".to_string(), out.clone()); diff --git a/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs b/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs index c49eaf4ec..8a425c34e 100644 --- a/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs @@ -62,7 +62,7 @@ use super::{Ctx, FormatResult, HostedPin, UpstreamClient, View}; use crate::formats::gem::{ bundler_manifest_for, parse_spec, same_remote, split_checksum_entry, BUNDLER_LOCKS, }; -use crate::utils::line_endings::{to_lf, LineEndings}; +use crate::utils::line_endings::{self, to_lf, LineEndings}; use crate::vendor::gem::{gem_declaration_any, quoted_literal}; /// The default upstream `GEM` remote. @@ -1178,7 +1178,14 @@ fn line_before_start(text: &str, at: usize) -> usize { } fn restore_manifest(text: &str, block: &Block, gem: &Gem<'_>, decl: &Decl) -> String { - let eol = if text.contains("\r\n") { "\r\n" } else { "\n" }; + // The restored line takes the block's ending: the rewriter writes it in + // the ending of the line it replaced (#1831). A block in LF, the only + // spelling before #1831, takes the ending of the lines around it. + let eol = if text[block.start..block.end].contains("\r\n") { + "\r\n" + } else { + line_endings::terminator(&format!("{}{}", &text[..block.start], &text[block.end..])) + }; let mut start = block.start; let replacement = match decl { // The append's own blank separator goes with it. @@ -1785,7 +1792,7 @@ mod tests { run(&t, &direct), "group :test do\n gem \"rails\", \"7.0.0\", require: false\nend\n" ); - // CRLF manifest (the rewriter wrote its block in LF). + // CRLF manifest, its block in LF (written before #1831). let t = format!("source \"https://rubygems.org\"\r\ngem \"puma\"\r\n{block}\n"); assert_eq!( run(&t, &direct), From 210c955b3813d9b5c0d4a3916b0a9c66e3a819ea Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 17:08:48 +0000 Subject: [PATCH 4/4] Test a lone trailing CR in a hosted Gemfile Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-core/src/patch/redirect/mod.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index 18eb2007f..f209cd7a1 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -18083,6 +18083,12 @@ mod tests { &direct, format!("{block}\r\nend"), ), + // A lone CR at the end of the file is not a line ending. + ( + format!("{head}gem \"rails\"\r"), + &direct, + format!("{block}\r\nend"), + ), ( format!("{head}gem \"puma\"\r\n"), &transitive,