Update for Rubocop 1.0 and 1.1 (#494)

.gitignore

@@ -16,3 +16,4 @@ test-cfn-model.rb
 .idea
 .DS_Store
 tmp
+.rake_tasks

.rubocop.yml

@@ -26,7 +26,7 @@ Metrics/MethodLength:
   Max: 25
 
 Metrics/AbcSize:
-  Max: 21
+  Max: 22
 
 Style/StderrPuts:
   Enabled: false

Gemfile.lock

@@ -12,7 +12,7 @@ PATH
 GEM
   remote: https://rubygems.org/
   specs:
-    ast (2.4.0)
+    ast (2.4.1)
     aws-eventstream (1.1.0)
     aws-partitions (1.381.0)
     aws-sdk-core (3.109.1)
@@ -34,7 +34,6 @@ GEM
       psych (~> 3)
     diff-lcs (1.3)
     docile (1.3.2)
-    jaro_winkler (1.5.4)
     jmespath (1.4.0)
     kwalify (0.7.2)
     lightly (0.3.3)
@@ -45,12 +44,13 @@ GEM
     multi_json (1.15.0)
     netaddr (2.0.4)
     optimist (3.0.1)
-    parallel (1.19.1)
-    parser (2.7.1.0)
-      ast (~> 2.4.0)
+    parallel (1.19.2)
+    parser (2.7.2.0)
+      ast (~> 2.4.1)
     psych (3.2.0)
     rainbow (3.0.0)
     rake (13.0.1)
+    regexp_parser (1.8.2)
     rexml (3.2.4)
     rspec (3.9.0)
       rspec-core (~> 3.9.0)
@@ -65,14 +65,17 @@ GEM
       diff-lcs (>= 1.2.0, < 2.0)
       rspec-support (~> 3.9.0)
     rspec-support (3.9.2)
-    rubocop (0.81.0)
-      jaro_winkler (~> 1.5.1)
+    rubocop (1.1.0)
       parallel (~> 1.10)
-      parser (>= 2.7.0.1)
+      parser (>= 2.7.1.5)
       rainbow (>= 2.2.2, < 4.0)
+      regexp_parser (>= 1.8)
       rexml
+      rubocop-ast (>= 1.0.1)
       ruby-progressbar (~> 1.7)
       unicode-display_width (>= 1.4.0, < 2.0)
+    rubocop-ast (1.1.0)
+      parser (>= 2.7.1.5)
     ruby-progressbar (1.10.1)
     simplecov (0.18.5)
       docile (~> 1.1)

Rakefile

@@ -6,8 +6,7 @@
 def docker_command
   docker_cmd = 'sudo docker'
   local_cmd = 'docker'
-  cmd_prefix = File.file?('/.dockerenv') ? docker_cmd : local_cmd
-  cmd_prefix
+  File.file?('/.dockerenv') ? docker_cmd : local_cmd
 end
 
 # Returns a docker run prefix based on the environment that you are running in, determined by the file '/.dockerenv'
@@ -20,8 +19,7 @@ def docker_run_prefix
     '--workdir /usr/src/app cfn-nag-dev:latest'
   local_env = "#{docker_command} run --tty --rm --mount source=#{Dir.pwd},target=/usr/src/app,type=bind " \
     '--workdir /usr/src/app cfn-nag-dev:latest'
-  prefix = File.file?('/.dockerenv') ? docker_env : local_env
-  prefix
+  File.file?('/.dockerenv') ? docker_env : local_env
 end
 
 def ensure_local_dev_image

lib/cfn-nag/cfn_nag.rb

@@ -135,12 +135,11 @@ def filter_violations_by_blacklist_and_profile(violations)
     )
 
     # this must come after - blacklist should always win
