From 145a4bba92aedb13a796c6d3fee712ddc9d28f25 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Tue, 21 Jul 2026 19:23:28 +0900 Subject: [PATCH 1/4] [ruby/rubygems] Validate gem name before building compact index cache paths Bundler::CompactIndexClient::Cache#info_path and #info_etag_path join the gem name into the cache directory without validation. The name comes from the remote index, either versions lines or transitive dependency names in info files, so a crafted name like "../../../../pwn" escapes the cache directory because the special-characters branch keeps the raw name and only appends an MD5 suffix. Reject any name that is not a plain basename before constructing the path, matching the fetch_spec guard from https://github.com/ruby/rubygems/commit/56ed326cb2, and apply the same guard to the independent Gem::CompactIndexClient::Cache port. https://github.com/ruby/rubygems/commit/4b1c454526 Co-Authored-By: Claude Fable 5 --- lib/bundler/compact_index_client/cache.rb | 11 +++++++++ lib/rubygems/compact_index_client/cache.rb | 11 +++++++++ .../compact_index_client/cache_spec.rb | 19 +++++++++++++++ spec/bundler/support/shards.rb | 1 + .../test_gem_compact_index_client_cache.rb | 23 +++++++++++++++++++ 5 files changed, 65 insertions(+) create mode 100644 spec/bundler/bundler/compact_index_client/cache_spec.rb diff --git a/lib/bundler/compact_index_client/cache.rb b/lib/bundler/compact_index_client/cache.rb index 3bae6c9efdf8f5..c47535a0196195 100644 --- a/lib/bundler/compact_index_client/cache.rb +++ b/lib/bundler/compact_index_client/cache.rb @@ -50,6 +50,7 @@ def versions_etag_path = directory.join("versions.etag") def info_path(name) name = name.to_s + validate_name!(name) # TODO: converge this into the info_root by hashing all filenames like info_etag_path if /[^a-z0-9_-]/.match?(name) name += "-#{SharedHelpers.digest(:MD5).hexdigest(name).downcase}" @@ -61,9 +62,19 @@ def info_path(name) def info_etag_path(name) name = name.to_s + validate_name!(name) @info_etag_root.join("#{name}-#{SharedHelpers.digest(:MD5).hexdigest(name).downcase}") end + # Gem names come from the remote index and are not otherwise + # validated, so refuse anything that would escape the cache + # directory when used as a path component. + def validate_name!(name) + return if File.basename(name) == name + + raise SecurityError, "malformed gem name: #{name.inspect}" + end + def mkdir(name) directory.join(name).tap do |dir| SharedHelpers.filesystem_access(dir) do diff --git a/lib/rubygems/compact_index_client/cache.rb b/lib/rubygems/compact_index_client/cache.rb index 34e20dfea0b832..84c74fb86a1fb7 100644 --- a/lib/rubygems/compact_index_client/cache.rb +++ b/lib/rubygems/compact_index_client/cache.rb @@ -60,6 +60,7 @@ def versions_etag_path = directory.join("versions.etag") def info_path(name) name = name.to_s + validate_name!(name) if /[^a-z0-9_-]/.match?(name) name += "-#{Digest::MD5.hexdigest(name).downcase}" @special_characters_info_root.join(name) @@ -70,9 +71,19 @@ def info_path(name) def info_etag_path(name) name = name.to_s + validate_name!(name) @info_etag_root.join("#{name}-#{Digest::MD5.hexdigest(name).downcase}") end + # Gem names come from the remote index and are not otherwise + # validated, so refuse anything that would escape the cache + # directory when used as a path component. + def validate_name!(name) + return if File.basename(name) == name + + raise Gem::Exception, "malformed gem name: #{name.inspect}" + end + def checksum_for_file(path) return unless path.file? Digest::MD5.file(path).hexdigest diff --git a/spec/bundler/bundler/compact_index_client/cache_spec.rb b/spec/bundler/bundler/compact_index_client/cache_spec.rb new file mode 100644 index 00000000000000..85528c0cac32da --- /dev/null +++ b/spec/bundler/bundler/compact_index_client/cache_spec.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require "bundler/compact_index_client" +require "bundler/compact_index_client/cache" +require "tmpdir" + +RSpec.describe Bundler::CompactIndexClient::Cache do + let(:directory) { Pathname.new(Dir.mktmpdir("compact_index")) } + let(:cache) { described_class.new(directory) } + + it "rejects a gem name that would escape the cache directory" do + expect { cache.info("../../../../pwn") }.to raise_error(Bundler::SecurityError, /malformed gem name/) + end + + it "accepts plain gem names, including special characters" do + expect { cache.info("rack") }.not_to raise_error + expect { cache.info("Rails.js") }.not_to raise_error + end +end diff --git a/spec/bundler/support/shards.rb b/spec/bundler/support/shards.rb index 72b1a4a1b7f6dd..0de964017030b5 100644 --- a/spec/bundler/support/shards.rb +++ b/spec/bundler/support/shards.rb @@ -148,6 +148,7 @@ module Shards ], shard_d: [ "spec/bundler/compact_index_client/cache_file_spec.rb", + "spec/bundler/compact_index_client/cache_spec.rb", "spec/bundler/rubygems_ext_spec.rb", "spec/bundler/resolver/cooldown_spec.rb", "spec/install/cooldown_spec.rb", diff --git a/test/rubygems/test_gem_compact_index_client_cache.rb b/test/rubygems/test_gem_compact_index_client_cache.rb index e34ddce6f633be..2951f8e324191e 100644 --- a/test/rubygems/test_gem_compact_index_client_cache.rb +++ b/test/rubygems/test_gem_compact_index_client_cache.rb @@ -113,6 +113,29 @@ def test_fetch_info_fetches_without_checksum assert_equal "a 1.0.0\n", @dir.join("info", "a").read end + def test_info_rejects_name_escaping_cache_directory + fetcher = FakeFetcher.new("1.0.0\n") + cache = Gem::CompactIndexClient::Cache.new(@dir, fetcher) + + e = assert_raise Gem::Exception do + cache.info("../../../../pwn", "no-match") + end + + assert_includes e.message, "malformed gem name" + assert_empty fetcher.requests + end + + def test_fetch_info_rejects_name_escaping_cache_directory + fetcher = FakeFetcher.new("1.0.0\n") + cache = Gem::CompactIndexClient::Cache.new(@dir, fetcher) + + assert_raise Gem::Exception do + cache.fetch_info("../pwn") + end + + assert_empty fetcher.requests + end + def test_info_with_special_characters_uses_hashed_path fetcher = FakeFetcher.new("1.0.0\n") cache = Gem::CompactIndexClient::Cache.new(@dir, fetcher) From e15f28c57debedd44bd45bc3aae811fee338e6c2 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Thu, 16 Jul 2026 17:13:49 +0900 Subject: [PATCH 2/4] [ruby/rubygems] Redact URI userinfo from settings names in Bundler User-Agent The `options/` segment of Bundler's User-Agent is built from `Bundler.settings.all`, which returns only setting names. Mirror settings, however, embed a URI in the key itself (`mirror.http://user:token@host/`), so the URI userinfo becomes part of the name. That name is then sent to every gem source Bundler contacts, including public or attacker-controlled sources, leaking the embedded credentials cross-origin. Strip the userinfo from any settings key that embeds a URI when building the User-Agent, keeping the useful telemetry (which settings are in use and the host) while dropping the secret. The redaction is limited to the User-Agent; `Bundler.settings.all` still returns the real keys, which `local_overrides` and `gem_mirrors` rely on. https://github.com/ruby/rubygems/commit/1468529cbe Co-Authored-By: Claude Opus 4.8 --- lib/bundler/fetcher.rb | 8 +++++++- spec/bundler/bundler/fetcher_spec.rb | 13 +++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/lib/bundler/fetcher.rb b/lib/bundler/fetcher.rb index 39e1856909a155..ce6f5fdcf402be 100644 --- a/lib/bundler/fetcher.rb +++ b/lib/bundler/fetcher.rb @@ -215,7 +215,13 @@ def user_agent agent << " #{ruby.engine}/#{ruby.versions_string(engine_version)}" end - agent << " options/#{Bundler.settings.all.join(",")}" + # Some settings keys embed a URI (e.g. `mirror.`) whose userinfo can + # carry credentials. Strip the userinfo before advertising the setting + # names so we don't leak secrets to every gem source we talk to. + options = Bundler.settings.all.map do |key| + key.include?("://") ? key.gsub(%r{//[^/@]+@}, "//") : key + end + agent << " options/#{options.join(",")}" agent << " ci/#{cis.join(",")}" if cis.any? diff --git a/spec/bundler/bundler/fetcher_spec.rb b/spec/bundler/bundler/fetcher_spec.rb index 3685d104c0eb42..6891a7991de636 100644 --- a/spec/bundler/bundler/fetcher_spec.rb +++ b/spec/bundler/bundler/fetcher_spec.rb @@ -141,6 +141,19 @@ expect(fetcher.user_agent).to match(%r{options/foo,bar}) end + it "strips userinfo from settings keys that embed a URI" do + allow(Bundler.settings).to receive(:all).and_return( + %w[foo mirror.http://user:token@example.org/ mirror.https://token@example.net/] + ) + options = fetcher.user_agent.split(" ").find {|x| x.start_with?("options/") } + + expect(options).not_to include("token") + expect(options).not_to include("user:") + expect(options).to include("mirror.http://example.org/") + expect(options).to include("mirror.https://example.net/") + expect(options).to include("foo") + end + describe "include CI information" do it "from one CI" do with_env_vars("CI" => nil, "JENKINS_URL" => "foo") do From 553f19d9b27bd265f18e1c754e2884e426023729 Mon Sep 17 00:00:00 2001 From: Shizuo Fujita Date: Thu, 30 Jul 2026 15:24:42 +0900 Subject: [PATCH 3/4] [DOC] Fix the $? description of Process::Status.wait (#18109) The documentation says that Process::Status.wait sets the thread-local variable $? when there are child processes, but it never does. rb_process_status_wait does not call rb_last_status_set, and $? is left untouched: $? # => nil Process.spawn('cat /nop') # => 4996 Process::Status.wait # => # $? # => nil The existing example showed this as well: the pid displayed for $? (1155508) did not match the pid of the returned status (1155880), contradicting the surrounding text. Since $? is never set, the "does not set thread-local variable $?" note in the no-child-process case is now redundant and has been folded into the general description. --- process.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/process.c b/process.c index 92c5502e3f5dab..9c2659af07d251 100644 --- a/process.c +++ b/process.c @@ -1116,20 +1116,19 @@ rb_process_status_wait(rb_pid_t pid, int flags) * * If there are child processes, * waits for a child process to exit and returns a Process::Status object - * containing information on that process; - * sets thread-local variable $?: + * containing information on that process. + * Unlike Process.wait, this method does not set thread-local variable + * $?: * * Process.spawn('cat /nop') # => 1155880 * Process::Status.wait # => # - * $? # => # + * $? # => nil # Not set. * * If there is no child process, * returns an "empty" Process::Status object - * that does not represent an actual process; - * does not set thread-local variable $?: + * that does not represent an actual process: * * Process::Status.wait # => # - * $? # => # # Unchanged. * * May invoke the scheduler hook Fiber::Scheduler#process_wait. * From acc86e6d5c9a316fc473959a91c941f150fb5254 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Tue, 28 Jul 2026 18:22:04 +0900 Subject: [PATCH 4/4] [ruby/rubygems] Warn when duplicate source declarations conflict on cooldown Declaring the same source URL twice with different cooldown values silently drops one of them, because duplicate declarations are deduped into a single source and cooldown applies per server, not per gem. The reporter of #9723 used a second source block with cooldown: 0 as a per-gem exemption and got no indication it had no effect. Keep the dedup behavior but warn when a conflicting cooldown value is discarded. https://github.com/rubygems/rubygems/issues/9723 https://github.com/ruby/rubygems/commit/a2a44cf4e0 Co-Authored-By: Claude Fable 5 --- lib/bundler/source_list.rb | 31 ++++++++++++++- spec/bundler/bundler/dsl_spec.rb | 1 + spec/bundler/bundler/source_list_spec.rb | 50 ++++++++++++++++++++++++ spec/bundler/install/cooldown_spec.rb | 39 ++++++++++++++++++ 4 files changed, 120 insertions(+), 1 deletion(-) diff --git a/lib/bundler/source_list.rb b/lib/bundler/source_list.rb index 23c73c23f1b27d..ea59ecdd5c5791 100644 --- a/lib/bundler/source_list.rb +++ b/lib/bundler/source_list.rb @@ -50,7 +50,13 @@ def add_git_source(options = {}) def add_rubygems_source(options = {}) new_source = Source::Rubygems.new(options) - return @global_rubygems_source if @global_rubygems_source == new_source + if @global_rubygems_source == new_source + warn_on_cooldown_conflict(new_source, @global_rubygems_source) + return @global_rubygems_source + end + + existing_source = @rubygems_sources.find {|s| s == new_source } + warn_on_cooldown_conflict(new_source, existing_source) if existing_source add_source_to_list new_source, @rubygems_sources end @@ -60,6 +66,13 @@ def add_plugin_source(source, options = {}) end def add_global_rubygems_remote(uri, cooldown: nil) + unless cooldown.nil? + new_source = source_class.new("remotes" => uri, "cooldown" => cooldown) + [global_rubygems_source, *@rubygems_sources].find do |existing_source| + warn_on_cooldown_conflict(new_source, existing_source) + end + end + global_rubygems_source.add_remote(uri, cooldown: cooldown) global_rubygems_source end @@ -222,6 +235,22 @@ def source_list_for(source) end end + def warn_on_cooldown_conflict(new_source, existing_source) + new_source.remote_cooldowns.any? do |uri, cooldown| + next false unless existing_source.remotes.include?(uri) + + existing_cooldown = existing_source.cooldown_for(uri) + next false if existing_cooldown == cooldown + + previous = existing_cooldown ? "`cooldown: #{existing_cooldown}`" : "no cooldown" + Bundler.ui.warn "The source #{uri} is declared more than once with different cooldown " \ + "values (`cooldown: #{cooldown}` here, #{previous} previously). All declarations of " \ + "the same source URL share a single cooldown, so only one of these values will apply " \ + "to all gems from this source." + true + end + end + def warn_on_git_protocol(source) return if Bundler.settings["git.allow_insecure"] diff --git a/spec/bundler/bundler/dsl_spec.rb b/spec/bundler/bundler/dsl_spec.rb index b6e67a312c0d41..902bd1ad6f2a45 100644 --- a/spec/bundler/bundler/dsl_spec.rb +++ b/spec/bundler/bundler/dsl_spec.rb @@ -370,6 +370,7 @@ describe "#source with cooldown" do before do allow(@rubygems).to receive(:add_remote) + allow(@rubygems).to receive(:remote_cooldowns).and_return({}) end it "accepts a non-negative integer" do diff --git a/spec/bundler/bundler/source_list_spec.rb b/spec/bundler/bundler/source_list_spec.rb index 7a8e885b2e1bbb..c67edb2f201b73 100644 --- a/spec/bundler/bundler/source_list_spec.rb +++ b/spec/bundler/bundler/source_list_spec.rb @@ -161,6 +161,56 @@ end end + describe "cooldown conflicts between duplicate source declarations" do + it "warns when a block source duplicates the global source with a different cooldown" do + source_list.add_global_rubygems_remote("https://rubygems.org", cooldown: 7) + expect(Bundler.ui).to receive(:warn).with(/declared more than once with different cooldown values \(`cooldown: 0` here, `cooldown: 7` previously\)/) + source_list.add_rubygems_source("remotes" => ["https://rubygems.org"], "cooldown" => 0) + end + + it "warns when a global source duplicates a block source with a different cooldown" do + source_list.add_rubygems_source("remotes" => ["https://rubygems.org"], "cooldown" => 0) + expect(Bundler.ui).to receive(:warn).with(/declared more than once with different cooldown values \(`cooldown: 7` here, `cooldown: 0` previously\)/) + source_list.add_global_rubygems_remote("https://rubygems.org", cooldown: 7) + end + + it "warns when a block source duplicates another block source with a different cooldown" do + source_list.add_rubygems_source("remotes" => ["https://rubygems.org"], "cooldown" => 7) + expect(Bundler.ui).to receive(:warn).with(/declared more than once with different cooldown values/) + source_list.add_rubygems_source("remotes" => ["https://rubygems.org"], "cooldown" => 0) + end + + it "warns when a global remote is declared again with a different cooldown" do + source_list.add_global_rubygems_remote("https://rubygems.org", cooldown: 7) + expect(Bundler.ui).to receive(:warn).with(/declared more than once with different cooldown values/) + source_list.add_global_rubygems_remote("https://rubygems.org", cooldown: 0) + end + + it "warns when a duplicate declaration adds a cooldown to a source declared without one" do + source_list.add_global_rubygems_remote("https://rubygems.org") + expect(Bundler.ui).to receive(:warn).with(/declared more than once with different cooldown values \(`cooldown: 7` here, no cooldown previously\)/) + source_list.add_rubygems_source("remotes" => ["https://rubygems.org"], "cooldown" => 7) + end + + it "does not warn when the duplicate declaration uses the same cooldown" do + source_list.add_global_rubygems_remote("https://rubygems.org", cooldown: 7) + expect(Bundler.ui).not_to receive(:warn) + source_list.add_rubygems_source("remotes" => ["https://rubygems.org"], "cooldown" => 7) + end + + it "does not warn when the duplicate declaration has no cooldown" do + source_list.add_global_rubygems_remote("https://rubygems.org", cooldown: 7) + expect(Bundler.ui).not_to receive(:warn) + source_list.add_rubygems_source("remotes" => ["https://rubygems.org"]) + end + + it "does not warn when different sources declare different cooldowns" do + source_list.add_global_rubygems_remote("https://rubygems.org", cooldown: 7) + expect(Bundler.ui).not_to receive(:warn) + source_list.add_rubygems_source("remotes" => ["https://other-rubygems.org"], "cooldown" => 0) + end + end + describe "#all_sources" do it "includes the global rubygems source when rubygems sources have been added" do source_list.add_git_source("uri" => "git://host/path.git") diff --git a/spec/bundler/install/cooldown_spec.rb b/spec/bundler/install/cooldown_spec.rb index 2724132263cd84..5825f91b5789bd 100644 --- a/spec/bundler/install/cooldown_spec.rb +++ b/spec/bundler/install/cooldown_spec.rb @@ -257,6 +257,45 @@ expect(the_bundle).to include_gems("ripe_gem 1.0.0", "child 1.0.0") end + it "warns when the same source is declared again with a different cooldown and keeps the first value" do + # https://github.com/rubygems/rubygems/issues/9723: a second declaration + # of the same URL is deduped into the first one, so its cooldown cannot + # act as a per-gem exemption. + install_gemfile <<-G, artifice: "compact_index_cooldown" + source "https://gem.repo3", cooldown: 7 + source "https://gem.repo3", cooldown: 0 do + gem "ripe_gem" + end + G + + expect(err).to include("The source https://gem.repo3/ is declared more than once with different cooldown values (`cooldown: 0` here, `cooldown: 7` previously).") + expect(the_bundle).to include_gems("ripe_gem 1.0.0") + end + + it "does not warn when the same source is declared again without a cooldown" do + install_gemfile <<-G, artifice: "compact_index_cooldown" + source "https://gem.repo3", cooldown: 7 + source "https://gem.repo3" do + gem "ripe_gem" + end + G + + expect(err).not_to include("cooldown") + expect(the_bundle).to include_gems("ripe_gem 1.0.0") + end + + it "does not warn when the same source is declared again with the same cooldown" do + install_gemfile <<-G, artifice: "compact_index_cooldown" + source "https://gem.repo3", cooldown: 7 + source "https://gem.repo3", cooldown: 7 do + gem "ripe_gem" + end + G + + expect(err).not_to include("cooldown") + expect(the_bundle).to include_gems("ripe_gem 1.0.0") + end + it "is overridden by CLI --cooldown when Gemfile sets a different per-source value" do gemfile <<-G source "https://gem.repo3", cooldown: 0