diff --git a/app/models/pusher.rb b/app/models/pusher.rb index 0f0dab75908..a409d420a0e 100644 --- a/app/models/pusher.rb +++ b/app/models/pusher.rb @@ -26,6 +26,7 @@ def process authorize && verify_gem_scope && verify_mfa_requirement && + verify_dependencies_resolvable && validate && save end @@ -46,6 +47,20 @@ def verify_mfa_requirement notify("Rubygem requires owners to enable MFA. You must enable MFA before pushing new version.", 403) end + def verify_dependencies_resolvable + return true if spec.dependencies.empty? + + dependency_names = spec.dependencies.map(&:name) + existing_gems = Rubygem.where(name: dependency_names).pluck(:name) + missing_gems = dependency_names - existing_gems + + if missing_gems.any? + return notify("There was a problem saving your gem: \nThe following dependencies don't exist: #{missing_gems.join(', ')}", 422) + end + + true + end + def validate unless validate_signature_exists? return notify("There was a problem saving your gem: \nYou have added cert_chain in gemspec but signature was empty", 403) diff --git a/test/integration/push_test.rb b/test/integration/push_test.rb index ae4471a3852..23d93a32cc4 100644 --- a/test/integration/push_test.rb +++ b/test/integration/push_test.rb @@ -236,13 +236,7 @@ class PushTest < ActionDispatch::IntegrationTest push_gem "sandworm-1.0.0.gem" - assert_response :success - - get rubygem_path("sandworm") - - assert_response :success - assert page.has_content?("mauddib") - assert page.has_content?("> 1") + assert_response :unprocessable_entity end test "pushing a gem with a new platform for the same version" do diff --git a/test/models/pusher_test.rb b/test/models/pusher_test.rb index 3e2e409af67..26279f78385 100644 --- a/test/models/pusher_test.rb +++ b/test/models/pusher_test.rb @@ -47,6 +47,7 @@ class PusherTest < ActiveSupport::TestCase @cutter.stubs(:authorize).returns true @cutter.stubs(:verify_mfa_requirement).returns true @cutter.stubs(:verify_gem_scope).returns true + @cutter.stubs(:verify_dependencies_resolvable).returns true @cutter.stubs(:validate).returns true @cutter.stubs(:verify_sigstore).returns true @cutter.stubs(:sign_sigstore).returns true @@ -100,12 +101,26 @@ class PusherTest < ActiveSupport::TestCase @cutter.process end - should "not attempt to validate if mfa check failed" do + should "not attempt to verify dependencies resolve if mfa check failed" do @cutter.stubs(:pull_spec).returns true @cutter.stubs(:find).returns true @cutter.stubs(:authorize).returns true @cutter.stubs(:verify_gem_scope).returns true @cutter.stubs(:verify_mfa_requirement).returns false + @cutter.stubs(:verify_dependencies_resolvable).never + @cutter.stubs(:validate).never + @cutter.stubs(:save).never + + @cutter.process + end + + should "not attempt to validate if dependencies don't resolve" do + @cutter.stubs(:pull_spec).returns true + @cutter.stubs(:find).returns true + @cutter.stubs(:authorize).returns true + @cutter.stubs(:verify_gem_scope).returns true + @cutter.stubs(:verify_mfa_requirement).returns true + @cutter.stubs(:verify_dependencies_resolvable).returns false @cutter.stubs(:validate).never @cutter.stubs(:save).never @@ -118,6 +133,7 @@ class PusherTest < ActiveSupport::TestCase @cutter.stubs(:authorize).returns true @cutter.stubs(:verify_gem_scope).returns true @cutter.stubs(:verify_mfa_requirement).returns true + @cutter.stubs(:verify_dependencies_resolvable).returns true @cutter.stubs(:validate).returns false @cutter.stubs(:save).never @@ -202,7 +218,7 @@ class PusherTest < ActiveSupport::TestCase .instance_variable_set(:@requirements, [["!!!", "0"]]) end) @cutter.stubs(:validate_signature_exists?).returns(true) - + @cutter.stubs(:verify_dependencies_resolvable).returns true @cutter.process assert_match(/requirements must be list of valid requirements/, @cutter.message) @@ -214,6 +230,7 @@ class PusherTest < ActiveSupport::TestCase s.add_runtime_dependency "\nother" end) @cutter.stubs(:validate_signature_exists?).returns(true) + @cutter.stubs(:verify_dependencies_resolvable).returns true @cutter.process @@ -221,6 +238,17 @@ class PusherTest < ActiveSupport::TestCase assert_equal 403, @cutter.code end + should "not be able to save a gem if the dependencies are not resolvable" do + @cutter.stubs(:spec).returns(new_gemspec("gem-with-nonexistent-dep", "1.0.0", "Summary", "ruby") do |s| + s.add_runtime_dependency "non-existent-gem", "~> 1.0" + end) + + @cutter.process + + assert_match(/There was a problem saving your gem: \nThe following dependencies don't exist: non-existent-gem/, @cutter.message) + assert_equal 422, @cutter.code + end + should "not be able to save a gem if the metadata has incorrect values" do @cutter.stubs(:spec).returns(new_gemspec("bad-metadata", "1.0.0", "Summary", "ruby") do |s| s.metadata["foo"] = []