-    violations = filter_violations_by_blacklist(
+    filter_violations_by_blacklist(
       blacklist_definition: @config.blacklist_definition,
       rule_definitions: @config.custom_rule_loader.rule_definitions,
       violations: violations
     )
-    violations
   rescue StandardError => blacklist_or_profile_parse_error
     violations << fatal_violation(blacklist_or_profile_parse_error.to_s)
     violations

lib/cfn-nag/cfn_nag_config.rb

@@ -29,12 +29,6 @@ def initialize(profile_definition: nil,
   end
   # rubocop:enable Metrics/ParameterLists
 
-  attr_reader :rule_arguments
-  attr_reader :rule_directory
-  attr_reader :custom_rule_loader
-  attr_reader :profile_definition
-  attr_reader :blacklist_definition
-  attr_reader :fail_on_warnings
-  attr_reader :rule_repositories
-  attr_reader :ignore_fatal
+  attr_reader :rule_arguments, :rule_directory, :custom_rule_loader, :profile_definition, :blacklist_definition, \
+              :fail_on_warnings, :rule_repositories, :ignore_fatal
 end

lib/cfn-nag/custom_rule_loader.rb

@@ -93,7 +93,7 @@ def filter_rule_classes(cfn_model, violations, rules_registry)
       rescue ScriptError, StandardError => rule_error
         raise rule_error unless @isolate_custom_rule_exceptions
 
-        STDERR.puts rule_error
+        $stderr.puts rule_error
       end
     end
   end

lib/cfn-nag/custom_rules/ElasticLoadBalancerV2AccessLoggingRule.rb

@@ -29,10 +29,9 @@ def audit_impl(cfn_model)
   private
 
   def access_logging_is_false?(load_balancer)
-    false_access_log_attribute = load_balancer.loadBalancerAttributes.find do |load_balancer_attribute|
+    load_balancer.loadBalancerAttributes.find do |load_balancer_attribute|
       load_balancer_attribute['Key'] == 'access_logs.s3.enabled' && not_truthy?(load_balancer_attribute['Value'])
     end
-    false_access_log_attribute
   end
 
   def missing_access_logs?(load_balancer)

lib/cfn-nag/custom_rules/ElasticsearchDomainEncryptionAtRestOptionsRule.rb

@@ -2,7 +2,6 @@
 
 require 'cfn-nag/util/truthy'
 require 'cfn-nag/violation'
-require 'cfn-nag/util/truthy'
 require_relative 'base'
 
 class ElasticsearchDomainEncryptionAtRestOptionsRule < BaseRule

lib/cfn-nag/custom_rules/IamRolePassRoleWildcardResourceRule.rb

@@ -5,7 +5,7 @@
 require 'cfn-nag/util/wildcard_patterns'
 
 class IamRolePassRoleWildcardResourceRule < BaseRule
-  IAM_ACTION_PATTERNS = wildcard_patterns('PassRole').map! { |x| 'iam:' + x } + ['*']
+  IAM_ACTION_PATTERNS = wildcard_patterns('PassRole').map! { |x| "iam:#{x}" } + ['*']
 
   def rule_text
     'IAM role should not allow * resource with PassRole action on its permissions policy'

lib/cfn-nag/custom_rules/RDSInstanceDeletionProtectionRule.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 require_relative 'base'
-require 'cfn-nag/util/truthy.rb'
+require 'cfn-nag/util/truthy'
 require 'cfn-nag/violation'
 
 class RDSInstanceDeletionProtectionRule < BaseRule

lib/cfn-nag/custom_rules/SPCMRule.rb

@@ -6,6 +6,7 @@
 
 class SPCMRule < BaseRule
   attr_accessor :spcm_threshold
+
   DEFAULT_THRESHOLD = 25
 
   def rule_text

lib/cfn-nag/custom_rules/SecurityGroupEgressOpenToWorldRule.rb

@@ -25,16 +25,22 @@ def rule_id
   def audit_impl(cfn_model)
     violating_security_groups = cfn_model.security_groups.select do |security_group|
       violating_egresses = security_group.egresses.select do |egress|
-        ip4_open?(egress) || ip6_open?(egress)
+        violating_egress(egress)
       end
 
       !violating_egresses.empty?
     end
 
     violating_egresses = cfn_model.standalone_egress.select do |standalone_egress|
-      ip4_open?(standalone_egress) || ip6_open?(standalone_egress)
+      violating_egress(standalone_egress)
     end
 
     violating_security_groups.map(&:logical_resource_id) + violating_egresses.map(&:logical_resource_id)
   end
+
+  private
+
+  def violating_egress(egress)
+    ip4_open?(egress) || ip6_open?(egress)
+  end
 end

lib/cfn-nag/custom_rules/SecurityGroupIngressCidrNon32Rule.rb

@@ -25,16 +25,22 @@ def rule_id
   def audit_impl(cfn_model)
     violating_security_groups = cfn_model.security_groups.select do |security_group|
       violating_ingresses = security_group.ingresses.select do |ingress|
-        ip4_cidr_range?(ingress) || ip6_cidr_range?(ingress)
+        violating_ingress(ingress)
       end
 
       !violating_ingresses.empty?
     end
 
     violating_ingresses = cfn_model.standalone_ingress.select do |standalone_ingress|
-      ip4_cidr_range?(standalone_ingress) || ip6_cidr_range?(standalone_ingress)
+      violating_ingress(standalone_ingress)
     end
 
     violating_security_groups.map(&:logical_resource_id) + violating_ingresses.map(&:logical_resource_id)
   end
+
+  private
+
+  def violating_ingress(ingress)
+    ip4_cidr_range?(ingress) || ip6_cidr_range?(ingress)
+  end
 end

lib/cfn-nag/custom_rules/SecurityGroupIngressOpenToWorldRule.rb

@@ -26,16 +26,22 @@ def rule_id
   def audit_impl(cfn_model)
     violating_security_groups = cfn_model.security_groups.select do |security_group|
       violating_ingresses = security_group.ingresses.select do |ingress|
-        ip4_open?(ingress) || ip6_open?(ingress)
+        violating_ingress(ingress)
       end
 
       !violating_ingresses.empty?
     end
 
     violating_ingresses = cfn_model.standalone_ingress.select do |standalone_ingress|
-      ip4_open?(standalone_ingress) || ip6_open?(standalone_ingress)
+      violating_ingress(standalone_ingress)
     end
 
     violating_security_groups.map(&:logical_resource_id) + violating_ingresses.map(&:logical_resource_id)
   end
+
+  private
+
+  def violating_ingress(ingress)
+    ip4_open?(ingress) || ip6_open?(ingress)
+  end
 end

lib/cfn-nag/custom_rules/SecurityGroupRuleDescriptionRule.rb

@@ -3,7 +3,7 @@
 require 'cfn-nag/violation'
 require_relative 'base'
 require 'cfn-nag/ip_addr'
-require 'cfn-nag/util/blank.rb'
+require 'cfn-nag/util/blank'
 
 class SecurityGroupRuleDescriptionRule < BaseRule
   def rule_text

lib/cfn-nag/custom_rules/boolean_base_rule.rb

@@ -2,7 +2,7 @@
 
 require 'cfn-nag/violation'
 require_relative 'base'
-require 'cfn-nag/util/truthy.rb'
+require 'cfn-nag/util/truthy'
 
 ##
 # Derive from this rule to ensure that a resource

lib/cfn-nag/custom_rules/passrole_base_rule.rb

@@ -5,7 +5,7 @@
 require 'cfn-nag/util/wildcard_patterns'
 
 class PassRoleBaseRule < BaseRule
-  IAM_ACTION_PATTERNS = wildcard_patterns('PassRole').map { |pattern| 'iam:' + pattern } + ['*']
+  IAM_ACTION_PATTERNS = wildcard_patterns('PassRole').map { |pattern| "iam:#{pattern}" } + ['*']
 
   def policy_type
     raise 'must implement in subclass'

lib/cfn-nag/iam_complexity_metric/condition_metric.rb

@@ -41,9 +41,10 @@ def all_values(conditions)
     result = []
     conditions.each do |_, expression|
       expression.each do |_, value|
-        if value.is_a? String
+        case value
+        when String
           result << value
-        elsif value.is_a? Array
+        when Array
           result += value
         end
       end

lib/cfn-nag/ip_addr.rb

@@ -47,9 +47,10 @@ def ip6_cidr_range?(ingress)
   # anything with it
   #
   def normalize_cidr_ip6(ingress)
-    if ingress.cidrIpv6.is_a?(Symbol)
+    case ingress.cidrIpv6
+    when Symbol
       ":#{ingress.cidrIpv6}"
-    elsif ingress.cidrIpv6.is_a?(String)
+    when String
       ingress.cidrIpv6
     end
   end

lib/cfn-nag/metadata.rb

@@ -13,7 +13,7 @@ def validate_cfn_nag_metadata(cfn_model)
       logical_resource_id = mangled_metadata.first
       mangled_rules = mangled_metadata[1]
 
-      STDERR.puts "#{logical_resource_id} has missing cfn_nag suppression rule id: #{mangled_rules}"
+      $stderr.puts "#{logical_resource_id} has missing cfn_nag suppression rule id: #{mangled_rules}"
     end
   end
 
@@ -46,7 +46,7 @@ def suppress_resource?(rules_to_suppress, rule_id, logical_resource_id, print_su
     end
     if found_suppression_rule && print_suppression
       message = "Suppressing #{rule_id} on #{logical_resource_id} for reason: #{found_suppression_rule['reason']}"
-      STDERR.puts message
+      $stderr.puts message
     end
     !found_suppression_rule.nil?
   end

lib/cfn-nag/result_view/stdout_results.rb

@@ -27,7 +27,7 @@ def print_warnings(violations)
   def render(results)
     results.each do |result|
       60.times { print '-' }
-      puts "\n" + result[:filename]
+      puts "\n#{result[:filename]}"
       60.times { print '-' }
 
       violations = result[:file_results][:violations]
@@ -45,6 +45,6 @@ def message
   end
 
   def indent_multiline_string_with_prefix(prefix, multiline_string)
-    prefix + ' ' + multiline_string.gsub(/\n/, "\n#{prefix} ")
+    "#{prefix} #{multiline_string.gsub(/\n/, "\n#{prefix} ")}"
   end
 end

lib/cfn-nag/rule_repos/gem_based_rule_repo.rb

@@ -53,7 +53,7 @@ def load_gem_entrypoint(rule_gem_name)
     require rule_gem_name
     true
   rescue LoadError
-    STDERR.puts "Could not require #{rule_gem_name} - does the rule gem have a top level entry point?"
+    $stderr.puts "Could not require #{rule_gem_name} - does the rule gem have a top level entry point?"
     false
   end
 

lib/cfn-nag/rule_repository_loader.rb

@@ -33,7 +33,7 @@ def rule_repository(rule_repository_definition_str)
     end
 
     repo_class = class_from_name(rule_repository_definition['repo_class_name'])
-    if rule_repository_definition['repo_arguments']&.is_a?(Hash)
+    if rule_repository_definition['repo_arguments'].is_a?(Hash)
       repo_class.new(**to_sym_keys(rule_repository_definition['repo_arguments']))
     else
       repo_class.new

lib/cfn-nag/util/enforce_reference_parameter.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-require 'cfn-nag/util/truthy.rb'
+require 'cfn-nag/util/truthy'
 
 # Returns false if the provided key_to_check is a no-echo parameter without a
 # default value, or pseudo parameter reference to 'AWS::NoValue'; true otherwise.

lib/cfn-nag/util/enforce_string_or_dynamic_reference.rb

@@ -9,17 +9,16 @@ def insecure_string_or_dynamic_reference?(_cfn_model, key_to_check)
 
   # Check if string starts with a Dynamic Reference pointing to SecretsManager
   # or SSM Secure
+  # &&
+  # Verify that the secure string ends properly with the double curly braces
   if key_to_check.start_with?(
     '{{resolve:secretsmanager:',
     '{{resolve:ssm-secure:'
-  )
-    # Verify that the secure string ends properly with the double curly braces
-    if key_to_check.end_with? '}}'
-      return false
-    end
+  ) && key_to_check.end_with?('}}')
+    return false
   end
 
-  # Retrun true if key_to_check is a string and is not calling a secured
+  # Return true if key_to_check is a string and is not calling a secured
   # dynamic reference pattern (Secrets Manager or SSM-Secure)
   true
 end