* lib/rubygems/commands/push_command.rb: Fixed credential download for

`gem push --host`
* lib/rubygems/gemcutter_utilities.rb:  ditto.
* test/rubygems/test_gem_commands_push_command.rb:  Test for the above.
* test/rubygems/test_gem_gemcutter_utilities.rb:  ditto.

* lib/rubygems/config_file.rb:  Abort if the `gem push` credentials
  file has insecure permissions.
* test/rubygems/test_gem_config_file.rb:  Test for the above.

* lib/rubygems/ext/builder.rb:  Do not look for Gemfile, Isolate, etc.
  while building gem extensions.

* lib/rubygems/package.rb:  Unset spec and files list if a gem's
  signatures cannot be verified.
* test/rubygems/test_gem_package.rb:  Test for the above.

* lib/rubygems/specification.rb:  Reduce use of eval.
* lib/rubygems/test_case.rb:  ditto.

* test/rubygems/test_gem_specification.rb:  Test setting
  specification_version for legacy gems.  Dup Gem.ruby before
  untainting in case it's frozen.

* lib/rubygems.rb:  Reduce use of eval.  Only read files when looking
  for Gemfile, Isolate, etc.
* test/rubygems/test_gem.rb:  Test for the above.


git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@39055 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

ChangeLog

@@ -1,3 +1,33 @@
+Tue Feb  5 11:35:35 2013  Eric Hodel  <drbrain@segment7.net>
+
+	* lib/rubygems/commands/push_command.rb:  Fixed credential download for
+	  `gem push --host`
+	* lib/rubygems/gemcutter_utilities.rb:  ditto.
+	* test/rubygems/test_gem_commands_push_command.rb:  Test for the above.
+	* test/rubygems/test_gem_gemcutter_utilities.rb:  ditto.
+
+	* lib/rubygems/config_file.rb:  Abort if the `gem push` credentials
+	  file has insecure permissions.
+	* test/rubygems/test_gem_config_file.rb:  Test for the above.
+
+	* lib/rubygems/ext/builder.rb:  Do not look for Gemfile, Isolate, etc.
+	  while building gem extensions.
+
+	* lib/rubygems/package.rb:  Unset spec and files list if a gem's
+	  signatures cannot be verified.
+	* test/rubygems/test_gem_package.rb:  Test for the above.
+
+	* lib/rubygems/specification.rb:  Reduce use of eval.
+	* lib/rubygems/test_case.rb:  ditto.
+
+	* test/rubygems/test_gem_specification.rb:  Test setting
+	  specification_version for legacy gems.  Dup Gem.ruby before
+	  untainting in case it's frozen.
+
+	* lib/rubygems.rb:  Reduce use of eval.  Only read files when looking
+	  for Gemfile, Isolate, etc.
+	* test/rubygems/test_gem.rb:  Test for the above.
+
 Tue Feb  5 10:15:00 2013  Zachary Scott  <zachary@zacharyscott.net>
 
 	* doc/security.rdoc: Wrap security guide at 80 columns

lib/rubygems.rb

@@ -207,7 +207,7 @@ module Gem
 
         begin
           while true
-            path = GEM_DEP_FILES.find { |f| File.exists?(f) }
+            path = GEM_DEP_FILES.find { |f| File.file?(f) }
 
             if path
               path = File.join here, path
@@ -226,7 +226,9 @@ module Gem
         end
       end
 
-      return unless File.exists? path
+      path.untaint
+
+      return unless File.file? path
 
       rs = Gem::RequestSet.new
       rs.load_gemdeps path
@@ -369,29 +371,6 @@ module Gem
     paths.path
   end
 
