From 4f53e5d5ca94aeab62d52f0717d1a0ac0cf82e72 Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Tue, 1 Sep 2026 11:35:48 -0400 Subject: [PATCH 01/24] ZJIT: Skip failing `test_specialize_inlined_megamorphic_receiver` I tried fixing the test for a bit but it's not clear to me what the test expects. Skip for now; I've pinged the author. --- zjit/src/hir/opt_tests.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/zjit/src/hir/opt_tests.rs b/zjit/src/hir/opt_tests.rs index 7d0c35244fdb83..98b236e3049532 100644 --- a/zjit/src/hir/opt_tests.rs +++ b/zjit/src/hir/opt_tests.rs @@ -18117,6 +18117,7 @@ mod hir_opt_tests { } #[test] + #[ignore = "started failing after profiling bucket was changed from 4 to 8"] fn test_specialize_inlined_megamorphic_receiver() { set_call_threshold(6); eval(" From 8d80fd4c066624f13c0725bfd9be54e3865854b7 Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Tue, 1 Sep 2026 11:24:46 -0700 Subject: [PATCH 02/24] Mark rb_zjit_throw as NORETURN MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes a compiler warning: In file included from ../vm.c:676: ../vm_insnhelper.c: In function ‘rb_zjit_throw’: ../vm_insnhelper.c:1886:1: warning: function might be candidate for attribute ‘noreturn’ [-Wsuggest-attribute=noreturn] 1886 | rb_zjit_throw(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp, rb_num_t throw_state, VALUE throwobj) | ^~~~~~~~~~~~~ --- vm_insnhelper.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/vm_insnhelper.c b/vm_insnhelper.c index ea4deb17727959..0542221f58d33e 100644 --- a/vm_insnhelper.c +++ b/vm_insnhelper.c @@ -1881,6 +1881,8 @@ rb_vm_throw(const rb_execution_context_t *ec, rb_control_frame_t *reg_cfp, rb_nu return vm_throw(ec, reg_cfp, throw_state, throwobj); } +NORETURN(VALUE rb_zjit_throw(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp, rb_num_t throw_state, VALUE throwobj)); + // Fallback for ZJIT. Make a longjmp and unwind to the most recent vm_exec(). VALUE rb_zjit_throw(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp, rb_num_t throw_state, VALUE throwobj) From 0ea980a7396cfa81985b24a49641ee8d138303b4 Mon Sep 17 00:00:00 2001 From: Max Bernstein Date: Tue, 1 Sep 2026 15:51:47 -0400 Subject: [PATCH 03/24] ZJIT: Fix test for new distribution size (#18593) We need to call more and have more classes for this test to make sense. --- zjit/src/hir/opt_tests.rs | 16 ++++++++-------- zjit/src/profile.rs | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/zjit/src/hir/opt_tests.rs b/zjit/src/hir/opt_tests.rs index 98b236e3049532..d2adf0478684c6 100644 --- a/zjit/src/hir/opt_tests.rs +++ b/zjit/src/hir/opt_tests.rs @@ -18117,17 +18117,17 @@ mod hir_opt_tests { } #[test] - #[ignore = "started failing after profiling bucket was changed from 4 to 8"] fn test_specialize_inlined_megamorphic_receiver() { - set_call_threshold(6); + assert_eq!(crate::profile::DISTRIBUTION_SIZE, 8, "If you change distribution size, update the number of classes used"); + set_call_threshold((crate::profile::DISTRIBUTION_SIZE + 2).try_into().unwrap()); eval(" def klass_eq(klass) = klass == Integer def test = klass_eq(String) - # 5 distinct receiver classes at the == site: one more than the profile's - # 4 buckets, so the distribution is megamorphic. - klass_eq(Integer); klass_eq(Array); klass_eq(Hash); klass_eq(Symbol); klass_eq(Float) + # 9 distinct receiver classes at the == site: one more than the profile's + # 8 buckets, so the distribution is megamorphic. + klass_eq(Integer); klass_eq(Array); klass_eq(Hash); klass_eq(Symbol); klass_eq(Float); klass_eq(NilClass); klass_eq(TrueClass); klass_eq(FalseClass); klass_eq(String) 6.times { test } "); assert_snapshot!(hir_string("test"), @" @@ -18149,11 +18149,11 @@ mod hir_opt_tests { PatchPoint StableConstantNames(0x1068, Integer) v31:ClassSubclass[Integer@0x1070] = Const Value(VALUE(0x1070)) PatchPoint MethodRedefined(Class@0x1078, ==@0x1080, cme:0x1088) - v45:CBool = IsBitEqual v12, v31 - v46:BoolExact = BoxBool v45 + v82:CBool = IsBitEqual v12, v31 + v83:BoolExact = BoxBool v82 CheckInterrupts PopInlineFrame - Return v46 + Return v83 "); } diff --git a/zjit/src/profile.rs b/zjit/src/profile.rs index 38cb44576aa4c1..f4a69d3afec3a0 100644 --- a/zjit/src/profile.rs +++ b/zjit/src/profile.rs @@ -143,7 +143,7 @@ pub fn num_arguments_on_stack(cd: *const rb_call_data) -> usize { (unsafe { vm_ci_argc(ci) }) as usize + has_blockarg as usize } -const DISTRIBUTION_SIZE: usize = 8; +pub const DISTRIBUTION_SIZE: usize = 8; pub type TypeDistribution = Distribution; From b59ea8efc7fd0c8e599e97e32ef1d54afc8abaf5 Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Tue, 1 Sep 2026 13:24:47 -0700 Subject: [PATCH 04/24] ZJIT: Reserve a label reference's bytes by writing them (#18519) --- zjit/src/asm/mod.rs | 48 +++++++++++++++++++++++++++++++++++++++------ 1 file changed, 42 insertions(+), 6 deletions(-) diff --git a/zjit/src/asm/mod.rs b/zjit/src/asm/mod.rs index d45e323253fbff..c0c03ba0ca0685 100644 --- a/zjit/src/asm/mod.rs +++ b/zjit/src/asm/mod.rs @@ -249,12 +249,13 @@ impl CodeBlock { // Keep track of the reference self.label_refs.push(LabelRef { pos: self.write_pos, label, num_bytes, encode: Box::new(encode) }); - // Move past however many bytes the instruction takes up - if self.write_pos + num_bytes < self.mem_size { - self.write_pos += num_bytes; - } else { - self.dropped_bytes = true; // retry emitting the Insn after next_page - } + // Move past however many bytes the instruction takes up. + // Reserve the bytes by writing them rather than by moving the cursor + // over them. Pages are mapped on first write, so a cursor bump alone + // leaves a page that cannot be mapped. + const RESERVED: [u8; 16] = [0; 16]; + assert!(num_bytes <= RESERVED.len(), "label reference wants {num_bytes} bytes"); + self.write_bytes(&RESERVED[..num_bytes]); } // Link internal label references @@ -470,4 +471,39 @@ mod tests assert_eq!(uimm_num_bits((u32::MAX as u64) + 1), 64); assert_eq!(uimm_num_bits(u64::MAX), 64); } + + #[test] + fn test_label_ref_at_an_unmappable_page_sets_dropped_bytes() { + // Two pages of address space, but a memory limit that only lets + // the first page be mapped. + let page_size = unsafe { crate::cruby::rb_jit_get_page_size() } as usize; + let limit = crate::stats::zjit_alloc_bytes() + page_size + page_size / 2; + let virt_mem = VirtualMem::alloc(2 * page_size, Some(limit)); + let mut cb = CodeBlock::new(Rc::new(RefCell::new(virt_mem)), false); + + // Fill the mappable page up to 2 bytes below its end, so that + // a 5-byte jump reservation straddles into the unmappable page. + for _ in 0..(page_size - 2) { + cb.write_byte(0x90); + } + assert!(!cb.has_dropped_bytes(), "the first page must map"); + + // label_ref() must reserve its bytes by writing them, so that the + // unmappable page surfaces as dropped_bytes here, where the caller + // still reads it and turns it into CompileError::OutOfMemory. + let label = cb.new_label("over_the_page_boundary".to_string()); + cb.label_ref(label, 5, |cb, _, _| { + cb.write_bytes(&[0; 5]); + Ok(()) + }); + cb.write_label(label); + + // Mirror what compile_with_regs does after emit. It calls link_labels() + // only if emit did not go OOM. This reproduces an assertion failure in + // link_labels() when dropped_bytes isn't updated properly. + if !cb.has_dropped_bytes() { + cb.link_labels().unwrap(); + } + assert!(cb.has_dropped_bytes(), "the reservation must discover the unmappable page"); + } } From 18ef46d7acfbba82c350f740405581a3f87cda63 Mon Sep 17 00:00:00 2001 From: XrXr Date: Mon, 31 Aug 2026 20:09:16 -0400 Subject: [PATCH 05/24] ZJIT: Drop `PartialEq` from `FrameState` No one was actually doing equality comparison on `FrameState`s and it was only derived because `FrameState` was in `ParseError::StackUnderflow`. No one was reading the `FrameState` out of `StackUnderflow` either. Save some binary size. --- zjit/src/hir.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index afc0e10bc46c2d..44c5da93f37dab 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -8058,7 +8058,7 @@ impl<'a> std::fmt::Display for FunctionPrinter<'a> { } } -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone)] pub struct FrameState { pub iseq: IseqPtr, insn_idx: YarvInsnIdx, @@ -8177,14 +8177,14 @@ impl FrameState { /// Pop a stack operand fn stack_pop(&mut self) -> Result { - self.stack.pop().ok_or_else(|| ParseError::StackUnderflow(self.clone())) + self.stack.pop().ok_or_else(|| ParseError::StackUnderflow(self.insn_idx)) } fn stack_pop_n(&mut self, count: usize) -> Result, ParseError> { // Check if we have enough values on the stack let stack_len = self.stack.len(); if stack_len < count { - return Err(ParseError::StackUnderflow(self.clone())); + return Err(ParseError::StackUnderflow(self.insn_idx)); } Ok(self.stack.split_off(stack_len - count)) @@ -8192,7 +8192,7 @@ impl FrameState { /// Get a stack-top operand fn stack_top(&self) -> Result { - self.stack.last().ok_or_else(|| ParseError::StackUnderflow(self.clone())).copied() + self.stack.last().ok_or_else(|| ParseError::StackUnderflow(self.insn_idx)).copied() } /// Set a stack operand at idx @@ -8204,9 +8204,9 @@ impl FrameState { /// Get a stack operand at idx fn stack_topn(&self, idx: usize) -> Result { let Some(idx) = self.stack.len().checked_sub(idx + 1) else { - return Err(ParseError::StackUnderflow(self.clone())); + return Err(ParseError::StackUnderflow(self.insn_idx)); }; - self.stack.get(idx).ok_or_else(|| ParseError::StackUnderflow(self.clone())).copied() + self.stack.get(idx).ok_or_else(|| ParseError::StackUnderflow(self.insn_idx)).copied() } fn setlocal(&mut self, ep_offset: u32, opnd: InsnId) { @@ -8327,7 +8327,8 @@ pub enum CallType { #[derive(Clone, Debug, PartialEq)] pub enum ParseError { - StackUnderflow(FrameState), + /// Instruction index of the YARV instruction that underflowed the stack. + StackUnderflow(YarvInsnIdx), MalformedIseq(u32), // insn_idx into iseq_encoded Validation(ValidationError), NotAllowed, From 837df2793b95b5d653d9c7d7351b90faf67b0946 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Tue, 1 Sep 2026 09:46:22 +0200 Subject: [PATCH 06/24] string.c: remove impossible condition `s` is the current string pointer (`self`), `t` is the new buffer pointer initialized with `malloc / realloc`. Hence this condition is always true. --- string.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/string.c b/string.c index 09384e4470b1c7..e4503a96bf1f2d 100644 --- a/string.c +++ b/string.c @@ -9158,11 +9158,10 @@ tr_trans(VALUE str, VALUE src, VALUE repl, int sflag) SIZED_REALLOC_N(buf, unsigned char, max + termlen, old); t = buf + offset; } - if (s != t) { - rb_enc_mbcput(c, t, enc); - if (may_modify && memcmp(s, t, tlen) != 0) { - modify = 1; - } + + rb_enc_mbcput(c, t, enc); + if (may_modify && memcmp(s, t, tlen) != 0) { + modify = 1; } CHECK_IF_ASCII(c); s += clen; From c92faa59c77fb4815e9d89cd520ae40a122cf499 Mon Sep 17 00:00:00 2001 From: BurdetteLamar Date: Tue, 1 Sep 2026 14:44:29 -0500 Subject: [PATCH 07/24] [DOC] Doc for File.identical? --- file.c | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/file.c b/file.c index 8f860f3dc10f74..7193bc28dd99f2 100644 --- a/file.c +++ b/file.c @@ -2482,21 +2482,29 @@ rb_file_sticky_p(VALUE obj, VALUE fname) /* * call-seq: - * File.identical?(file_1, file_2) -> true or false + * File.identical?(object_0, object_1) -> true or false * - * Returns true if the named files are identical. + * Returns whether the given objects represent filesystem entries that are identical; + * each object may be a string path or an IO object: * - * _file_1_ and _file_2_ can be an IO object. + * # Paths. + * File.identical?('README.md', 'README.md') # => true # Same path. + * File.identical?('README.md', './README.md') # => true # Same entry. + * File.identical?('.', '.') # => true # Directory. + * File.identical?('README.md', 'LEGAL') # => false + * File.identical?('README.md', 'nosuch') # => false # Non-existent entry. + * # Links and File object. + * File.link('README.md', 'link') # Symbolic link. + * File.symlink('README.md', 'symlink') # Hard link. + * file = File.open('README.md', 'r') # File object. + * File.identical?('README.md', 'link') # => true + * File.identical?('README.md', 'symlink') # => true + * File.identical?('README.md', file) # => true + * # Clean up. + * File.unlink('link') + * File.unlink('symlink') + * file.close * - * open("a", "w") {} - * p File.identical?("a", "a") #=> true - * p File.identical?("a", "./a") #=> true - * File.link("a", "b") - * p File.identical?("a", "b") #=> true - * File.symlink("a", "c") - * p File.identical?("a", "c") #=> true - * open("d", "w") {} - * p File.identical?("a", "d") #=> false */ static VALUE From ae0d477c712a9fc0e87e4aee23147b7991d3bcdc Mon Sep 17 00:00:00 2001 From: thyogo Date: Wed, 2 Sep 2026 09:24:37 +0900 Subject: [PATCH 08/24] [ruby/time] Fix wrong result in Time.rfc2822 documentation (https://github.com/ruby/time/pull/76) The sample code and its result in the Time.rfc2822 documentation appear to be inconsistent. https://github.com/ruby/time/commit/b602d284aa --- lib/time.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/time.rb b/lib/time.rb index 577d0060327212..b157e60863354e 100644 --- a/lib/time.rb +++ b/lib/time.rb @@ -510,7 +510,7 @@ def strptime(date, format, now=self.now) # require 'time' # # Time.rfc2822("Wed, 05 Oct 2011 22:26:12 -0400") - # #=> 2010-10-05 22:26:12 -0400 + # #=> 2011-10-05 22:26:12 -0400 # # You must require 'time' to use this method. # From 2742123f8c1ca3f61ca4527b703d87598641a25f Mon Sep 17 00:00:00 2001 From: "Stanislav (Stas) Katkov" Date: Tue, 1 Sep 2026 01:42:17 +0200 Subject: [PATCH 09/24] [ruby/rubygems] Normalize legacy Latin-1 bytes for pre 1.8 ruby gems MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Normalizes legacy Latin-1 bytes in Gem::Specification.normalize_yaml_input. - Works with both Psych and RubyGems’ internal YAML parser. https://github.com/ruby/rubygems/commit/76215364df --- lib/rubygems/specification.rb | 8 +++++++- test/rubygems/test_gem_package.rb | 16 ++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/lib/rubygems/specification.rb b/lib/rubygems/specification.rb index adee800051fc07..370a464c10ad7c 100644 --- a/lib/rubygems/specification.rb +++ b/lib/rubygems/specification.rb @@ -1143,8 +1143,14 @@ def self.non_nil_attributes def self.normalize_yaml_input(input) result = input.respond_to?(:read) ? input.read : input - result = "--- " + result unless result.start_with?("--- ") result = result.dup + # Ruby 1.8 gem metadata may contain Latin-1 bytes without an encoding declaration. + if [Encoding::BINARY, Encoding::UTF_8].include?(result.encoding) + result.force_encoding(Encoding::UTF_8).scrub! do |bytes| + bytes.encode(Encoding::UTF_8, Encoding::ISO_8859_1) + end + end + result = "--- " + result unless result.start_with?("--- ") result.gsub!(/ !!null \n/, " \n") # date: 2011-04-26 00:00:00.000000000Z # date: 2011-04-26 00:00:00.000000000 Z diff --git a/test/rubygems/test_gem_package.rb b/test/rubygems/test_gem_package.rb index 7b8ac4736d2f46..58562c1c949036 100644 --- a/test/rubygems/test_gem_package.rb +++ b/test/rubygems/test_gem_package.rb @@ -944,6 +944,22 @@ def entry.full_name assert_equal @spec, spec end + def test_load_spec_from_metadata_with_latin1 + @spec.description = "Based on Mauricio Fern\u00E1ndez's implementation" + metadata = @spec.to_yaml.encode Encoding::ISO_8859_1 + entry = StringIO.new Gem::Util.gzip metadata + def entry.full_name + "metadata.gz" + end + + package = Gem::Package.new "nonexistent.gem" + + spec = package.load_spec_from_metadata entry + + assert_equal @spec, spec + assert_equal @spec.description, spec.description + end + def test_verify package = Gem::Package.new @gem From 900dbd0b3a352916928ff4c5b0e4a1093b64a0bb Mon Sep 17 00:00:00 2001 From: "Stanislav (Stas) Katkov" Date: Tue, 1 Sep 2026 01:54:37 +0200 Subject: [PATCH 10/24] [ruby/rubygems] simplify code https://github.com/ruby/rubygems/commit/c4793effbf --- lib/rubygems/specification.rb | 4 +--- test/rubygems/test_gem_package.rb | 16 +--------------- 2 files changed, 2 insertions(+), 18 deletions(-) diff --git a/lib/rubygems/specification.rb b/lib/rubygems/specification.rb index 370a464c10ad7c..230cfecb89683e 100644 --- a/lib/rubygems/specification.rb +++ b/lib/rubygems/specification.rb @@ -1146,9 +1146,7 @@ def self.normalize_yaml_input(input) result = result.dup # Ruby 1.8 gem metadata may contain Latin-1 bytes without an encoding declaration. if [Encoding::BINARY, Encoding::UTF_8].include?(result.encoding) - result.force_encoding(Encoding::UTF_8).scrub! do |bytes| - bytes.encode(Encoding::UTF_8, Encoding::ISO_8859_1) - end + result.force_encoding(Encoding::UTF_8).scrub! {|bytes| bytes.encode(Encoding::UTF_8, Encoding::ISO_8859_1) } end result = "--- " + result unless result.start_with?("--- ") result.gsub!(/ !!null \n/, " \n") diff --git a/test/rubygems/test_gem_package.rb b/test/rubygems/test_gem_package.rb index 58562c1c949036..1178482291a48f 100644 --- a/test/rubygems/test_gem_package.rb +++ b/test/rubygems/test_gem_package.rb @@ -932,22 +932,8 @@ def test_extract_tar_gz_rejects_suffix_escape end def test_load_spec_from_metadata - entry = StringIO.new Gem::Util.gzip @spec.to_yaml - def entry.full_name - "metadata.gz" - end - - package = Gem::Package.new "nonexistent.gem" - - spec = package.load_spec_from_metadata entry - - assert_equal @spec, spec - end - - def test_load_spec_from_metadata_with_latin1 @spec.description = "Based on Mauricio Fern\u00E1ndez's implementation" - metadata = @spec.to_yaml.encode Encoding::ISO_8859_1 - entry = StringIO.new Gem::Util.gzip metadata + entry = StringIO.new Gem::Util.gzip @spec.to_yaml.encode(Encoding::ISO_8859_1) def entry.full_name "metadata.gz" end From 03a95577c1cc5b60fd90ce086d48c28071ccf788 Mon Sep 17 00:00:00 2001 From: "Stanislav (Stas) Katkov" Date: Tue, 1 Sep 2026 13:59:59 +0200 Subject: [PATCH 11/24] [ruby/rubygems] Preserve non-UTF-8 legacy gem metadata Fall back to RubyGems' safe YAML parser when Psych cannot accept the metadata as UTF-8. This preserves bytes from ISO-8859-1, EUC-JP, and other legacy encodings without guessing their encoding. https://github.com/ruby/rubygems/commit/503e144810 --- lib/rubygems/safe_yaml.rb | 9 ++++++++- lib/rubygems/specification.rb | 6 +----- test/rubygems/test_gem_package.rb | 27 ++++++++++++++++----------- 3 files changed, 25 insertions(+), 17 deletions(-) diff --git a/lib/rubygems/safe_yaml.rb b/lib/rubygems/safe_yaml.rb index f4bba001365fea..a23374ca955979 100644 --- a/lib/rubygems/safe_yaml.rb +++ b/lib/rubygems/safe_yaml.rb @@ -35,7 +35,8 @@ def self.aliases_enabled? # :nodoc: end def self.safe_load(input) - if Gem.use_psych? + # Psych rejects legacy metadata bytes, so preserve them with the internal parser. + if Gem.use_psych? && valid_encoding?(input) ::Psych.safe_load(input, permitted_classes: PERMITTED_CLASSES, permitted_symbols: PERMITTED_SYMBOLS, aliases: @aliases_enabled) else @@ -51,5 +52,11 @@ def self.safe_load(input) class << self alias_method :load, :safe_load end + + private_class_method def valid_encoding?(input) + return false unless input.is_a?(String) + + input.dup.force_encoding(Encoding::UTF_8).valid_encoding? + end end end diff --git a/lib/rubygems/specification.rb b/lib/rubygems/specification.rb index 230cfecb89683e..adee800051fc07 100644 --- a/lib/rubygems/specification.rb +++ b/lib/rubygems/specification.rb @@ -1143,12 +1143,8 @@ def self.non_nil_attributes def self.normalize_yaml_input(input) result = input.respond_to?(:read) ? input.read : input - result = result.dup - # Ruby 1.8 gem metadata may contain Latin-1 bytes without an encoding declaration. - if [Encoding::BINARY, Encoding::UTF_8].include?(result.encoding) - result.force_encoding(Encoding::UTF_8).scrub! {|bytes| bytes.encode(Encoding::UTF_8, Encoding::ISO_8859_1) } - end result = "--- " + result unless result.start_with?("--- ") + result = result.dup result.gsub!(/ !!null \n/, " \n") # date: 2011-04-26 00:00:00.000000000Z # date: 2011-04-26 00:00:00.000000000 Z diff --git a/test/rubygems/test_gem_package.rb b/test/rubygems/test_gem_package.rb index 1178482291a48f..2ff144617bc34f 100644 --- a/test/rubygems/test_gem_package.rb +++ b/test/rubygems/test_gem_package.rb @@ -931,19 +931,24 @@ def test_extract_tar_gz_rejects_suffix_escape assert_path_not_exist parent end - def test_load_spec_from_metadata - @spec.description = "Based on Mauricio Fern\u00E1ndez's implementation" - entry = StringIO.new Gem::Util.gzip @spec.to_yaml.encode(Encoding::ISO_8859_1) - def entry.full_name - "metadata.gz" - end - - package = Gem::Package.new "nonexistent.gem" + def test_load_spec_from_metadata_with_legacy_encodings + { + "Based on Mauricio Fern\u00E1ndez's implementation" => Encoding::ISO_8859_1, + "\u65E5\u672C\u8A9E" => Encoding::EUC_JP, + }.each do |description, encoding| + @spec.description = description + metadata = @spec.to_yaml.encode encoding + entry = StringIO.new Gem::Util.gzip metadata + def entry.full_name + "metadata.gz" + end - spec = package.load_spec_from_metadata entry + package = Gem::Package.new "nonexistent.gem" + spec = package.load_spec_from_metadata entry - assert_equal @spec, spec - assert_equal @spec.description, spec.description + assert_equal @spec, spec + assert_equal description.encode(encoding).b, spec.description.b + end end def test_verify From e746974ea18b0d1a4056e8895e3c93833e3a1e98 Mon Sep 17 00:00:00 2001 From: "Stanislav (Stas) Katkov" Date: Tue, 1 Sep 2026 14:12:14 +0200 Subject: [PATCH 12/24] [ruby/rubygems] return a test that executes Psych.safe_load https://github.com/ruby/rubygems/commit/971dfd6e5f --- lib/rubygems/safe_yaml.rb | 4 ++-- test/rubygems/test_gem_package.rb | 13 +++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/lib/rubygems/safe_yaml.rb b/lib/rubygems/safe_yaml.rb index a23374ca955979..6ecdd1d50b041e 100644 --- a/lib/rubygems/safe_yaml.rb +++ b/lib/rubygems/safe_yaml.rb @@ -53,8 +53,8 @@ class << self alias_method :load, :safe_load end - private_class_method def valid_encoding?(input) - return false unless input.is_a?(String) + private_class_method def self.valid_encoding?(input) + return true unless input.is_a?(String) input.dup.force_encoding(Encoding::UTF_8).valid_encoding? end diff --git a/test/rubygems/test_gem_package.rb b/test/rubygems/test_gem_package.rb index 2ff144617bc34f..fdc0c22f45ab1b 100644 --- a/test/rubygems/test_gem_package.rb +++ b/test/rubygems/test_gem_package.rb @@ -931,6 +931,19 @@ def test_extract_tar_gz_rejects_suffix_escape assert_path_not_exist parent end + def test_load_spec_from_metadata + entry = StringIO.new Gem::Util.gzip @spec.to_yaml + def entry.full_name + "metadata.gz" + end + + package = Gem::Package.new "nonexistent.gem" + + spec = package.load_spec_from_metadata entry + + assert_equal @spec, spec + end + def test_load_spec_from_metadata_with_legacy_encodings { "Based on Mauricio Fern\u00E1ndez's implementation" => Encoding::ISO_8859_1, From d3c5230a6363263ae2f8df6aea4544989d9247eb Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Mon, 31 Aug 2026 14:47:32 +0900 Subject: [PATCH 13/24] [ruby/rubygems] Let the tests control GITHUB_ACTIONS On GitHub Actions the push tests inherited the real variable, took the auto-attestation path, and spawned `gem exec sigstore-cli` subprocesses. Scrubbing it in setup also made the host and engine skip guards decide nothing, so those tests now set it themselves. https://github.com/ruby/rubygems/commit/8f4bb82839 Co-Authored-By: Claude Fable 5 --- test/rubygems/helper.rb | 1 + .../test_gem_commands_push_command.rb | 64 +++++++++---------- 2 files changed, 33 insertions(+), 32 deletions(-) diff --git a/test/rubygems/helper.rb b/test/rubygems/helper.rb index 1fe8577b4fc94b..66e1e3e293459c 100644 --- a/test/rubygems/helper.rb +++ b/test/rubygems/helper.rb @@ -391,6 +391,7 @@ def setup ENV["XDG_STATE_HOME"] = nil ENV["MAKEFLAGS"] = nil ENV["SOURCE_DATE_EPOCH"] = nil + ENV["GITHUB_ACTIONS"] = nil ENV["BUNDLER_VERSION"] = nil ENV["BUNDLE_CONFIG"] = nil ENV["BUNDLE_USER_CONFIG"] = nil diff --git a/test/rubygems/test_gem_commands_push_command.rb b/test/rubygems/test_gem_commands_push_command.rb index f8bb09d60062ff..c6c1a7401f1083 100644 --- a/test/rubygems/test_gem_commands_push_command.rb +++ b/test/rubygems/test_gem_commands_push_command.rb @@ -122,55 +122,53 @@ def test_execute_attestation_auto omit if RUBY_ENGINE == "jruby" ENV["GITHUB_ACTIONS"] = "true" - begin - @response = "Successfully registered gem: freewill (1.0.0)" - @fetcher.data["#{Gem.host}/api/v1/gems"] = HTTPResponseFactory.create(body: @response, code: 200, msg: "OK") - attestation_path = "#{@path}.sigstore.json" - attestation_content = "auto-attestation" - File.write(attestation_path, attestation_content) - @cmd.options[:args] = [@path] + @response = "Successfully registered gem: freewill (1.0.0)" + @fetcher.data["#{Gem.host}/api/v1/gems"] = HTTPResponseFactory.create(body: @response, code: 200, msg: "OK") - @cmd.stub(:attest!, attestation_path) do - @cmd.execute - end + attestation_path = "#{@path}.sigstore.json" + attestation_content = "auto-attestation" + File.write(attestation_path, attestation_content) + @cmd.options[:args] = [@path] - assert_equal Gem::Net::HTTP::Post, @fetcher.last_request.class - content_length = @fetcher.last_request["Content-Length"].to_i - assert_equal content_length, @fetcher.last_request.body.length - assert_attestation_multipart attestation_content - ensure - ENV.delete("GITHUB_ACTIONS") + @cmd.stub(:attest!, attestation_path) do + @cmd.execute end + + assert_equal Gem::Net::HTTP::Post, @fetcher.last_request.class + content_length = @fetcher.last_request["Content-Length"].to_i + assert_equal content_length, @fetcher.last_request.body.length + assert_attestation_multipart attestation_content end def test_execute_attestation_fallback omit if RUBY_ENGINE == "jruby" ENV["GITHUB_ACTIONS"] = "true" - begin - @response = "Successfully registered gem: freewill (1.0.0)" - @fetcher.data["#{Gem.host}/api/v1/gems"] = HTTPResponseFactory.create(body: @response, code: 200, msg: "OK") - @cmd.options[:args] = [@path] + @response = "Successfully registered gem: freewill (1.0.0)" + @fetcher.data["#{Gem.host}/api/v1/gems"] = HTTPResponseFactory.create(body: @response, code: 200, msg: "OK") - @cmd.stub(:attest!, proc { raise Gem::Exception, "boom" }) do - use_ui @ui do - @cmd.execute - end - end + @cmd.options[:args] = [@path] - assert_match "Failed to push with attestation, retrying without attestation.", @ui.error - assert_equal Gem::Net::HTTP::Post, @fetcher.last_request.class - assert_equal Gem.read_binary(@path), @fetcher.last_request.body - assert_equal "application/octet-stream", - @fetcher.last_request["Content-Type"] - ensure - ENV.delete("GITHUB_ACTIONS") + @cmd.stub(:attest!, proc { raise Gem::Exception, "boom" }) do + use_ui @ui do + @cmd.execute + end end + + assert_match "Failed to push with attestation, retrying without attestation.", @ui.error + assert_equal Gem::Net::HTTP::Post, @fetcher.last_request.class + assert_equal Gem.read_binary(@path), @fetcher.last_request.body + assert_equal "application/octet-stream", + @fetcher.last_request["Content-Type"] end def test_execute_attestation_skipped_on_non_rubygems_host + omit if RUBY_ENGINE == "jruby" + + ENV["GITHUB_ACTIONS"] = "true" + @spec, @path = util_gem "freebird", "1.0.1" do |spec| spec.metadata["allowed_push_host"] = "https://privategemserver.example" end @@ -193,6 +191,8 @@ def test_execute_attestation_skipped_on_non_rubygems_host end def test_execute_attestation_skipped_on_jruby + ENV["GITHUB_ACTIONS"] = "true" + @response = "Successfully registered gem: freewill (1.0.0)" @fetcher.data["#{Gem.host}/api/v1/gems"] = HTTPResponseFactory.create(body: @response, code: 200, msg: "OK") From 7a8392da6e361634dd7040a2ec06b0146ede12e5 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Mon, 31 Aug 2026 14:48:02 +0900 Subject: [PATCH 14/24] [ruby/rubygems] Only auto-attest when GITHUB_ACTIONS is "true" GitHub Actions documents the variable as "true", so any other value, including "false", should not trigger the auto-attestation path. https://github.com/ruby/rubygems/commit/f6b15f8b45 Co-Authored-By: Claude Fable 5 --- lib/rubygems/commands/push_command.rb | 2 +- .../test_gem_commands_push_command.rb | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/lib/rubygems/commands/push_command.rb b/lib/rubygems/commands/push_command.rb index 494525d661af2b..badad20c4f8da7 100644 --- a/lib/rubygems/commands/push_command.rb +++ b/lib/rubygems/commands/push_command.rb @@ -96,7 +96,7 @@ def send_gem(name) def send_push_request(name, args) # Always honor explicit --attestation option # Auto-attestation is only supported on rubygems.org with GitHub Actions (not JRuby) - if options[:attestations].any? || (RUBY_ENGINE != "jruby" && attestation_supported_host? && ENV["GITHUB_ACTIONS"]) + if options[:attestations].any? || (RUBY_ENGINE != "jruby" && attestation_supported_host? && ENV["GITHUB_ACTIONS"] == "true") send_push_request_with_attestation(name, args) else send_push_request_without_attestation(name, args) diff --git a/test/rubygems/test_gem_commands_push_command.rb b/test/rubygems/test_gem_commands_push_command.rb index c6c1a7401f1083..518492c0f9bb5d 100644 --- a/test/rubygems/test_gem_commands_push_command.rb +++ b/test/rubygems/test_gem_commands_push_command.rb @@ -164,6 +164,26 @@ def test_execute_attestation_fallback @fetcher.last_request["Content-Type"] end + def test_execute_attestation_auto_skipped_unless_github_actions_true + omit if RUBY_ENGINE == "jruby" + + ENV["GITHUB_ACTIONS"] = "false" + + @response = "Successfully registered gem: freewill (1.0.0)" + @fetcher.data["#{Gem.host}/api/v1/gems"] = HTTPResponseFactory.create(body: @response, code: 200, msg: "OK") + + @cmd.options[:args] = [@path] + + attest_called = false + @cmd.stub(:attest!, proc { attest_called = true }) do + @cmd.execute + end + + refute attest_called, "attest! should not be called when GITHUB_ACTIONS is not \"true\"" + assert_equal "application/octet-stream", + @fetcher.last_request["Content-Type"] + end + def test_execute_attestation_skipped_on_non_rubygems_host omit if RUBY_ENGINE == "jruby" From 84724ea898df8f50fcbeb5a05d359a69128fc773 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Mon, 31 Aug 2026 14:48:24 +0900 Subject: [PATCH 15/24] [ruby/rubygems] Prevent silent attestation downgrade on push The whole attested push was wrapped in rescue StandardError. A failure to read a file given to --attestation printed a warning and published the gem unattested with exit 0, and a network error after the server may have accepted the multipart push retried it unattested, letting an on-path attacker strip attestations by cutting the first connection. Only the opportunistic auto-signing step falls back now. attest! also returned just the tempfile path, so GC could unlink the bundle before it was read, which fell into that same rescue. Each bundle is now validated as a JSON object before it is sent. https://github.com/ruby/rubygems/commit/9ce35a5f21 Co-Authored-By: Claude Fable 5 --- lib/rubygems/commands/push_command.rb | 82 ++++++--- .../test_gem_commands_push_command.rb | 173 +++++++++++++++++- 2 files changed, 220 insertions(+), 35 deletions(-) diff --git a/lib/rubygems/commands/push_command.rb b/lib/rubygems/commands/push_command.rb index badad20c4f8da7..ba2cc420c85832 100644 --- a/lib/rubygems/commands/push_command.rb +++ b/lib/rubygems/commands/push_command.rb @@ -48,7 +48,8 @@ def initialize end add_option("--attestation FILE", - "Push with sigstore attestations") do |value, options| + "Push with sigstore attestations", + " (FILE must be a JSON sigstore bundle)") do |value, options| options[:attestations] << value end @@ -117,14 +118,23 @@ def send_push_request_without_attestation(name, args) def send_push_request_with_attestation(name, args) attestations = if options[:attestations].any? options[:attestations].map do |attestation| - Gem.read_binary(attestation) + load_attestation(attestation) end else - bundle_path = attest!(name) + # Only the opportunistic signing step falls back. The request below stays + # outside this rescue because once the server may have seen the attested + # push, a network error must not trigger an unattested retry. begin - [Gem.read_binary(bundle_path)] - ensure - File.unlink(bundle_path) if bundle_path && File.exist?(bundle_path) + [attest!(name)] + rescue StandardError => e + message = "Failed to create an attestation, pushing without one.\n" + message += if Gem.configuration.really_verbose + e.full_message + else + e.message + end + alert_warning message + return send_push_request_without_attestation(name, args) end end bundles = "[" + attestations.join(",") + "]" @@ -136,15 +146,27 @@ def send_push_request_with_attestation(name, args) ], "multipart/form-data") request.add_field "Authorization", api_key end - rescue StandardError => e - message = "Failed to push with attestation, retrying without attestation.\n" - message += if Gem.configuration.really_verbose - e.full_message - else - e.message + end + + def load_attestation(file) + data = begin + Gem.read_binary(file) + rescue SystemCallError, IOError, ArgumentError => e + raise Gem::Exception, "Failed to read attestation #{file}: #{e.message}" + end + validate_attestation_json(data, file) + end + + def validate_attestation_json(data, source) + require "json" + + parsed = begin + JSON.parse(data) + rescue JSON::ParserError => e + raise Gem::Exception, "Attestation #{source} is not valid JSON: #{e.message}" end - alert_warning message - send_push_request_without_attestation(name, args) + raise Gem::Exception, "Attestation #{source} is not a JSON object" unless parsed.is_a?(Hash) + data end def attest!(name) @@ -152,22 +174,24 @@ def attest!(name) require "shellwords" require "tempfile" - tempfile = Tempfile.new([File.basename(name, ".*"), ".sigstore.json"]) - bundle = tempfile.path - tempfile.close(false) - env = defined?(Bundler.unbundled_env) ? Bundler.unbundled_env : ENV.to_h - # Gem.ruby is quoted if it contains whitespace, so split it into argv - # elements to keep the quotes out of the spawned command. - out, st = Open3.capture2e( - env, - *Shellwords.split(Gem.ruby), "-S", "gem", "exec", "--conservative", - "sigstore-cli", "sign", name, "--bundle", bundle, - unsetenv_others: true - ) - raise Gem::Exception, "Failed to sign gem:\n\n#{out}" unless st.success? - - bundle + + Tempfile.create([File.basename(name, ".*"), ".sigstore.json"]) do |tempfile| + tempfile.close + bundle = tempfile.path + + # Gem.ruby is quoted if it contains whitespace, so split it into argv + # elements to keep the quotes out of the spawned command. + out, st = Open3.capture2e( + env, + *Shellwords.split(Gem.ruby), "-S", "gem", "exec", "--conservative", + "sigstore-cli", "sign", name, "--bundle", bundle, + unsetenv_others: true + ) + raise Gem::Exception, "Failed to sign gem:\n\n#{out}" unless st.success? + + validate_attestation_json(Gem.read_binary(bundle), "generated by sigstore-cli") + end end def get_hosts_for(name) diff --git a/test/rubygems/test_gem_commands_push_command.rb b/test/rubygems/test_gem_commands_push_command.rb index 518492c0f9bb5d..904fdfcd8e87bf 100644 --- a/test/rubygems/test_gem_commands_push_command.rb +++ b/test/rubygems/test_gem_commands_push_command.rb @@ -106,7 +106,7 @@ def test_execute_attestation @response = "Successfully registered gem: freewill (1.0.0)" @fetcher.data["#{Gem.host}/api/v1/gems"] = HTTPResponseFactory.create(body: @response, code: 200, msg: "OK") - File.write("#{@path}.sigstore.json", "attestation") + File.write("#{@path}.sigstore.json", '{"attestation":true}') @cmd.options[:args] = [@path] @cmd.options[:attestations] = ["#{@path}.sigstore.json"] @@ -118,6 +118,20 @@ def test_execute_attestation assert_attestation_multipart Gem.read_binary("#{@path}.sigstore.json") end + def test_execute_attestation_multiple + @response = "Successfully registered gem: freewill (1.0.0)" + @fetcher.data["#{Gem.host}/api/v1/gems"] = HTTPResponseFactory.create(body: @response, code: 200, msg: "OK") + + File.write("#{@path}.a.sigstore.json", '{"attestation":"a"}') + File.write("#{@path}.b.sigstore.json", '{"attestation":"b"}') + @cmd.options[:args] = [@path] + @cmd.options[:attestations] = ["#{@path}.a.sigstore.json", "#{@path}.b.sigstore.json"] + + @cmd.execute + + assert_attestation_multipart '{"attestation":"a"},{"attestation":"b"}' + end + def test_execute_attestation_auto omit if RUBY_ENGINE == "jruby" @@ -126,12 +140,10 @@ def test_execute_attestation_auto @response = "Successfully registered gem: freewill (1.0.0)" @fetcher.data["#{Gem.host}/api/v1/gems"] = HTTPResponseFactory.create(body: @response, code: 200, msg: "OK") - attestation_path = "#{@path}.sigstore.json" - attestation_content = "auto-attestation" - File.write(attestation_path, attestation_content) + attestation_content = '{"auto":"attestation"}' @cmd.options[:args] = [@path] - @cmd.stub(:attest!, attestation_path) do + @cmd.stub(:attest!, attestation_content) do @cmd.execute end @@ -157,13 +169,90 @@ def test_execute_attestation_fallback end end - assert_match "Failed to push with attestation, retrying without attestation.", @ui.error + assert_match "Failed to create an attestation, pushing without one.", @ui.error assert_equal Gem::Net::HTTP::Post, @fetcher.last_request.class assert_equal Gem.read_binary(@path), @fetcher.last_request.body assert_equal "application/octet-stream", @fetcher.last_request["Content-Type"] end + def test_execute_attestation_explicit_missing_file + @fetcher.data["#{Gem.host}/api/v1/gems"] = HTTPResponseFactory.create(body: "", code: 200, msg: "OK") + + @cmd.options[:args] = [@path] + @cmd.options[:attestations] = ["#{@path}.sigstore.json"] + + e = assert_raise Gem::Exception do + use_ui @ui do + @cmd.execute + end + end + + assert_match "Failed to read attestation", e.message + refute_match "pushing without one", @ui.error + assert_nil @fetcher.last_request + end + + def test_execute_attestation_explicit_invalid_json + @fetcher.data["#{Gem.host}/api/v1/gems"] = HTTPResponseFactory.create(body: "", code: 200, msg: "OK") + + File.write("#{@path}.sigstore.json", "not json") + @cmd.options[:args] = [@path] + @cmd.options[:attestations] = ["#{@path}.sigstore.json"] + + e = assert_raise Gem::Exception do + use_ui @ui do + @cmd.execute + end + end + + assert_match "is not valid JSON", e.message + refute_match "pushing without one", @ui.error + assert_nil @fetcher.last_request + end + + def test_execute_attestation_explicit_json_scalar + @fetcher.data["#{Gem.host}/api/v1/gems"] = HTTPResponseFactory.create(body: "", code: 200, msg: "OK") + + File.write("#{@path}.sigstore.json", "null") + @cmd.options[:args] = [@path] + @cmd.options[:attestations] = ["#{@path}.sigstore.json"] + + e = assert_raise Gem::Exception do + use_ui @ui do + @cmd.execute + end + end + + assert_match "is not a JSON object", e.message + assert_nil @fetcher.last_request + end + + def test_execute_attestation_network_error_not_retried_without_attestation + omit if RUBY_ENGINE == "jruby" + + ENV["GITHUB_ACTIONS"] = "true" + + requests = 0 + @fetcher.data["#{Gem.host}/api/v1/gems"] = proc do + requests += 1 + raise Gem::RemoteFetcher::FetchError.new("timed out", "#{Gem.host}/api/v1/gems") + end + + @cmd.options[:args] = [@path] + + assert_raise Gem::RemoteFetcher::FetchError do + @cmd.stub(:attest!, '{"auto":"attestation"}') do + use_ui @ui do + @cmd.execute + end + end + end + + assert_equal 1, requests + refute_match "pushing without one", @ui.error + end + def test_execute_attestation_auto_skipped_unless_github_actions_true omit if RUBY_ENGINE == "jruby" @@ -254,6 +343,7 @@ def fake_status.success? captured = nil capture_stub = lambda do |*args, **_kwargs| captured = args + File.write(args[args.index("--bundle") + 1], "{}") ["", fake_status] end Gem.stub(:ruby, '"/path with space/bin/ruby"') do @@ -269,6 +359,77 @@ def fake_status.success? assert_equal "-S", captured[2] end + def test_attest_aborts_when_signing_fails + require "open3" + + fake_status = Object.new + def fake_status.success? + false + end + + bundle_path = nil + capture_stub = lambda do |*args, **_kwargs| + bundle_path = args[args.index("--bundle") + 1] + ["sigstore-cli: no identity token available", fake_status] + end + + e = assert_raise Gem::Exception do + Open3.stub(:capture2e, capture_stub) do + @cmd.send(:attest!, @path) + end + end + + assert_match "Failed to sign gem", e.message + assert_match "no identity token available", e.message + refute_nil bundle_path, "signing command should have been spawned" + refute File.exist?(bundle_path), "bundle tempfile should be removed" + end + + def test_attest_rejects_a_bundle_that_is_not_json + require "open3" + + fake_status = Object.new + def fake_status.success? + true + end + + capture_stub = lambda do |*args, **_kwargs| + File.write(args[args.index("--bundle") + 1], "not json") + ["", fake_status] + end + + e = assert_raise Gem::Exception do + Open3.stub(:capture2e, capture_stub) do + @cmd.send(:attest!, @path) + end + end + + assert_match "is not valid JSON", e.message + end + + def test_attest_returns_bundle_content_and_removes_tempfile + require "open3" + + fake_status = Object.new + def fake_status.success? + true + end + + bundle_path = nil + capture_stub = lambda do |*args, **_kwargs| + bundle_path = args[args.index("--bundle") + 1] + File.write(bundle_path, '{"signed":true}') + ["", fake_status] + end + + content = Open3.stub(:capture2e, capture_stub) do + @cmd.send(:attest!, @path) + end + + assert_equal '{"signed":true}', content + refute File.exist?(bundle_path), "bundle tempfile should be removed" + end + def test_execute_allowed_push_host @spec, @path = util_gem "freebird", "1.0.1" do |spec| spec.metadata["allowed_push_host"] = "https://privategemserver.example" From 29baff291653f6a53c80becf1f3c5d1d53a7d808 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Wed, 2 Sep 2026 10:13:43 +0900 Subject: [PATCH 16/24] [ruby/rubygems] Follow the remote's default branch when the tracked one is gone A git source that pins no branch, tag or ref follows whatever branch the cached bare clone's HEAD points at. Once the remote renames its default branch that fetch fails, and `Source::Git#fetch` swallowed the failure as a network error, so the lockfile stayed pinned to the old revision and `bundle update` reported success with nothing to explain it. Ask the remote for its current default branch on that failure, fetch it, and repoint the cached clone. Anything that goes wrong in there leaves HEAD alone and lets the caller report the original failure, since the cache is shared with every other project on the machine using the same URI, and a half-moved cache would never heal on its own. Fixes https://github.com/ruby/rubygems/pull/5810. https://github.com/ruby/rubygems/commit/5831d9cccb Co-Authored-By: Claude Opus 5 --- lib/bundler/source/git/git_proxy.rb | 67 ++++++++++++++++++- .../bundler/source/git/git_proxy_spec.rb | 30 +++++++++ spec/bundler/update/git_spec.rb | 44 ++++++++++++ 3 files changed, 138 insertions(+), 3 deletions(-) diff --git a/lib/bundler/source/git/git_proxy.rb b/lib/bundler/source/git/git_proxy.rb index 3df37e487bdb91..42f1e9dd71ffb2 100644 --- a/lib/bundler/source/git/git_proxy.rb +++ b/lib/bundler/source/git/git_proxy.rb @@ -193,7 +193,13 @@ def git_remote_fetch(args) return out if status.success? if err.include?("couldn't find remote ref") || err.include?("not our ref") - raise MissingGitRevisionError.new(command_with_no_credentials, path, commit || explicit_ref, credential_filtered_uri) + default_branch = renamed_remote_default_branch if tracking_remote_default_branch? + if default_branch + out = follow_remote_default_branch(args, default_branch) + return out if out + end + + raise MissingGitRevisionError.new(command_with_no_credentials, path, commit || explicit_ref || current_branch, credential_filtered_uri) else if shallow? args -= depth_args @@ -298,6 +304,61 @@ def not_pinned? branch_option || ref.nil? end + def tracking_remote_default_branch? + explicit_ref.nil? && commit.nil? + end + + # Returns nil on any failure, leaving HEAD untouched so the caller reports + # the original fetch failure. Runs inside the retry block, so it must not + # touch the caller's command locals. + def follow_remote_default_branch(args, default_branch) + reference = "refs/heads/#{default_branch}" + command = fetch_command(args, "#{reference}:#{reference}") + check_allowed(command) + + out, err, status = capture(command, path) + unless status.success? + Bundler.ui.debug "Could not fetch #{reference} from #{credential_filtered_uri}: #{err}" + return + end + + previous_branch = current_branch + begin + git "symbolic-ref", "HEAD", reference, dir: path + rescue GitError => e + Bundler.ui.debug "Could not repoint the cached clone at #{reference}: #{e.message}" + return + end + @current_branch = nil + Bundler.ui.warn "#{credential_filtered_uri} no longer has #{previous_branch}, " \ + "now following its default branch #{default_branch}" + out + end + + # The cached clone's HEAD branch is gone from the remote, so the remote's + # own idea of its default branch is the only thing left to follow. + def renamed_remote_default_branch + default_branch = remote_default_branch + return if default_branch.nil? || default_branch == current_branch + + default_branch + end + + def remote_default_branch + command = ["ls-remote", "--symref", "--", configured_uri, "HEAD"] + check_allowed(command) + + out, err, status = capture(command, path) + unless status.success? + Bundler.ui.debug "Could not ask #{credential_filtered_uri} for its default branch: #{err}" + return + end + + # A remote is free to advertise a ref name that is not valid UTF-8, and + # matching that as text raises out of the GitError family. + out.b[%r{^ref:\s+refs/heads/(.+?)\s+HEAD}, 1] + end + def pinned_to_full_sha? full_sha_revision?(ref) end @@ -465,8 +526,8 @@ def extra_clone_args args end - def fetch_command(args) - ["fetch", "--force", "--quiet", "--no-tags", *args, "--", configured_uri, refspec].compact + def fetch_command(args, spec = refspec) + ["fetch", "--force", "--quiet", "--no-tags", *args, "--", configured_uri, spec].compact end def clone_command(args) diff --git a/spec/bundler/bundler/source/git/git_proxy_spec.rb b/spec/bundler/bundler/source/git/git_proxy_spec.rb index 9633c910171043..760819c7e51ff4 100644 --- a/spec/bundler/bundler/source/git/git_proxy_spec.rb +++ b/spec/bundler/bundler/source/git/git_proxy_spec.rb @@ -438,6 +438,36 @@ end end + context "when the remote no longer has the branch HEAD points at" do + let(:cached_branch) { "main" } + let(:missing_ref) { ["", "fatal: couldn't find remote ref refs/heads/#{cached_branch}", fail_result] } + let(:symref_advertisement) { ["ref: refs/heads/renamed\tHEAD\n", "", clone_result] } + + before do + allow(git_proxy).to receive(:git_local).with("--version").and_return("git version 2.14.0") + allow(git_proxy).to receive(:git_local).with("rev-parse", "--abbrev-ref", "HEAD", dir: path).and_return(cached_branch) + allow(git_proxy).to receive(:capture).with([*base_fetch_args, "--", uri, "refs/heads/#{cached_branch}:refs/heads/#{cached_branch}"], path).and_return(missing_ref) + end + + it "follows the branch the remote now points HEAD at" do + expect(git_proxy).to receive(:capture).with(["ls-remote", "--symref", "--", uri, "HEAD"], path).and_return(symref_advertisement) + expect(git_proxy).to receive(:capture).with([*base_fetch_args, "--", uri, "refs/heads/renamed:refs/heads/renamed"], path).and_return(["", "", clone_result]) + expect(git_proxy).to receive(:git).with("symbolic-ref", "HEAD", "refs/heads/renamed", dir: path) + subject.checkout + end + + context "and a revision is locked" do + let(:revision) { Digest::SHA1.hexdigest("ruby") } + + it "does not ask the remote for its default branch" do + expect(git_proxy).to receive(:git).with("cat-file", "-e", revision, dir: path).and_raise(Bundler::GitError) + expect(git_proxy).to receive(:capture).with([*base_fetch_args, "--", uri, "#{revision}:refs/#{revision}-sha"], path).and_return(missing_ref) + expect(git_proxy).not_to receive(:capture).with(["ls-remote", "--symref", "--", uri, "HEAD"], path) + expect { subject.checkout }.to raise_error(Bundler::Source::Git::MissingGitRevisionError) + end + end + end + context "URI is HTTP" do let(:uri) { "http://github.com/ruby/rubygems.git" } diff --git a/spec/bundler/update/git_spec.rb b/spec/bundler/update/git_spec.rb index 17955672f542dc..853c07fcc04dcd 100644 --- a/spec/bundler/update/git_spec.rb +++ b/spec/bundler/update/git_spec.rb @@ -2,6 +2,11 @@ RSpec.describe "bundle update" do describe "git sources" do + def cached_head_for(name) + path = default_cache_path("git/#{name}-#{Digest(:SHA1).hexdigest(lib_path(name).to_s)}") + File.read(path.join("HEAD")).strip + end + it "floats on a branch when :branch is used" do build_git "foo", "1.0" update_git "foo", branch: "omg" @@ -22,6 +27,45 @@ expect(the_bundle).to include_gems "foo 1.1" end + it "updates a source with no :branch when its default branch was renamed" do + build_git "foo", "1.0" + + install_gemfile <<-G + source "https://gem.repo1" + gem "foo", :git => "#{lib_path("foo-1.0")}" + G + + git "branch -m renamed", lib_path("foo-1.0") + update_git "foo" do |s| + s.write "lib/foo.rb", "FOO = '1.1'" + end + + bundle "update", all: true + + expect(err).to include("no longer has main, now following its default branch renamed") + expect(the_bundle).to include_gems "foo 1.1" + end + + it "does not follow a renamed default branch when :branch is used" do + build_git "foo", "1.0" + + install_gemfile <<-G + source "https://gem.repo1" + gem "foo", :git => "#{lib_path("foo-1.0")}", :branch => "main" + G + + git "branch -m renamed", lib_path("foo-1.0") + update_git "foo" do |s| + s.write "lib/foo.rb", "FOO = '1.1'" + end + + bundle "update", all: true + + expect(err).to include("Revision main does not exist") + expect(the_bundle).to include_gems "foo 1.0" + expect(cached_head_for("foo-1.0")).to eq("ref: refs/heads/main") + end + it "updates correctly when you have like craziness" do build_lib "activesupport", "3.0", path: lib_path("rails/activesupport") build_git "rails", "3.0", path: lib_path("rails") do |s| From c3436dfd82ac39a14df7bab4c19f18c6677c3d5a Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Fri, 28 Aug 2026 17:39:11 +0900 Subject: [PATCH 17/24] [ruby/rubygems] Restrict the shared global gem cache to remote sources A gem installed from a local path was copied into the global cache under its canonical file name, where later remote installs of the same name and version would reuse it without re-verification, so one local install could poison every other project on the machine. Two smaller problems in the same expression go with it. The global cache branch was evaluated before the one gem fetch relies on, so gem fetch wrote into the cache instead of the working directory, and an unwritable cache directory aborted the install rather than falling back the way it does with the cache disabled. The working directory is compared by identity because the paths can differ while naming the same place, and because Dir.pwd raises once that directory is gone. https://github.com/ruby/rubygems/commit/645fe42ef9 Co-Authored-By: Claude Fable 5 --- lib/rubygems/config_file.rb | 7 +- lib/rubygems/remote_fetcher.rb | 50 +++++--- test/rubygems/test_gem_remote_fetcher.rb | 150 +++++++++++++++++++++++ 3 files changed, 192 insertions(+), 15 deletions(-) diff --git a/lib/rubygems/config_file.rb b/lib/rubygems/config_file.rb index e9cd2c6c355429..853de9e8ecd143 100644 --- a/lib/rubygems/config_file.rb +++ b/lib/rubygems/config_file.rb @@ -191,7 +191,12 @@ class Gem::ConfigFile ## # Use a global cache for .gem files shared across all Ruby installations. - # When enabled, gems are cached to ~/.cache/gem/gems (or XDG_CACHE_HOME/gem/gems). + # When enabled, gems fetched from a remote source are cached to + # ~/.cache/gem/gems (or XDG_CACHE_HOME/gem/gems). Gems installed from a + # local path are not, since a cached copy is later reused without being + # verified again. gem fetch still writes to the working directory, + # and an unwritable cache directory falls back to the cache of the + # installation. attr_accessor :global_gem_cache diff --git a/lib/rubygems/remote_fetcher.rb b/lib/rubygems/remote_fetcher.rb index 5b83dc6f6f2030..3aa55f7963fb04 100644 --- a/lib/rubygems/remote_fetcher.rb +++ b/lib/rubygems/remote_fetcher.rb @@ -47,6 +47,10 @@ class UnknownHostError < FetchError end deprecate_constant(:UnknownHostError) + # Schemes fetched over the network, as opposed to copied from a local path. + REMOTE_SCHEMES = %w[http https s3].freeze + private_constant :REMOTE_SCHEMES + @fetcher = nil ## @@ -113,12 +117,21 @@ def download_to_cache(dependency) def download(spec, source_uri, install_dir = Gem.dir) gem_file_name = File.basename spec.cache_file + source_uri = Gem::Uri.new(source_uri) + + scheme = source_uri.scheme + + # Gem::URI.parse gets confused by MS Windows paths with forward slashes. + scheme = nil if /^[a-z]$/i.match?(scheme) + + remote_source = REMOTE_SCHEMES.include?(scheme) + install_cache_dir = File.join install_dir, "cache" cache_dir = - if Gem.configuration.global_gem_cache - Gem.global_gem_cache_path - elsif Dir.pwd == install_dir # see fetch_command + if File.identical?(".", install_dir) # gem fetch asks for it this way install_dir + elsif Gem.configuration.global_gem_cache && remote_source && ensure_writable_cache_dir(Gem.global_gem_cache_path) + Gem.global_gem_cache_path elsif File.writable?(install_cache_dir) || (File.writable?(install_dir) && !File.exist?(install_cache_dir)) install_cache_dir else @@ -134,20 +147,15 @@ def download(spec, source_uri, install_dir = Gem.dir) nil end unless File.exist? cache_dir - source_uri = Gem::Uri.new(source_uri) - - scheme = source_uri.scheme - - # Gem::URI.parse gets confused by MS Windows paths with forward slashes. - scheme = nil if /^[a-z]$/i.match?(scheme) - # REFACTOR: split this up and dispatch on scheme (eg download_http) # REFACTOR: be sure to clean up fake fetcher when you do this... cleaner case scheme - when "http", "https", "s3" then - unless File.exist? local_gem_path + when *REMOTE_SCHEMES then + if File.exist? local_gem_path + verbose "Using local gem #{local_gem_path}" + else begin - verbose "Downloading gem #{gem_file_name}" + verbose "Downloading gem #{gem_file_name} to #{cache_dir}" remote_gem_path = source_uri + "gems/#{gem_file_name}" @@ -157,7 +165,7 @@ def download(spec, source_uri, install_dir = Gem.dir) alternate_name = "#{spec.original_name}.gem" - verbose "Failed, downloading gem #{alternate_name}" + verbose "Failed, downloading gem #{alternate_name} to #{cache_dir}" remote_gem_path = source_uri + "gems/#{alternate_name}" @@ -338,6 +346,20 @@ def close_all private + # Creates +cache_dir+ so its writability can be probed, since File.writable? + # is false for a path that does not exist yet. + + def ensure_writable_cache_dir(cache_dir) + require "fileutils" + begin + FileUtils.mkdir_p cache_dir + rescue SystemCallError + return false + end + + File.writable?(cache_dir) + end + def proxy_for(proxy, uri) Gem::Request.proxy_uri(proxy || Gem::Request.get_proxy_from_env(uri.scheme)) end diff --git a/test/rubygems/test_gem_remote_fetcher.rb b/test/rubygems/test_gem_remote_fetcher.rb index c35da2fc5ae273..d754a35ab0ff2d 100644 --- a/test/rubygems/test_gem_remote_fetcher.rb +++ b/test/rubygems/test_gem_remote_fetcher.rb @@ -643,6 +643,156 @@ def fetcher.fetch_path(uri, *rest) end end + def test_download_with_global_gem_cache_fetches_to_current_directory + test_cache_dir = File.join(@tempdir, "global_gem_cache_test") + + Gem.stub :global_gem_cache_path, test_cache_dir do + Gem.configuration.global_gem_cache = true + + fetcher = Gem::RemoteFetcher.fetcher + def fetcher.fetch_path(uri, *rest) + File.binread File.join(@test_gem_dir, "a-1.gem") + end + fetcher.instance_variable_set(:@test_gem_dir, File.dirname(@a1_gem)) + + fetch_dir = File.join @tempdir, "fetch_dir" + FileUtils.mkdir_p fetch_dir + + # gem fetch downloads into the current directory, see fetch_command + fetched_gem = Dir.chdir fetch_dir do + fetcher.download(@a1, "http://gems.example.com", fetch_dir) + end + + assert_equal File.join(fetch_dir, @a1.file_name), fetched_gem + assert File.exist?(fetched_gem) + refute File.exist?(test_cache_dir), + "gem fetch output should not be diverted to the global cache" + end + ensure + Gem.configuration.global_gem_cache = false + end + + def test_download_to_current_directory_reached_through_a_symlink + omit "symlinks are not usable on Windows" if Gem.win_platform? + + fetch_dir = File.join @tempdir, "fetch_dir" + FileUtils.mkdir_p fetch_dir + linked_dir = File.join @tempdir, "linked_dir" + File.symlink fetch_dir, linked_dir + + fetcher = Gem::RemoteFetcher.fetcher + def fetcher.fetch_path(uri, *rest) + File.binread File.join(@test_gem_dir, "a-1.gem") + end + fetcher.instance_variable_set(:@test_gem_dir, File.dirname(@a1_gem)) + + # gem fetch passes the working directory as install_dir, and the two can + # name the same directory through different paths + fetched_gem = Dir.chdir fetch_dir do + fetcher.download(@a1, "http://gems.example.com", linked_dir) + end + + assert_equal File.join(linked_dir, @a1.file_name), fetched_gem + assert File.exist?(fetched_gem) + end + + def test_download_local_with_global_gem_cache + omit "doesn't work if tempdir has +" if @tempdir.include?("+") + test_cache_dir = File.join(@tempdir, "global_gem_cache_test") + + Gem.stub :global_gem_cache_path, test_cache_dir do + Gem.configuration.global_gem_cache = true + + FileUtils.mv @a1_gem, @tempdir + local_path = File.join @tempdir, @a1.file_name + inst = nil + + Dir.chdir @tempdir do + inst = Gem::RemoteFetcher.fetcher + end + + assert_equal @a1.cache_file, inst.download(@a1, local_path) + refute File.exist?(test_cache_dir), + "local gems should not be copied to the global cache" + end + ensure + Gem.configuration.global_gem_cache = false + end + + def test_download_file_scheme_with_global_gem_cache + test_cache_dir = File.join(@tempdir, "global_gem_cache_test") + + Gem.stub :global_gem_cache_path, test_cache_dir do + Gem.configuration.global_gem_cache = true + + repo_dir = File.join @tempdir, "repo" + FileUtils.mkdir_p File.join(repo_dir, "gems") + FileUtils.cp @a1_gem, File.join(repo_dir, "gems", @a1.file_name) + + uri_path = repo_dir.start_with?("/") ? repo_dir : "/#{repo_dir}" + inst = Gem::RemoteFetcher.fetcher + + assert_equal @a1.cache_file, inst.download(@a1, "file://#{uri_path}") + assert File.exist?(@a1.cache_file) + refute File.exist?(test_cache_dir), + "local gems should not be copied to the global cache" + end + ensure + Gem.configuration.global_gem_cache = false + end + + unless Gem.win_platform? || Process.uid.zero? # File.chmod doesn't work + def test_download_with_global_gem_cache_not_writable + test_cache_dir = File.join(@tempdir, "global_gem_cache_test") + FileUtils.mkdir_p test_cache_dir + FileUtils.chmod 0o555, test_cache_dir + + Gem.stub :global_gem_cache_path, test_cache_dir do + Gem.configuration.global_gem_cache = true + + fetcher = Gem::RemoteFetcher.fetcher + def fetcher.fetch_path(uri, *rest) + File.binread File.join(@test_gem_dir, "a-1.gem") + end + fetcher.instance_variable_set(:@test_gem_dir, File.dirname(@a1_gem)) + + a1_cache_gem = @a1.cache_file + assert_equal a1_cache_gem, fetcher.download(@a1, "http://gems.example.com") + assert File.exist?(a1_cache_gem) + assert_empty Dir.children(test_cache_dir) + end + ensure + FileUtils.chmod 0o755, test_cache_dir if File.exist?(test_cache_dir) + Gem.configuration.global_gem_cache = false + end + + def test_download_with_global_gem_cache_not_creatable + parent_dir = File.join(@tempdir, "global_gem_cache_parent") + FileUtils.mkdir_p parent_dir + FileUtils.chmod 0o555, parent_dir + test_cache_dir = File.join(parent_dir, "gems") + + Gem.stub :global_gem_cache_path, test_cache_dir do + Gem.configuration.global_gem_cache = true + + fetcher = Gem::RemoteFetcher.fetcher + def fetcher.fetch_path(uri, *rest) + File.binread File.join(@test_gem_dir, "a-1.gem") + end + fetcher.instance_variable_set(:@test_gem_dir, File.dirname(@a1_gem)) + + a1_cache_gem = @a1.cache_file + assert_equal a1_cache_gem, fetcher.download(@a1, "http://gems.example.com") + assert File.exist?(a1_cache_gem) + refute File.exist?(test_cache_dir) + end + ensure + FileUtils.chmod 0o755, parent_dir if File.exist?(parent_dir) + Gem.configuration.global_gem_cache = false + end + + end + def test_fetch_http_with_custom_error_header fetcher = Gem::RemoteFetcher.new nil @fetcher = fetcher From 8088aeefbd803197e418374839278b8a60888285 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Fri, 28 Aug 2026 17:39:33 +0900 Subject: [PATCH 18/24] [ruby/rubygems] Copy local gems into the cache dir atomically The file and path schemes wrote into the shared cache directory with a plain FileUtils.cp, so a concurrent install could read a half-written .gem file. Route them through Gem::AtomicFileWriter like the http scheme, which also makes replacing a read-only cache copy work. cp passed the source mode to File.open, so it reached only a file being created and the umask still applied to it. Reproduce that rather than chmodding unconditionally, which would skip the umask, carry setuid across, and rewrite the mode of a file cp would have left alone. https://github.com/ruby/rubygems/commit/9507af2bf3 Co-Authored-By: Claude Fable 5 --- lib/rubygems/remote_fetcher.rb | 24 +++++++- test/rubygems/test_gem_remote_fetcher.rb | 78 ++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 2 deletions(-) diff --git a/lib/rubygems/remote_fetcher.rb b/lib/rubygems/remote_fetcher.rb index 3aa55f7963fb04..d3ab256029e851 100644 --- a/lib/rubygems/remote_fetcher.rb +++ b/lib/rubygems/remote_fetcher.rb @@ -179,7 +179,7 @@ def download(spec, source_uri, install_dir = Gem.dir) remote_gem_path = Gem::Util.correct_for_windows_path(File.join(path, "gems", gem_file_name)) - FileUtils.cp(remote_gem_path, local_gem_path) + atomic_copy(remote_gem_path, local_gem_path) rescue Errno::EACCES local_gem_path = source_uri.to_s end @@ -196,7 +196,7 @@ def download(spec, source_uri, install_dir = Gem.dir) source_path = Gem::UriFormatter.new(source_path).unescape begin - FileUtils.cp source_path, local_gem_path unless + atomic_copy(source_path, local_gem_path) unless File.identical?(source_path, local_gem_path) rescue Errno::EACCES local_gem_path = source_uri.to_s @@ -360,6 +360,26 @@ def ensure_writable_cache_dir(cache_dir) File.writable?(cache_dir) end + def atomic_copy(source_path, destination_path) + File.open(source_path, "rb") do |source| + # FileUtils.cp passed the source mode to File.open, so it only reached a + # file being created and the umask still applied to it. The writer + # already carries over the mode of a file it replaces. + mode = source.stat.mode & 0o777 & ~File.umask + replacing = File.exist?(destination_path) + + Gem::AtomicFileWriter.open(destination_path) do |io| + IO.copy_stream(source, io) + + begin + io.chmod(mode) unless replacing + rescue Errno::EPERM, Errno::EACCES + # the filesystem does not carry permissions + end + end + end + end + def proxy_for(proxy, uri) Gem::Request.proxy_uri(proxy || Gem::Request.get_proxy_from_env(uri.scheme)) end diff --git a/test/rubygems/test_gem_remote_fetcher.rb b/test/rubygems/test_gem_remote_fetcher.rb index d754a35ab0ff2d..2c599e00986b90 100644 --- a/test/rubygems/test_gem_remote_fetcher.rb +++ b/test/rubygems/test_gem_remote_fetcher.rb @@ -672,6 +672,63 @@ def fetcher.fetch_path(uri, *rest) Gem.configuration.global_gem_cache = false end + def test_download_local_takes_the_source_permissions_through_the_umask + omit "File.chmod doesn't work on Windows" if Gem.win_platform? + omit "doesn't work if tempdir has +" if @tempdir.include?("+") + + FileUtils.mv @a1_gem, @tempdir + local_path = File.join @tempdir, @a1.file_name + FileUtils.chmod 0o666, local_path + inst = nil + + Dir.chdir @tempdir do + inst = Gem::RemoteFetcher.fetcher + end + + assert_equal @a1.cache_file, inst.download(@a1, local_path) + assert_equal 0o666 & ~File.umask, File.stat(@a1.cache_file).mode & 0o777 + end + + def test_download_local_keeps_a_restrictive_source_permission + omit "File.chmod doesn't work on Windows" if Gem.win_platform? + omit "doesn't work if tempdir has +" if @tempdir.include?("+") + + FileUtils.mv @a1_gem, @tempdir + local_path = File.join @tempdir, @a1.file_name + FileUtils.chmod 0o600, local_path + inst = nil + + Dir.chdir @tempdir do + inst = Gem::RemoteFetcher.fetcher + end + + # a mode the writer would not produce on its own, so dropping the chmod + # would show up here + assert_equal @a1.cache_file, inst.download(@a1, local_path) + assert_equal 0o600, File.stat(@a1.cache_file).mode & 0o777 + end + + def test_download_local_keeps_the_replaced_cache_file_permissions + omit "File.chmod doesn't work on Windows" if Gem.win_platform? + omit "doesn't work if tempdir has +" if @tempdir.include?("+") + + FileUtils.mv @a1_gem, @tempdir + local_path = File.join @tempdir, @a1.file_name + FileUtils.chmod 0o666, local_path + inst = nil + + FileUtils.mkdir_p File.dirname(@a1.cache_file) + FileUtils.touch @a1.cache_file + FileUtils.chmod 0o640, @a1.cache_file + + Dir.chdir @tempdir do + inst = Gem::RemoteFetcher.fetcher + end + + assert_equal @a1.cache_file, inst.download(@a1, local_path) + assert_equal 0o640, File.stat(@a1.cache_file).mode & 0o777 + end + def test_download_to_current_directory_reached_through_a_symlink omit "symlinks are not usable on Windows" if Gem.win_platform? @@ -791,6 +848,27 @@ def fetcher.fetch_path(uri, *rest) Gem.configuration.global_gem_cache = false end + def test_download_local_replaces_read_only_cache_file + omit "doesn't work if tempdir has +" if @tempdir.include?("+") + FileUtils.mv @a1_gem, @tempdir + local_path = File.join @tempdir, @a1.file_name + inst = nil + + FileUtils.mkdir_p File.dirname(@a1.cache_file) + FileUtils.touch @a1.cache_file + FileUtils.chmod 0o444, @a1.cache_file + + Dir.chdir @tempdir do + inst = Gem::RemoteFetcher.fetcher + end + + # the atomic replacement of the cache copy must not depend on the + # permissions of the previous file + assert_equal @a1.cache_file, inst.download(@a1, local_path) + assert_equal File.binread(local_path), File.binread(@a1.cache_file) + ensure + FileUtils.chmod 0o644, @a1.cache_file if File.exist?(@a1.cache_file) + end end def test_fetch_http_with_custom_error_header From 0fd832630e7d3e249fc865b0b15ed3ef851984d6 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Fri, 28 Aug 2026 17:39:46 +0900 Subject: [PATCH 19/24] [ruby/rubygems] Gate the global cache specs on the version that ships the API The expected cache base was computed from Gem.global_gem_cache_path, the same expression the code under test uses, so the assertion could never catch an unintended path change. That method was added after 4.0 was cut, so no released 4.0.x has it and the old gate would make the specs expect the RubyGems layout from a Bundler that falls back to its own. https://github.com/ruby/rubygems/commit/ca3c7c08b7 Co-Authored-By: Claude Fable 5 --- spec/bundler/install/global_cache_spec.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/spec/bundler/install/global_cache_spec.rb b/spec/bundler/install/global_cache_spec.rb index 5e5494dbe9d349..ce0564ee785136 100644 --- a/spec/bundler/install/global_cache_spec.rb +++ b/spec/bundler/install/global_cache_spec.rb @@ -9,11 +9,11 @@ let(:source2) { "http://gemserver.example.org" } def cache_base - # Use the unified global gem cache path if the RubyGems under test - # provides it, otherwise fall back to the Bundler-specific cache - # location that Bundler uses on RubyGems older than 4.0 - if exercised_rubygems_version >= Gem::Version.new("4.0.0.a") - Pathname.new(Gem.global_gem_cache_path) + # Gem.global_gem_cache_path first ships in RubyGems 4.1. Older RubyGems + # fall back to the Bundler-specific cache location. The suite clears + # XDG_CACHE_HOME, so the path resolves to the ~/.cache default. + if exercised_rubygems_version >= Gem::Version.new("4.1.0.a") + home(".cache", "gem", "gems") else home(".bundle", "cache", "gems") end From e04de71f7f79a17386244b0f4399b305b84a30aa Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Wed, 2 Sep 2026 10:17:46 +0900 Subject: [PATCH 20/24] [ruby/rubygems] Remove Gem::BasicSpecification#datadir It was deprecated in 4.0 with a 4.1 removal horizon, and its tests were already dropped with the deprecation. Nothing else in this file uses `rubygems_deprecate`, so the `Gem::Deprecate` extension goes too. https://github.com/ruby/rubygems/commit/a0b5cb5e03 Co-Authored-By: Claude Fable 5.1 --- lib/rubygems/basic_specification.rb | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/lib/rubygems/basic_specification.rb b/lib/rubygems/basic_specification.rb index 61d35307f4ea13..f1a04fc180e85d 100644 --- a/lib/rubygems/basic_specification.rb +++ b/lib/rubygems/basic_specification.rb @@ -182,17 +182,6 @@ def full_require_paths end end - ## - # The path to the data directory for this gem. - - def datadir - # TODO: drop the extra ", gem_name" which is uselessly redundant - File.expand_path(File.join(gems_dir, full_name, "data", name)) - end - - extend Gem::Deprecate - rubygems_deprecate :datadir, :none, "4.1" - ## # Full path of the target library file. # If the file is not in this gem, return nil. From 1e391d9e40384895c7dc769fc35e8816adc57f7d Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 2 Sep 2026 02:13:24 +0000 Subject: [PATCH 21/24] Bump the github-actions group across 1 directory with 2 updates Bumps the github-actions group with 2 updates in the / directory: [zizmorcore/zizmor-action](https://github.com/zizmorcore/zizmor-action) and [taiki-e/install-action](https://github.com/taiki-e/install-action). Updates `zizmorcore/zizmor-action` from 0.6.2 to 0.6.3 - [Release notes](https://github.com/zizmorcore/zizmor-action/releases) - [Commits](https://github.com/zizmorcore/zizmor-action/compare/3dc1ecc9bcb9e94e9b2c709687979e1298497054...70fb788f84895a7701f5643d103d587e460b5c99) Updates `taiki-e/install-action` from 2.87.1 to 2.87.2 - [Release notes](https://github.com/taiki-e/install-action/releases) - [Changelog](https://github.com/taiki-e/install-action/blob/main/CHANGELOG.md) - [Commits](https://github.com/taiki-e/install-action/compare/742a3317eac7bd62f91cd888b4eead5e784ba833...1ed6d7be6168f6c9046541087ff549b6bc581fdf) --- updated-dependencies: - dependency-name: zizmorcore/zizmor-action dependency-version: 0.6.3 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: github-actions - dependency-name: taiki-e/install-action dependency-version: 2.87.2 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: github-actions ... Signed-off-by: dependabot[bot] --- .github/workflows/check_sast.yml | 2 +- .github/workflows/zjit-macos.yml | 2 +- .github/workflows/zjit-ubuntu.yml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/check_sast.yml b/.github/workflows/check_sast.yml index 669261535b13a0..90ed8a5bc9767f 100644 --- a/.github/workflows/check_sast.yml +++ b/.github/workflows/check_sast.yml @@ -45,7 +45,7 @@ jobs: persist-credentials: false - name: Run zizmor - uses: zizmorcore/zizmor-action@3dc1ecc9bcb9e94e9b2c709687979e1298497054 # v0.6.2 + uses: zizmorcore/zizmor-action@70fb788f84895a7701f5643d103d587e460b5c99 # v0.6.3 continue-on-error: true analyze: diff --git a/.github/workflows/zjit-macos.yml b/.github/workflows/zjit-macos.yml index e0cc2eff496ab7..0481646faa75fc 100644 --- a/.github/workflows/zjit-macos.yml +++ b/.github/workflows/zjit-macos.yml @@ -98,7 +98,7 @@ jobs: rustup install ${{ matrix.rust_version }} --profile minimal rustup default ${{ matrix.rust_version }} - - uses: taiki-e/install-action@742a3317eac7bd62f91cd888b4eead5e784ba833 # v2.87.1 + - uses: taiki-e/install-action@1ed6d7be6168f6c9046541087ff549b6bc581fdf # v2.87.2 with: tool: nextest@0.9 if: ${{ matrix.test_task == 'zjit-check' }} diff --git a/.github/workflows/zjit-ubuntu.yml b/.github/workflows/zjit-ubuntu.yml index 4b57137ba7f42c..2443e3aaa3446a 100644 --- a/.github/workflows/zjit-ubuntu.yml +++ b/.github/workflows/zjit-ubuntu.yml @@ -152,7 +152,7 @@ jobs: ruby-version: '3.1' bundler: none - - uses: taiki-e/install-action@742a3317eac7bd62f91cd888b4eead5e784ba833 # v2.87.1 + - uses: taiki-e/install-action@1ed6d7be6168f6c9046541087ff549b6bc581fdf # v2.87.2 with: tool: nextest@0.9 if: ${{ matrix.test_task == 'zjit-check' }} From d311b3411692ffb1125c0a20a969f078409718fb Mon Sep 17 00:00:00 2001 From: ydah Date: Sat, 29 Aug 2026 09:56:06 +0900 Subject: [PATCH 22/24] parse.y: clear errno before parsing float literals --- parse.y | 1 + 1 file changed, 1 insertion(+) diff --git a/parse.y b/parse.y index 7a566681964070..78d3f4ab4c3baf 100644 --- a/parse.y +++ b/parse.y @@ -9885,6 +9885,7 @@ parse_numeric(struct parser_params *p, int c) type = tRATIONAL; } else { + errno = 0; strtod(tok(p), 0); if (errno == ERANGE) { rb_warning1("Float %s out of range", WARN_S(tok(p))); From bd2a73448641e3d2bd98116ab3420006a18a5233 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Thu, 4 Jun 2026 17:32:19 +0900 Subject: [PATCH 23/24] [ruby/rubygems] Avoid duplicate relative path prefix in bundle exec When bundle exec resolves an explicit relative path like ./script and falls back to Kernel.exec, Bundler currently prepends another ./ because the resolved path is not absolute. The same issue affects Windows paths such as .\script. Only prepend ./ for non-absolute paths that do not already begin with an explicit relative path marker. Recognize the platform's alternative path separator without changing the meaning of backslashes on POSIX. Add integration coverage for the original regression and focused coverage for primary and alternative path separators. Closes https://github.com/ruby/rubygems/issues/8930 Assisted-By: devx/caa7d694-19ff-42bb-9306-686cb9d69649 https://github.com/ruby/rubygems/commit/d172a35a54 --- lib/bundler/cli/exec.rb | 16 +++++++- spec/bundler/bundler/cli/exec_spec.rb | 54 +++++++++++++++++++++++++++ spec/bundler/commands/exec_spec.rb | 26 +++++++++++++ spec/bundler/support/shards.rb | 1 + 4 files changed, 96 insertions(+), 1 deletion(-) create mode 100644 spec/bundler/bundler/cli/exec_spec.rb diff --git a/lib/bundler/cli/exec.rb b/lib/bundler/cli/exec.rb index 2fdc4162868abe..20ed1d8bc62c38 100644 --- a/lib/bundler/cli/exec.rb +++ b/lib/bundler/cli/exec.rb @@ -23,7 +23,7 @@ def run bin_path.delete_suffix!(".bat") if Gem.win_platform? kernel_load(bin_path, *args) else - bin_path = "./" + bin_path unless File.absolute_path?(bin_path) + bin_path = explicit_path(bin_path) kernel_exec(bin_path, *args) end else @@ -71,6 +71,20 @@ def process_title(file, args) "#{file} #{args.join(" ")}".strip end + def explicit_path(path) + if File.absolute_path?(path) || explicit_relative_path?(path) + path + else + ".#{File::SEPARATOR}#{path}" + end + end + + def explicit_relative_path?(path) + [File::SEPARATOR, File::ALT_SEPARATOR].compact.any? do |separator| + path.start_with?(".#{separator}", "..#{separator}") + end + end + def directly_loadable?(file) if Gem.win_platform? script_wrapper?(file) diff --git a/spec/bundler/bundler/cli/exec_spec.rb b/spec/bundler/bundler/cli/exec_spec.rb new file mode 100644 index 00000000000000..c1b0db6176c59e --- /dev/null +++ b/spec/bundler/bundler/cli/exec_spec.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require "bundler/cli" +require "bundler/cli/exec" + +RSpec.describe Bundler::CLI::Exec do + subject(:command) { described_class.new(options, ["script"]) } + + let(:options) { double("options", keep_file_descriptors?: false) } + + before do + allow(Bundler.current_ruby).to receive(:jruby?).and_return(false) + allow(Bundler::SharedHelpers).to receive(:set_bundle_environment) + allow(Bundler).to receive(:settings).and_return({ disable_exec_load: true }) + end + + def expect_exec_path(path, expected_path) + allow(Bundler).to receive(:which).with("script").and_return(path) + expect(command).to receive(:kernel_exec).with(expected_path, kind_of(Hash)) + command.run + end + + it "preserves explicit relative paths using the primary path separator" do + path = ".#{File::SEPARATOR}script" + expect_exec_path(path, path) + end + + it "preserves parent-relative paths using the primary path separator" do + path = "..#{File::SEPARATOR}script" + expect_exec_path(path, path) + end + + it "preserves explicit relative paths using the alternative path separator" do + stub_const("File::ALT_SEPARATOR", "\\") + + expect_exec_path(".\\script", ".\\script") + end + + it "preserves parent-relative paths using the alternative path separator" do + stub_const("File::ALT_SEPARATOR", "\\") + + expect_exec_path("..\\script", "..\\script") + end + + it "prepends the primary path separator to other relative paths" do + expect_exec_path(".script", ".#{File::SEPARATOR}.script") + end + + it "treats a backslash as part of a filename when it is not a path separator" do + stub_const("File::ALT_SEPARATOR", nil) + + expect_exec_path(".\\script", ".#{File::SEPARATOR}.\\script") + end +end diff --git a/spec/bundler/commands/exec_spec.rb b/spec/bundler/commands/exec_spec.rb index e7a294b42932fa..5a381e5073dde5 100644 --- a/spec/bundler/commands/exec_spec.rb +++ b/spec/bundler/commands/exec_spec.rb @@ -343,6 +343,32 @@ expect(out).to include(rubylib) end + it "does not duplicate a relative path prefix when exec'ing to a relative path" do + skip "https://github.com/ruby/rubygems/issues/3351" if Gem.win_platform? + + create_file("script", "#!/usr/bin/env ruby\nputs $0") + + install_gemfile <<-G + source "https://gem.repo1" + G + + bundle "exec ./script", env: { "BUNDLE_DISABLE_EXEC_LOAD" => "true" } + expect(out).to eq("./script") + end + + it "prepends a relative path prefix when exec'ing to a hidden file in the current directory" do + skip "https://github.com/ruby/rubygems/issues/3351" if Gem.win_platform? + + create_file(".script", "#!/usr/bin/env ruby\nputs $0") + + install_gemfile <<-G + source "https://gem.repo1" + G + + bundle "exec .script", env: { "BUNDLE_DISABLE_EXEC_LOAD" => "true" } + expect(out).to eq("./.script") + end + it "errors nicely when the argument doesn't exist" do install_gemfile <<-G source "https://gem.repo1" diff --git a/spec/bundler/support/shards.rb b/spec/bundler/support/shards.rb index ccadff75faa97e..93e1ccb9946275 100644 --- a/spec/bundler/support/shards.rb +++ b/spec/bundler/support/shards.rb @@ -144,6 +144,7 @@ module Shards "spec/bundler/ci_detector_spec.rb", ], shard_d: [ + "spec/bundler/cli/exec_spec.rb", "spec/bundler/rubygems_ext_spec.rb", "spec/bundler/resolver/cooldown_spec.rb", "spec/install/cooldown_spec.rb", From 90e8610e0d7e43a860b7590e670ca5d6985e926e Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Wed, 2 Sep 2026 10:42:57 +0900 Subject: [PATCH 24/24] JIT: Reduce jit_entry_calls/jit_exception_calls to 32 bits We don't need 64 bits for jit_entry_calls/jit_exception_calls so dropping them down to 32 bits will allow us to save 8 bytes per iseq. --- vm.c | 6 +++--- vm_core.h | 10 +++++----- yjit.h | 4 ++-- yjit/src/options.rs | 8 ++++---- yjit/src/yjit.rs | 2 +- zjit.h | 4 ++-- zjit/src/options.rs | 2 +- 7 files changed, 18 insertions(+), 18 deletions(-) diff --git a/vm.c b/vm.c index d1084a851bfb4b..5454bff570e974 100644 --- a/vm.c +++ b/vm.c @@ -470,14 +470,14 @@ static VALUE vm_invoke_proc(rb_execution_context_t *ec, rb_proc_t *proc, VALUE s #if USE_YJIT // Counter to serve as a proxy for execution time, total number of calls -static uint64_t yjit_total_entry_hits = 0; +static unsigned int yjit_total_entry_hits = 0; // Number of calls used to estimate how hot an ISEQ is #define YJIT_CALL_COUNT_INTERV 20u /// Test whether we are ready to compile an ISEQ or not static inline bool -rb_yjit_threshold_hit(const rb_iseq_t *iseq, uint64_t entry_calls) +rb_yjit_threshold_hit(const rb_iseq_t *iseq, unsigned int entry_calls) { yjit_total_entry_hits += 1; @@ -494,7 +494,7 @@ rb_yjit_threshold_hit(const rb_iseq_t *iseq, uint64_t entry_calls) return true; } - uint64_t num_calls = yjit_total_entry_hits - ISEQ_BODY(iseq)->yjit_calls_at_interv; + unsigned int num_calls = yjit_total_entry_hits - ISEQ_BODY(iseq)->yjit_calls_at_interv; // Reject ISEQs that don't get called often enough if (num_calls > rb_yjit_cold_threshold) { diff --git a/vm_core.h b/vm_core.h index a1e6bfac431ff6..8843b7ac124ead 100644 --- a/vm_core.h +++ b/vm_core.h @@ -566,20 +566,20 @@ struct rb_iseq_constant_body { const rb_iseq_t *mandatory_only_iseq; #if USE_YJIT || USE_ZJIT + // Number of calls on jit_exec() + unsigned int jit_entry_calls; + // Number of calls on jit_exec_exception() + unsigned int jit_exception_calls; // Function pointer for JIT code on jit_exec() rb_jit_func_t jit_entry; - // Number of calls on jit_exec() - long unsigned jit_entry_calls; // Function pointer for JIT code on jit_exec_exception() rb_jit_func_t jit_exception; - // Number of calls on jit_exec_exception() - long unsigned jit_exception_calls; void *jit_payload; #endif #if USE_YJIT // Used to estimate how frequently this ISEQ gets called - uint64_t yjit_calls_at_interv; + unsigned int yjit_calls_at_interv; #endif // Hash of the source this iseq was compiled from, or 0 if it is diff --git a/yjit.h b/yjit.h index 93acf4e60d1dd9..772ccc5b1b5ede 100644 --- a/yjit.h +++ b/yjit.h @@ -24,8 +24,8 @@ #endif // Expose these as declarations since we are building YJIT. -extern uint64_t rb_yjit_call_threshold; -extern uint64_t rb_yjit_cold_threshold; +extern unsigned int rb_yjit_call_threshold; +extern unsigned int rb_yjit_cold_threshold; extern uint64_t rb_yjit_live_iseq_count; extern uint64_t rb_yjit_iseq_alloc_count; extern bool rb_yjit_enabled_p; diff --git a/yjit/src/options.rs b/yjit/src/options.rs index c87a436091279f..0f24bfdddfa29d 100644 --- a/yjit/src/options.rs +++ b/yjit/src/options.rs @@ -3,10 +3,10 @@ use crate::{backend::current::TEMP_REGS, cruby::*, stats::Counter}; use std::os::raw::{c_char, c_int, c_uint}; // Call threshold for small deployments and command-line apps -pub static SMALL_CALL_THRESHOLD: u64 = 30; +pub static SMALL_CALL_THRESHOLD: u32 = 30; // Call threshold for larger deployments and production-sized applications -pub static LARGE_CALL_THRESHOLD: u64 = 120; +pub static LARGE_CALL_THRESHOLD: u32 = 120; // Number of live ISEQs after which we consider an app to be large pub static LARGE_ISEQ_COUNT: u64 = 40_000; @@ -15,13 +15,13 @@ pub static LARGE_ISEQ_COUNT: u64 = 40_000; // Number of method calls after which to start generating code // Threshold==1 means compile on first execution #[no_mangle] -pub static mut rb_yjit_call_threshold: u64 = SMALL_CALL_THRESHOLD; +pub static mut rb_yjit_call_threshold: u32 = SMALL_CALL_THRESHOLD; // This option is exposed to the C side in a global variable for performance, see vm.c // Number of execution requests after which a method is no longer // considered hot. Raising this results in more generated code. #[no_mangle] -pub static mut rb_yjit_cold_threshold: u64 = 200_000; +pub static mut rb_yjit_cold_threshold: u32 = 200_000; // Command-line options #[derive(Debug)] diff --git a/yjit/src/yjit.rs b/yjit/src/yjit.rs index 33e4308b2999af..2a48c94de6f4c1 100644 --- a/yjit/src/yjit.rs +++ b/yjit/src/yjit.rs @@ -238,7 +238,7 @@ pub extern "C" fn rb_yjit_enable(_ec: EcPtr, _ruby_self: VALUE, gen_stats: VALUE if !call_threshold.nil_p() { let threshold = call_threshold.as_isize() >> 1; unsafe { - rb_yjit_call_threshold = threshold as u64; + rb_yjit_call_threshold = threshold as u32; } } diff --git a/zjit.h b/zjit.h index f7d84e2a1157aa..a81011c6b572f0 100644 --- a/zjit.h +++ b/zjit.h @@ -109,8 +109,8 @@ ZJIT_STACK_MAP_BASE_PTR_STACK_SIZE(VALUE entry) extern void *rb_zjit_entry; extern bool rb_zjit_compiling_p; extern const zjit_jit_frame_t rb_zjit_c_frame; -extern uint64_t rb_zjit_call_threshold; -extern uint64_t rb_zjit_profile_threshold; +extern unsigned int rb_zjit_call_threshold; +extern unsigned int rb_zjit_profile_threshold; void rb_zjit_compile_iseq(const rb_iseq_t *iseq, rb_execution_context_t *ec, bool jit_exception); void rb_zjit_profile_insn(uint32_t insn, rb_execution_context_t *ec); void rb_zjit_profile_enable(const rb_iseq_t *iseq); diff --git a/zjit/src/options.rs b/zjit/src/options.rs index 3529bbc8c143e7..27f2fa90762daf 100644 --- a/zjit/src/options.rs +++ b/zjit/src/options.rs @@ -25,7 +25,7 @@ pub type NumProfiles = u16; /// Default --zjit-call-threshold. This should be large enough to avoid compiling /// warmup code, but small enough to perform well on micro-benchmarks. pub const DEFAULT_CALL_THRESHOLD: CallThreshold = 30; -pub type CallThreshold = u64; +pub type CallThreshold = u32; /// Default --zjit-inline-threshold /// TODO (nirvdrum 2026-06-25): 30 has proven to work well with ruby-bench, but we should finely