ruby-changes:27003
From: drbrain <ko1@a...>
Date: Tue, 5 Feb 2013 11:43:01 +0900 (JST)
Subject: [ruby-changes:27003] drbrain:r39055 (trunk): * lib/rubygems/commands/push_command.rb: Fixed credential download for
drbrain 2013-02-05 11:37:35 +0900 (Tue, 05 Feb 2013) New Revision: 39055 http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=rev&revision=39055 Log: * 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. Modified files: trunk/ChangeLog trunk/lib/rubygems/commands/push_command.rb trunk/lib/rubygems/config_file.rb trunk/lib/rubygems/ext/builder.rb trunk/lib/rubygems/gemcutter_utilities.rb trunk/lib/rubygems/package.rb trunk/lib/rubygems/specification.rb trunk/lib/rubygems/test_case.rb trunk/lib/rubygems.rb trunk/test/rubygems/test_gem.rb trunk/test/rubygems/test_gem_commands_push_command.rb trunk/test/rubygems/test_gem_config_file.rb trunk/test/rubygems/test_gem_gemcutter_utilities.rb trunk/test/rubygems/test_gem_package.rb trunk/test/rubygems/test_gem_specification.rb Index: ChangeLog =================================================================== --- ChangeLog (revision 39054) +++ ChangeLog (revision 39055) @@ -1,3 +1,33 @@ https://github.com/ruby/ruby/blob/trunk/ChangeLog#L1 +Tue Feb 5 11:35:35 2013 Eric Hodel <drbrain@s...> + + * 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@z...> * doc/security.rdoc: Wrap security guide at 80 columns Index: lib/rubygems/gemcutter_utilities.rb =================================================================== --- lib/rubygems/gemcutter_utilities.rb (revision 39054) +++ lib/rubygems/gemcutter_utilities.rb (revision 39055) @@ -27,17 +27,25 @@ module Gem::GemcutterUtilities https://github.com/ruby/ruby/blob/trunk/lib/rubygems/gemcutter_utilities.rb#L27 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." - say "Don't have an account yet? Create one at http://rubygems.org/sign_up" + pretty_host = if Gem::DEFAULT_HOST == sign_in_host then + '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 Index: lib/rubygems/ext/builder.rb =================================================================== --- lib/rubygems/ext/builder.rb (revision 39054) +++ lib/rubygems/ext/builder.rb (revision 39055) @@ -43,12 +43,18 @@ class Gem::Ext::Builder https://github.com/ruby/ruby/blob/trunk/lib/rubygems/ext/builder.rb#L43 def self.run(command, results, command_name = nil) verbose = Gem.configuration.really_verbose - if verbose - puts(command) - system(command) - else - results << command - results << `#{command} #{redirector}` + begin + # TODO use Process.spawn when ruby 1.8 support is dropped. + rubygems_gemdeps, ENV['RUBYGEMS_GEMDEPS'] = ENV['RUBYGEMS_GEMDEPS'], nil + if verbose + puts(command) + system(command) + else + results << command + results << `#{command} #{redirector}` + end + ensure + ENV['RUBYGEMS_GEMDEPS'] = rubygems_gemdeps end unless $?.success? then Index: lib/rubygems/config_file.rb =================================================================== --- lib/rubygems/config_file.rb (revision 39054) +++ lib/rubygems/config_file.rb (revision 39055) @@ -33,6 +33,8 @@ https://github.com/ruby/ruby/blob/trunk/lib/rubygems/config_file.rb#L33 class Gem::ConfigFile + include Gem::UserInteraction + DEFAULT_BACKTRACE = false DEFAULT_BULK_THRESHOLD = 1000 DEFAULT_VERBOSITY = true @@ -224,6 +226,34 @@ class Gem::ConfigFile https://github.com/ruby/ruby/blob/trunk/lib/rubygems/config_file.rb#L226 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 def credentials_path @@ -231,6 +261,8 @@ class Gem::ConfigFile https://github.com/ruby/ruby/blob/trunk/lib/rubygems/config_file.rb#L261 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 https://github.com/ruby/ruby/blob/trunk/lib/rubygems/config_file.rb#L275 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 Index: lib/rubygems/specification.rb =================================================================== --- lib/rubygems/specification.rb (revision 39054) +++ lib/rubygems/specification.rb (revision 39055) @@ -904,7 +904,7 @@ class Gem::Specification https://github.com/ruby/ruby/blob/trunk/lib/rubygems/specification.rb#L904 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 Index: lib/rubygems/package.rb =================================================================== --- lib/rubygems/package.rb (revision 39054) +++ lib/rubygems/package.rb (revision 39055) @@ -473,6 +473,10 @@ EOM https://github.com/ruby/ruby/blob/trunk/lib/rubygems/package.rb#L473 @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 Index: lib/rubygems/commands/push_command.rb =================================================================== --- lib/rubygems/commands/push_command.rb (revision 39054) +++ lib/rubygems/commands/push_command.rb (revision 39055) @@ -24,16 +24,19 @@ class Gem::Commands::PushCommand < Gem:: https://github.com/ruby/ruby/blob/trunk/lib/rubygems/commands/push_command.rb#L24 add_proxy_option add_key_option - add_option( - '--host HOST', - 'Push to another gemcutter-compatible host' - ) do |value, options| + add_option('--host HOST', + '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:: https://github.com/ruby/ruby/blob/trunk/lib/rubygems/commands/push_command.rb#L47 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 + unless @host then 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 Index: lib/rubygems/test_case.rb =================================================================== --- lib/rubygems/test_case.rb (revision 39054) +++ lib/rubygems/test_case.rb (revision 39055) @@ -143,8 +143,9 @@ class Gem::TestCase < MiniTest::Unit::Te https://github.com/ruby/ruby/blob/trunk/lib/rubygems/test_case.rb#L143 @gemhome = File.join @tempdir, 'gemhome' @userhome = File.join @tempdir, 'userhome' - @orig_ruby = if ruby = ENV['RUBY'] then - Gem.class_eval { ruby, @ruby = @ruby, ruby.dup } + @orig_ruby = if ENV['RUBY'] then + ruby = Gem.instance_variable_get :@ruby + Gem.instance_variable_set :@ruby, ENV['RUBY'] ruby end @@ -264,7 +265,7 @@ class Gem::TestCase < MiniTest::Unit::Te https://github.com/ruby/ruby/blob/trunk/lib/rubygems/test_case.rb#L265 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 Index: lib/rubygems.rb =================================================================== --- lib/rubygems.rb (revision 39054) +++ lib/rubygems.rb (revision 39055) @@ -207,7 +207,7 @@ module Gem https://github.com/ruby/ruby/blob/trunk/lib/rubygems.rb#L207 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 https://github.com/ruby/ruby/blob/trunk/lib/rubygems.rb#L226 end end - return unless File.exists? path + path.untaint + + return unless File.file? path rs = Gem::RequestSet.new rs.load_gemdeps path @@ -370,29 +372,6 @@ module Gem https://github.com/ruby/ruby/blob/trunk/lib/rubygems.rb#L372 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 # problem, then we will silently continue. Index: test/rubygems/test_gem_package.rb =================================================================== --- test/rubygems/test_gem_package.rb (revision 39054) +++ test/rubygems/test_gem_package.rb (revision 39055) @@ -499,6 +499,9 @@ class TestGemPackage < Gem::Package::Tar https://github.com/ruby/ruby/blob/trunk/test/rubygems/test_gem_package.rb#L499 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 Index: test/rubygems/test_gem_gemcutter_utilities.rb =================================================================== --- test/rubygems/test_gem_gemcutter_utilities.rb (revision 39054) +++ test/rubygems/test_gem_gemcutter_utilities.rb (revision 39055) @@ -77,9 +77,24 @@ class TestGemGemcutterUtilities < Gem::T https://github.com/ruby/ruby/blob/trunk/test/rubygems/test_gem_gemcutter_utilities.rb#L77 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::T https://github.com/ruby/ruby/blob/trunk/test/rubygems/test_gem_gemcutter_utilities.rb#L140 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@e...' 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::T https://github.com/ruby/ruby/blob/trunk/test/rubygems/test_gem_gemcutter_utilities.rb#L159 @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 Index: test/rubygems/test_gem.rb =================================================================== --- test/rubygems/test_gem.rb (revision 39054) +++ test/rubygems/test_gem.rb (revision 39055) @@ -667,6 +667,25 @@ class TestGem < Gem::TestCase https://github.com/ruby/ruby/blob/trunk/test/rubygems/test_gem.rb#L667 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 https://github.com/ruby/ruby/blob/trunk/test/rubygems/test_gem.rb#L1476 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 https://github.com/ruby/ruby/blob/trunk/test/rubygems/test_gem.rb#L1508 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" Index: test/rubygems/test_gem_commands_push_command.rb =================================================================== --- test/rubygems/test_gem_commands_push_command.rb (revision 39054) +++ test/rubygems/test_gem_commands_push_command.rb (revision 39055) @@ -46,6 +46,7 @@ class TestGemCommandsPushCommand < Gem:: https://github.com/ruby/ruby/blob/trunk/test/rubygems/test_gem_commands_push_command.rb#L46 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:: https://github.com/ruby/ruby/blob/trunk/test/rubygems/test_gem_commands_push_command.rb#L134 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 Index: test/rubygems/test_gem_specification.rb =================================================================== --- test/rubygems/test_gem_specification.rb (revision 39054) +++ test/rubygems/test_gem_specification.rb (revision 39055) @@ -118,6 +118,15 @@ end https://github.com/ruby/ruby/blob/trunk/test/rubygems/test_gem_specification.rb#L118 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. Index: test/rubygems/test_gem_config_file.rb =================================================================== --- test/rubygems/test_gem_config_file.rb (revision 39054) +++ test/rubygems/test_gem_config_file.rb (revision 39055) @@ -164,6 +164,36 @@ class TestGemConfigFile < Gem::TestCase https://github.com/ruby/ruby/blob/trunk/test/rubygems/test_gem_config_file.rb#L164 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 https://github.com/ruby/ruby/blob/trunk/test/rubygems/test_gem_config_file.rb#L245 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 tem (... truncated) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/