-  ##
-  # Expand each partial gem path with each of the required paths specified
-  # in the Gem spec.  Each expanded path is yielded.
-
-  def self.each_load_path(partials)
-    partials.each do |gp|
-      base = File.basename gp
-      specfn = File.join(dir, "specifications", "#{base}.gemspec")
-      if File.exists? specfn
-        spec = eval(File.read(specfn))
-        spec.require_paths.each do |rp|
-          yield File.join(gp,rp)
-        end
-      else
-        filename = File.join(gp, 'lib')
-        yield(filename) if File.exists? filename
-      end
-    end
-  end
-
-  private_class_method :each_load_path
-
-
   ##
   # Quietly ensure the named Gem directory contains all the proper
   # subdirectories.  If we can't create a directory due to a permission

lib/rubygems/commands/push_command.rb

@@ -24,16 +24,19 @@ class Gem::Commands::PushCommand < Gem::Command
     add_proxy_option
     add_key_option
 
-    add_option(
+    add_option('--host HOST',
-      '--host HOST',
+               'Push to another gemcutter-compatible host') do |value, options|
-      'Push to another gemcutter-compatible host'
-    ) do |value, options|
       options[:host] = value
     end
+
+    @host = nil
   end
 
   def execute
-    sign_in
+    @host = options[:host]
+
+    sign_in @host
+
     send_gem get_one_gem_name
   end
 
@@ -44,26 +47,30 @@ class Gem::Commands::PushCommand < Gem::Command
 
     if latest_rubygems_version < Gem.rubygems_version and
          Gem.rubygems_version.prerelease? and
