Skip to content

Commit e5c8f5f

Browse files
authored
Merge pull request #9754 from ruby/claude/infallible-sinoussi-16ca9f
Prevent test git commands from mutating the checkout's own git config
2 parents d834fa4 + 168266e commit e5c8f5f

6 files changed

Lines changed: 66 additions & 4 deletions

File tree

spec/bundler/installer/parallel_installer_spec.rb

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@
88
RSpec.describe Bundler::ParallelInstaller do
99
describe "priority queue" do
1010
before do
11+
# Anchor the vendored Persistent classes on the real Gem::Net::HTTP
12+
# before Artifice replaces it, see Artifice.activate_with. Requiring
13+
# support/artifice/compact_index already activates Artifice, so this
14+
# must come first.
15+
require "bundler/vendored_persistent"
1116
require "support/artifice/compact_index"
1217
Artifice.activate_with(CompactIndexAPI)
1318

@@ -98,6 +103,11 @@
98103
skip "This example does not work under a parent make jobserver"
99104
end
100105

106+
# Anchor the vendored Persistent classes on the real Gem::Net::HTTP
107+
# before Artifice replaces it, see Artifice.activate_with. Requiring
108+
# support/artifice/compact_index already activates Artifice, so this
109+
# must come first.
110+
require "bundler/vendored_persistent"
101111
require "support/artifice/compact_index"
102112
Artifice.activate_with(CompactIndexAPI)
103113

spec/commands/show_spec.rb

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,13 @@
164164
before :each do
165165
build_git "foo", path: lib_path("foo")
166166
File.open(lib_path("foo/Gemfile"), "w") {|f| f.puts "gemspec" }
167-
sys_exec "rm -rf .git && git init", dir: lib_path("foo")
167+
# sys_exec does not go through a shell, so this cannot be a single
168+
# `rm -rf .git && git init` command: `&&` would be passed to `rm` as a
169+
# literal argument, silently skipping the `git init` part and leaving a
170+
# non-repository directory from which git would discover the rubygems
171+
# checkout itself.
172+
FileUtils.rm_rf lib_path("foo/.git")
173+
git "init", lib_path("foo")
168174
end
169175

170176
it "does not output git errors" do

spec/spec_helper.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,14 @@ def self.ruby=(ruby)
147147
ENV["GIT_CONFIG_NOSYSTEM"] = "1"
148148
end
149149

150+
# Prevent git commands spawned by specs (directly or through Bundler)
151+
# from discovering the rubygems checkout itself when run in a directory
152+
# that is not a fixture repository, e.g. a fixture whose .git has been
153+
# deleted by a concurrent cleanup. Without this, repository discovery
154+
# walks up into the checkout and a stray `git config` writes the fixture
155+
# identity to the checkout's own (possibly worktree-shared) .git/config.
156+
ENV["GIT_CEILING_DIRECTORIES"] = [Spec::Path.tmp_root.to_s, Spec::Path.source_root.to_s].uniq.join(File::PATH_SEPARATOR)
157+
150158
# Disable git background maintenance. Since Git 2.46, commands like
151159
# `git commit` spawn a detached `git maintenance run --auto` process,
152160
# which briefly creates `.git/objects/maintenance.lock`. That races with

spec/support/artifice/helpers/artifice.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,14 @@ module Artifice
99
# Rack endpoint.
1010
#
1111
# @param [#call] endpoint A valid Rack endpoint
12+
# In-process users that also deactivate must load bundler/vendored_persistent
13+
# before activating. If it gets lazily required while Artifice is active, the
14+
# vendored Persistent classes are defined under the Artifice replacement of
15+
# Gem::Net::HTTP instead of the real one, and after deactivation
16+
# Gem::Net::HTTP::Persistent becomes unresolvable, blowing up the
17+
# connection_pool fork hook on any later Process.fork. Spawned bundler
18+
# processes are unaffected: they never deactivate, and requiring it here
19+
# would double-load bundler files in them through mismatched load paths.
1220
def self.activate_with(endpoint)
1321
require_relative "rack_request"
1422

spec/support/subprocess.rb

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,33 @@ def exitstatus
3131
end
3232

3333
def git(cmd, path = Dir.pwd, options = {})
34+
reject_git_config_pollution!(cmd, path)
3435
sh("git #{cmd}", options.merge(dir: path))
3536
end
3637

38+
# A local `git config` write in a directory without a `.git` makes git
39+
# discover an enclosing repository, which can be the rubygems checkout
40+
# itself, polluting its (possibly worktree-shared) `.git/config` with
41+
# fixture identities. Only allow local config writes inside tmp/.
42+
def reject_git_config_pollution!(cmd, path)
43+
require "shellwords"
44+
args = cmd.to_s.shellsplit
45+
return unless args.first == "config"
46+
return if args.any? {|a| ["--global", "--system", "-f", "--file"].include?(a) || a.start_with?("--file=") }
47+
return if args.any? {|a| ["--get", "--get-all", "--get-regexp", "--get-urlmatch", "--list", "-l"].include?(a) }
48+
49+
# Required lazily because this file is loaded in every spawned ruby
50+
# before RubygemsVersionManager switches RubyGems, where loading extra
51+
# default gems (pathname, through support/path) breaks the setup.
52+
require_relative "path"
53+
dir = File.expand_path(path.to_s)
54+
tmp_root = Spec::Path.tmp_root.to_s
55+
return if dir == tmp_root || dir.start_with?(tmp_root + File::SEPARATOR)
56+
57+
raise "Refusing to run `git #{cmd}` in #{dir}: " \
58+
"a local git config write outside tmp/ could end up in the checkout's own .git/config"
59+
end
60+
3761
def sh(cmd, options = {})
3862
dir = options[:dir]
3963
env = options[:env] || {}

test/rubygems/helper.rb

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,12 @@ def setup
373373

374374
@tempdir = Dir.mktmpdir("test_rubygems_", @tmp)
375375

376+
# @tmp lives inside the checkout by default, so stop git repository
377+
# discovery from walking up into the checkout itself. Otherwise a git
378+
# command run in a non-repository directory under @tempdir could mutate
379+
# the checkout's own (possibly worktree-shared) .git/config.
380+
ENV["GIT_CEILING_DIRECTORIES"] = File.realpath(top_srcdir)
381+
376382
ENV["GEM_VENDOR"] = nil
377383
ENV["GEMRC"] = nil
378384
ENV["XDG_CACHE_HOME"] = nil
@@ -661,9 +667,9 @@ def git_gem(name = "a", version = 1)
661667

662668
Dir.chdir directory do
663669
unless File.exist? ".git"
664-
system @git, "init", "--quiet"
665-
system @git, "config", "user.name", "RubyGems Tests"
666-
system @git, "config", "user.email", "rubygems@example"
670+
system @git, "init", "--quiet", exception: true
671+
system @git, "config", "user.name", "RubyGems Tests", exception: true
672+
system @git, "config", "user.email", "rubygems@example", exception: true
667673
end
668674

669675
system @git, "add", gemspec

0 commit comments

Comments
 (0)