diff --git a/changelog/new_add_new_gemspecrequiremfa_cop.md b/changelog/new_add_new_gemspecrequiremfa_cop.md new file mode 100644 index 00000000000..d5dbd222d52 --- /dev/null +++ b/changelog/new_add_new_gemspecrequiremfa_cop.md @@ -0,0 +1 @@ +* [#10243](https://github.com/rubocop/rubocop/pull/10243): Add new `Gemspec/RequireMFA` cop. ([@dvandersluis][]) diff --git a/config/default.yml b/config/default.yml index a748dd3cd70..eb5e1e2119a 100644 --- a/config/default.yml +++ b/config/default.yml @@ -259,6 +259,15 @@ Gemspec/OrderedDependencies: Include: - '**/*.gemspec' +Gemspec/RequireMFA: + Description: 'Checks that the gemspec has metadata to require MFA from RubyGems.' + Enabled: pending + VersionAdded: '<>' + Reference: + - https://guides.rubygems.org/mfa-requirement-opt-in/ + Include: + - '**/*.gemspec' + Gemspec/RequiredRubyVersion: Description: 'Checks that `required_ruby_version` of gemspec is specified and equal to `TargetRubyVersion` of .rubocop.yml.' Enabled: true diff --git a/lib/rubocop.rb b/lib/rubocop.rb index d353cca8478..1cd9d5110a8 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -161,6 +161,7 @@ require_relative 'rubocop/cop/gemspec/date_assignment' require_relative 'rubocop/cop/gemspec/duplicated_assignment' require_relative 'rubocop/cop/gemspec/ordered_dependencies' +require_relative 'rubocop/cop/gemspec/require_mfa' require_relative 'rubocop/cop/gemspec/required_ruby_version' require_relative 'rubocop/cop/gemspec/ruby_version_globals_usage' diff --git a/lib/rubocop/cop/gemspec/require_mfa.rb b/lib/rubocop/cop/gemspec/require_mfa.rb new file mode 100644 index 00000000000..994135300e1 --- /dev/null +++ b/lib/rubocop/cop/gemspec/require_mfa.rb @@ -0,0 +1,146 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Gemspec + # Requires a gemspec to have `rubygems_mfa_required` metadata set. + # + # This setting tells RubyGems that MFA is required for accounts to + # be able perform any of these privileged operations: + # + # * gem push + # * gem yank + # * gem owner --add/remove + # * adding or removing owners using gem ownership page + # + # This helps make your gem more secure, as users can be more + # confident that gem updates were pushed by maintainers. + # + # @example + # + # # bad + # Gem::Specification.new do |spec| + # # no `rubygems_mfa_required` metadata specified + # end + # + # # good + # Gem::Specification.new do |spec| + # spec.metadata = { + # 'rubygems_mfa_required' => 'true' + # } + # end + # + # # good + # Gem::Specification.new do |spec| + # spec.metadata['rubygems_mfa_required'] = 'true' + # end + # + # # bad + # Gem::Specification.new do |spec| + # spec.metadata = { + # 'rubygems_mfa_required' => 'false' + # } + # end + # + # # good + # Gem::Specification.new do |spec| + # spec.metadata = { + # 'rubygems_mfa_required' => 'true' + # } + # end + # + # # bad + # Gem::Specification.new do |spec| + # spec.metadata['rubygems_mfa_required'] = 'false' + # end + # + # # good + # Gem::Specification.new do |spec| + # spec.metadata['rubygems_mfa_required'] = 'true' + # end + # + class RequireMFA < Base + include GemspecHelp + extend AutoCorrector + + MSG = "`metadata['rubygems_mfa_required']` must be set to `'true'`." + + # @!method metadata(node) + def_node_matcher :metadata, <<~PATTERN + `{ + (send _ :metadata= $_) + (send (send _ :metadata) :[]= (str "rubygems_mfa_required") $_) + } + PATTERN + + # @!method rubygems_mfa_required(node) + def_node_search :rubygems_mfa_required, <<~PATTERN + (pair (str "rubygems_mfa_required") $_) + PATTERN + + # @!method true_string?(node) + def_node_matcher :true_string?, <<~PATTERN + (str "true") + PATTERN + + def on_block(node) # rubocop:disable Metrics/MethodLength + gem_specification(node) do |block_var| + metadata_value = metadata(node) + mfa_value = mfa_value(metadata_value) + + if mfa_value + unless true_string?(mfa_value) + add_offense(mfa_value) do |corrector| + change_value(corrector, mfa_value) + end + end + else + add_offense(node) do |corrector| + autocorrect(corrector, node, block_var, metadata_value) + end + end + end + end + + private + + def mfa_value(metadata_value) + return unless metadata_value + return metadata_value if metadata_value.str_type? + + rubygems_mfa_required(metadata_value).first + end + + def autocorrect(corrector, node, block_var, metadata) + if metadata + return unless metadata.hash_type? + + correct_metadata(corrector, metadata) + else + correct_missing_metadata(corrector, node, block_var) + end + end + + def correct_metadata(corrector, metadata) + if metadata.pairs.any? + corrector.insert_after(metadata.pairs.last, ",\n'rubygems_mfa_required' => 'true'") + else + corrector.insert_before(metadata.loc.end, "'rubygems_mfa_required' => 'true'") + end + end + + def correct_missing_metadata(corrector, node, block_var) + corrector.insert_before(node.loc.end, <<~RUBY) + #{block_var}.metadata = { + 'rubygems_mfa_required' => 'true' + } + RUBY + end + + def change_value(corrector, value) + corrector.replace(value, "'true'") + end + end + end + end +end diff --git a/spec/rubocop/cop/gemspec/require_mfa_spec.rb b/spec/rubocop/cop/gemspec/require_mfa_spec.rb new file mode 100644 index 00000000000..05171002a40 --- /dev/null +++ b/spec/rubocop/cop/gemspec/require_mfa_spec.rb @@ -0,0 +1,171 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Gemspec::RequireMFA, :config do + context 'when the gemspec is blank' do + it 'does not register an offense' do + expect_no_offenses('', 'my.gemspec') + end + end + + context 'when the specification is blank' do + it 'registers an offense and corrects' do + expect_offense(<<~RUBY, 'my.gemspec') + Gem::Specification.new do |spec| + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `metadata['rubygems_mfa_required']` must be set to `'true'`. + end + RUBY + + expect_correction(<<~RUBY) + Gem::Specification.new do |spec| + spec.metadata = { + 'rubygems_mfa_required' => 'true' + } + end + RUBY + end + end + + context 'when the specification has a metadata hash but no rubygems_mfa_required key' do + it 'registers an offense and corrects' do + expect_offense(<<~RUBY, 'my.gemspec') + Gem::Specification.new do |spec| + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `metadata['rubygems_mfa_required']` must be set to `'true'`. + spec.metadata = { + } + end + RUBY + + expect_correction(<<~RUBY) + Gem::Specification.new do |spec| + spec.metadata = { + 'rubygems_mfa_required' => 'true'} + end + RUBY + end + end + + context 'when the specification has an non-hash metadata' do + it 'registers an offense but does not correct' do + expect_offense(<<~RUBY, 'my.gemspec') + Gem::Specification.new do |spec| + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `metadata['rubygems_mfa_required']` must be set to `'true'`. + spec.metadata = Metadata.new + end + RUBY + end + end + + context 'when there are other metadata keys' do + context 'and `rubygems_mfa_required` is included' do + it 'does not register an offense' do + expect_no_offenses(<<~RUBY, 'my.gemspec') + Gem::Specification.new do |spec| + spec.metadata = { + 'foo' => 'bar', + 'rubygems_mfa_required' => 'true', + 'baz' => 'quux' + } + end + RUBY + end + end + + context 'and `rubygems_mfa_required` is not included' do + it 'registers an offense and corrects' do + expect_offense(<<~RUBY, 'my.gemspec') + Gem::Specification.new do |spec| + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `metadata['rubygems_mfa_required']` must be set to `'true'`. + spec.metadata = { + 'foo' => 'bar', + 'baz' => 'quux' + } + end + RUBY + + expect_correction(<<~RUBY) + Gem::Specification.new do |spec| + spec.metadata = { + 'foo' => 'bar', + 'baz' => 'quux', + 'rubygems_mfa_required' => 'true' + } + end + RUBY + end + end + end + + context 'when metadata is set by key assignment' do + context 'and `rubygems_mfa_required` is included' do + it 'does not register an offense' do + expect_no_offenses(<<~RUBY, 'my.gemspec') + Gem::Specification.new do |spec| + spec.metadata['foo'] = 'bar' + spec.metadata['rubygems_mfa_required'] = 'true' + end + RUBY + end + end + + context 'and `rubygems_mfa_required` is not included' do + it 'registers an offense' do + expect_offense(<<~RUBY, 'my.gemspec') + Gem::Specification.new do |spec| + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `metadata['rubygems_mfa_required']` must be set to `'true'`. + spec.metadata['foo'] = 'bar' + end + RUBY + end + end + end + + context 'with rubygems_mfa_required: true' do + it 'does not register an offense' do + expect_no_offenses(<<~RUBY, 'my.gemspec') + Gem::Specification.new do |spec| + spec.metadata = { + 'rubygems_mfa_required' => 'true' + } + end + RUBY + end + end + + context 'with rubygems_mfa_required: false' do + it 'registers an offense and corrects' do + expect_offense(<<~RUBY, 'my.gemspec') + Gem::Specification.new do |spec| + spec.metadata = { + 'rubygems_mfa_required' => 'false' + ^^^^^^^ `metadata['rubygems_mfa_required']` must be set to `'true'`. + } + end + RUBY + + expect_correction(<<~RUBY) + Gem::Specification.new do |spec| + spec.metadata = { + 'rubygems_mfa_required' => 'true' + } + end + RUBY + end + end + + context 'with rubygems_mfa_required: false by key access' do + it 'registers an offense and corrects' do + expect_offense(<<~RUBY, 'my.gemspec') + Gem::Specification.new do |spec| + spec.metadata['rubygems_mfa_required'] = 'false' + ^^^^^^^ `metadata['rubygems_mfa_required']` must be set to `'true'`. + end + RUBY + + expect_correction(<<~RUBY) + Gem::Specification.new do |spec| + spec.metadata['rubygems_mfa_required'] = 'true' + end + RUBY + end + end +end