Skip to content

Commit

Permalink
Add new Gemspec/RequireMFA cop.
Browse files Browse the repository at this point in the history
  • Loading branch information
dvandersluis authored and bbatsov committed Nov 15, 2021
1 parent 12e0993 commit 5780761
Show file tree
Hide file tree
Showing 5 changed files with 328 additions and 0 deletions.
1 change: 1 addition & 0 deletions changelog/new_add_new_gemspecrequiremfa_cop.md
@@ -0,0 +1 @@
* [#10243](https://github.com/rubocop/rubocop/pull/10243): Add new `Gemspec/RequireMFA` cop. ([@dvandersluis][])
9 changes: 9 additions & 0 deletions config/default.yml
Expand Up @@ -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: '<<next>>'
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
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Expand Up @@ -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'

Expand Down
146 changes: 146 additions & 0 deletions 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
171 changes: 171 additions & 0 deletions 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

0 comments on commit 5780761

Please sign in to comment.