-         Gem::Version.new('2.0.0.preview3') != Gem.rubygems_version then
+         Gem::Version.new('2.0.0.rc.2') != Gem.rubygems_version then
       alert_error <<-ERROR
 You are using a beta release of RubyGems (#{Gem::VERSION}) which is not
 allowed to push gems.  Please downgrade or upgrade to a release version.
 
 The latest released RubyGems version is #{latest_rubygems_version}
+
+You can upgrade or downgrade to the latest release version with:
+
+  gem update --system=#{latest_rubygems_version}
+
       ERROR
       terminate_interaction 1
     end
 
-    host = options[:host]
+    unless @host then
-    unless host
       if gem_data = Gem::Package.new(name) then
-        host = gem_data.spec.metadata['default_gem_server']
+        @host = gem_data.spec.metadata['default_gem_server']
       end
     end
 
-    args << host if host
+    args << @host if @host
 
-    say "Pushing gem to #{host || Gem.host}..."
+    say "Pushing gem to #{@host || Gem.host}..."
 
     response = rubygems_api_request(*args) do |request|
       request.body = Gem.read_binary name

lib/rubygems/config_file.rb

@@ -33,6 +33,8 @@
 
 class Gem::ConfigFile
 
+  include Gem::UserInteraction
+
   DEFAULT_BACKTRACE = false
   DEFAULT_BULK_THRESHOLD = 1000
   DEFAULT_VERBOSITY = true
@@ -223,6 +225,34 @@ class Gem::ConfigFile
     handle_arguments arg_list
   end
 
+  ##
+  # Checks the permissions of the credentials file.  If they are not 0600 an
+  # error message is displayed and RubyGems aborts.
+
+  def check_credentials_permissions
+    return unless File.exist? credentials_path
+
+    existing_permissions = File.stat(credentials_path).mode & 0777
+
+    return if existing_permissions == 0600
+
+    alert_error <<-ERROR
+Your gem push credentials file located at:
+
+\t#{credentials_path}
+
+has file permissions of 0#{existing_permissions.to_s 8} but 0600 is required.
+
+You should reset your credentials at:
+
+\thttps://rubygems.org/profile/edit
+
+if you believe they were disclosed to a third party.
+    ERROR
+
+    terminate_interaction 1
+  end
+
   ##
   # Location of RubyGems.org credentials
 
@@ -231,6 +261,8 @@ class Gem::ConfigFile
   end
 
   def load_api_keys
+    check_credentials_permissions
+
     @api_keys = if File.exist? credentials_path then
                   load_file(credentials_path)
                 else
@@ -243,7 +275,9 @@ class Gem::ConfigFile
     end
   end
 
-  def rubygems_api_key=(api_key)
+  def rubygems_api_key= api_key
+    check_credentials_permissions
+
     config = load_file(credentials_path).merge(:rubygems_api_key => api_key)
 
     dirname = File.dirname credentials_path

lib/rubygems/ext/builder.rb

@@ -43,12 +43,18 @@ class Gem::Ext::Builder
   def self.run(command, results, command_name = nil)
     verbose = Gem.configuration.really_verbose
 
-    if verbose
+    begin
-      puts(command)
+      # TODO use Process.spawn when ruby 1.8 support is dropped.
-      system(command)
+      rubygems_gemdeps, ENV['RUBYGEMS_GEMDEPS'] = ENV['RUBYGEMS_GEMDEPS'], nil
-    else
+      if verbose
-      results << command
+        puts(command)
-      results << `#{command} #{redirector}`
+        system(command)
+      else
+        results << command
+        results << `#{command} #{redirector}`
+      end
+    ensure
+      ENV['RUBYGEMS_GEMDEPS'] = rubygems_gemdeps
     end
 
     unless $?.success? then

lib/rubygems/gemcutter_utilities.rb

@@ -27,17 +27,25 @@ module Gem::GemcutterUtilities
     end
   end
 
-  def sign_in
+  def sign_in sign_in_host = self.host
     return if Gem.configuration.rubygems_api_key
 
-    say "Enter your RubyGems.org credentials."
+    pretty_host = if Gem::DEFAULT_HOST == sign_in_host then
-    say "Don't have an account yet? Create one at http://rubygems.org/sign_up"
+                    'RubyGems.org'
+                  else
+                    sign_in_host
+                  end
+
+    say "Enter your #{pretty_host} credentials."
+    say "Don't have an account yet? " +
+        "Create one at https://#{sign_in_host}/sign_up"
 
     email    =              ask "   Email: "
     password = ask_for_password "Password: "
     say "\n"
 
-    response = rubygems_api_request :get, "api/v1/api_key" do |request|
+    response = rubygems_api_request(:get, "api/v1/api_key",
+                                    sign_in_host) do |request|
       request.basic_auth email, password
     end
 

lib/rubygems/package.rb

@@ -473,6 +473,10 @@ EOM
       @security_policy
 
     true
+  rescue Gem::Security::Exception
+    @spec = nil
+    @files = []
+    raise
   rescue Errno::ENOENT => e
     raise Gem::Package::FormatError.new e.message
   rescue Gem::Package::TarInvalidError => e

lib/rubygems/specification.rb

@@ -904,7 +904,7 @@ class Gem::Specification
       raise Gem::Exception, "YAML data doesn't evaluate to gem specification"
     end
 
-    spec.instance_eval { @specification_version ||= NONEXISTENT_SPECIFICATION_VERSION }
+    spec.specification_version ||= NONEXISTENT_SPECIFICATION_VERSION
     spec.reset_nil_attributes_to_default
 
     spec

lib/rubygems/test_case.rb

@@ -143,8 +143,9 @@ class Gem::TestCase < MiniTest::Unit::TestCase
     @gemhome  = File.join @tempdir, 'gemhome'
     @userhome = File.join @tempdir, 'userhome'
 
-    @orig_ruby = if ruby = ENV['RUBY'] then
+    @orig_ruby = if ENV['RUBY'] then
-                   Gem.class_eval { ruby, @ruby = @ruby, ruby.dup }
+                   ruby = Gem.instance_variable_get :@ruby
+                   Gem.instance_variable_set :@ruby, ENV['RUBY']
                    ruby
                  end
 
@@ -264,7 +265,7 @@ class Gem::TestCase < MiniTest::Unit::TestCase
     ENV['GEM_PATH'] = @orig_gem_path
 
     _ = @orig_ruby
-    Gem.class_eval { @ruby = _ } if _
+    Gem.instance_variable_set :@ruby, @orig_ruby if @orig_ruby
 
     if @orig_ENV_HOME then
       ENV['HOME'] = @orig_ENV_HOME

test/rubygems/test_gem.rb

@@ -667,6 +667,25 @@ class TestGem < Gem::TestCase
     assert_equal %w[http://rubygems.org/], Gem.default_sources
   end
 
+  def test_self_detect_gemdeps
+    rubygems_gemdeps, ENV['RUBYGEMS_GEMDEPS'] = ENV['RUBYGEMS_GEMDEPS'], '-'
+
+    FileUtils.mkdir_p 'detect/a/b'
+    FileUtils.mkdir_p 'detect/a/Isolate'
+
+    FileUtils.touch 'detect/Isolate'
+
+    begin
+      Dir.chdir 'detect/a/b'
+
+      assert_empty Gem.detect_gemdeps
+    ensure
+      Dir.chdir @tempdir
+    end
+  ensure
+    ENV['RUBYGEMS_GEMDEPS'] = rubygems_gemdeps
+  end
+
   def test_self_dir
     assert_equal @gemhome, Gem.dir
   end
@@ -1457,7 +1476,7 @@ class TestGem < Gem::TestCase
     ENV['GEM_PATH'] = path
     ENV['RUBYGEMS_GEMDEPS'] = "-"
 
-    out = `#{Gem.ruby.untaint} -I #{LIB_PATH.untaint} -rubygems -e "p Gem.loaded_specs.values.map(&:full_name).sort"`
+    out = `#{Gem.ruby.dup.untaint} -I #{LIB_PATH.untaint} -rubygems -e "p Gem.loaded_specs.values.map(&:full_name).sort"`
 
     assert_equal '["a-1", "b-1", "c-1"]', out.strip
   end
@@ -1489,7 +1508,7 @@ class TestGem < Gem::TestCase
 
     Dir.mkdir "sub1"
     out = Dir.chdir "sub1" do
-      `#{Gem.ruby.untaint} -I #{LIB_PATH.untaint} -rubygems -e "p Gem.loaded_specs.values.map(&:full_name).sort"`
+      `#{Gem.ruby.dup.untaint} -I #{LIB_PATH.untaint} -rubygems -e "p Gem.loaded_specs.values.map(&:full_name).sort"`
     end
 
     Dir.rmdir "sub1"

test/rubygems/test_gem_commands_push_command.rb

@@ -46,6 +46,7 @@ class TestGemCommandsPushCommand < Gem::TestCase
 
   def send_battery
     use_ui @ui do
+      @cmd.instance_variable_set :@host, @host
       @cmd.send_gem(@path)
     end
 
@@ -133,7 +134,7 @@ class TestGemCommandsPushCommand < Gem::TestCase
   end
 
   def test_raises_error_with_no_arguments
-    def @cmd.sign_in; end
+    def @cmd.sign_in(*); end
     assert_raises Gem::CommandLineError do
       @cmd.execute
     end

test/rubygems/test_gem_config_file.rb

@@ -164,6 +164,36 @@ class TestGemConfigFile < Gem::TestCase
     assert_equal 2048, @cfg.bulk_threshold
   end
 
+  def test_check_credentials_permissions
+    @cfg.rubygems_api_key = 'x'
+
+    File.chmod 0644, @cfg.credentials_path
+
+    use_ui @ui do
+      assert_raises Gem::MockGemUi::TermError do
+        @cfg.load_api_keys
+      end
+    end
+
+    assert_empty @ui.output
+
+    expected = <<-EXPECTED
+ERROR:  Your gem push credentials file located at:
+
+\t#{@cfg.credentials_path}
+
+has file permissions of 0644 but 0600 is required.
+
+You should reset your credentials at:
+
+\thttps://rubygems.org/profile/edit
+
+if you believe they were disclosed to a third party.
+    EXPECTED
+
+    assert_equal expected, @ui.error
+  end
+
   def test_handle_arguments
     args = %w[--backtrace --bunch --of --args here]
 
@@ -215,6 +245,32 @@ class TestGemConfigFile < Gem::TestCase
     assert_equal true, @cfg.backtrace
   end
 
+  def test_load_api_keys
+    temp_cred = File.join Gem.user_home, '.gem', 'credentials'
+    FileUtils.mkdir File.dirname(temp_cred)
+    File.open temp_cred, 'w', 0600 do |fp|
+      fp.puts ":rubygems_api_key: 701229f217cdf23b1344c7b4b54ca97"
+      fp.puts ":other: a5fdbb6ba150cbb83aad2bb2fede64c"
+    end
+
+    util_config_file
+
+    assert_equal({:rubygems => '701229f217cdf23b1344c7b4b54ca97',
+                  :other => 'a5fdbb6ba150cbb83aad2bb2fede64c'}, @cfg.api_keys)
+  end
+
+  def test_load_api_keys_bad_permission
+    skip 'chmod not supported' if win_platform?
+
+    @cfg.rubygems_api_key = 'x'
+
+    File.chmod 0644, @cfg.credentials_path
+
+    assert_raises Gem::MockGemUi::TermError do
+      @cfg.load_api_keys
+    end
+  end
+
   def test_really_verbose
     assert_equal false, @cfg.really_verbose
 
@@ -227,6 +283,46 @@ class TestGemConfigFile < Gem::TestCase
     assert_equal true, @cfg.really_verbose
   end
 
+  def test_rubygems_api_key_equals
+    @cfg.rubygems_api_key = 'x'
+
+    assert_equal 'x', @cfg.rubygems_api_key
+
+    expected = {
+      :rubygems_api_key => 'x',
+    }
+
+    assert_equal expected, YAML.load_file(@cfg.credentials_path)
+
+    unless win_platform? then
+      stat = File.stat @cfg.credentials_path
+
+      assert_equal 0600, stat.mode & 0600
+    end
+  end
+
+  def test_rubygems_api_key_equals_bad_permission
+    skip 'chmod not supported' if win_platform?
+
+    @cfg.rubygems_api_key = 'x'
+
+    File.chmod 0644, @cfg.credentials_path
+
+    assert_raises Gem::MockGemUi::TermError do
+      @cfg.rubygems_api_key = 'y'
+    end
+
+    expected = {
+      :rubygems_api_key => 'x',
+    }
+
+    assert_equal expected, YAML.load_file(@cfg.credentials_path)
+
+    stat = File.stat @cfg.credentials_path
+
+    assert_equal 0644, stat.mode & 0644
+  end
+
   def test_write
     @cfg.backtrace = true
     @cfg.update_sources = false
@@ -287,40 +383,6 @@ class TestGemConfigFile < Gem::TestCase
     assert_equal %w[http://even-more-gems.example.com], Gem.sources
   end
 
-  def test_load_rubygems_api_key_from_credentials
-    temp_cred = File.join Gem.user_home, '.gem', 'credentials'
-    FileUtils.mkdir File.dirname(temp_cred)
-    File.open temp_cred, 'w' do |fp|
-      fp.puts ":rubygems_api_key: 701229f217cdf23b1344c7b4b54ca97"
-    end
-
-    util_config_file
-
-    assert_equal "701229f217cdf23b1344c7b4b54ca97", @cfg.rubygems_api_key
-  end
-
-  def test_load_api_keys_from_config
-    temp_cred = File.join Gem.user_home, '.gem', 'credentials'
-    FileUtils.mkdir File.dirname(temp_cred)
-    File.open temp_cred, 'w' do |fp|
-      fp.puts ":rubygems_api_key: 701229f217cdf23b1344c7b4b54ca97"
-      fp.puts ":other: a5fdbb6ba150cbb83aad2bb2fede64c"
-    end
-
-    util_config_file
-
-    assert_equal({:rubygems => '701229f217cdf23b1344c7b4b54ca97',
-                  :other => 'a5fdbb6ba150cbb83aad2bb2fede64c'}, @cfg.api_keys)
-  end
-
-  def test_save_credentials_file_with_strict_permissions
-    util_config_file
-    FileUtils.mkdir File.dirname(@cfg.credentials_path)
-    @cfg.rubygems_api_key = '701229f217cdf23b1344c7b4b54ca97'
-    mode = 0100600 & (~File.umask)
-    assert_equal mode, File.stat(@cfg.credentials_path).mode unless win_platform?
-  end
-
   def test_ignore_invalid_config_file
     File.open @temp_conf, 'w' do |fp|
       fp.puts "some-non-yaml-hash-string"

test/rubygems/test_gem_gemcutter_utilities.rb

@@ -77,9 +77,24 @@ class TestGemGemcutterUtilities < Gem::TestCase
 
   def test_sign_in_with_host
     api_key     = 'a5fdbb6ba150cbb83aad2bb2fede64cf040453903'
+
+    util_sign_in [api_key, 200, 'OK'], 'http://example.com', :param
+
+    assert_match "Enter your http://example.com credentials.",
+                 @sign_in_ui.output
+    assert @fetcher.last_request["authorization"]
+    assert_match %r{Signed in.}, @sign_in_ui.output
+
+    credentials = YAML.load_file Gem.configuration.credentials_path
+    assert_equal api_key, credentials[:rubygems_api_key]
+  end
+
+  def test_sign_in_with_host_ENV
+    api_key     = 'a5fdbb6ba150cbb83aad2bb2fede64cf040453903'
     util_sign_in [api_key, 200, 'OK'], 'http://example.com'
 
-    assert_match %r{Enter your RubyGems.org credentials.}, @sign_in_ui.output
+    assert_match "Enter your http://example.com credentials.",
+                 @sign_in_ui.output
     assert @fetcher.last_request["authorization"]
     assert_match %r{Signed in.}, @sign_in_ui.output
 
@@ -125,14 +140,14 @@ class TestGemGemcutterUtilities < Gem::TestCase
     assert_match %r{Access Denied.}, @sign_in_ui.output
   end
 
-  def util_sign_in response, host = nil
+  def util_sign_in response, host = nil, style = :ENV
     skip 'Always uses $stdin on windows' if Gem.win_platform?
 
     email    = 'you@example.com'
     password = 'secret'
 
     if host
-      ENV['RUBYGEMS_HOST'] = host
+      ENV['RUBYGEMS_HOST'] = host if style == :ENV
     else
       host = Gem.host
     end
@@ -144,7 +159,11 @@ class TestGemGemcutterUtilities < Gem::TestCase
     @sign_in_ui = Gem::MockGemUi.new "#{email}\n#{password}\n"
 
     use_ui @sign_in_ui do
-      @cmd.sign_in
+      if style == :param then
+        @cmd.sign_in host
+      else
+        @cmd.sign_in
+      end
     end
   end
 

test/rubygems/test_gem_package.rb

@@ -499,6 +499,9 @@ class TestGemPackage < Gem::Package::TarTestCase
 
     assert_equal 'unsigned gems are not allowed by the High Security policy',
                  e.message
+
+    refute package.instance_variable_get(:@spec), '@spec must not be loaded'
+    assert_empty package.instance_variable_get(:@files), '@files must empty'
   end
 
   def test_verify_truncate

test/rubygems/test_gem_specification.rb

@@ -118,6 +118,15 @@ end
     assert_equal @current_version, new_spec.specification_version
   end
 
+  def test_self_from_yaml
+    @a1.instance_variable_set :@specification_version, nil
+
+    spec = Gem::Specification.from_yaml @a1.to_yaml
+
+    assert_equal Gem::Specification::NONEXISTENT_SPECIFICATION_VERSION,
+                 spec.specification_version
+  end
+
   def test_self_from_yaml_syck_date_bug
     # This is equivalent to (and totally valid) psych 1.0 output and
     # causes parse errors on syck.