Add element_types for parameters and resources (#578)

* BAM: Add element_types for parameters and resources

* BAM: Add rubocop config

.rubocop.yml

@@ -4,6 +4,7 @@ AllCops:
     - 'spec/**/*'
     - 'vendor/**/*'
   NewCops: enable
+  SuggestExtensions: false
 
 Style/IfUnlessModifier:
   Enabled: false
@@ -50,3 +51,6 @@ Style/HashTransformKeys:
 
 Style/HashTransformValues:
   Enabled: true
+
+Metrics/ParameterLists:
+  Max: 6

Gemfile.lock

@@ -3,7 +3,7 @@ PATH
   specs:
     cfn-nag (0.0.0)
       aws-sdk-s3 (~> 1.76)
-      cfn-model (= 0.6.5)
+      cfn-model (= 0.6.6)
       lightly (~> 0.3.2)
       logging (~> 2.2.2)
       netaddr (~> 2.0.4)
@@ -30,7 +30,7 @@ GEM
       aws-sigv4 (~> 1.4)
     aws-sigv4 (1.4.0)
       aws-eventstream (~> 1, >= 1.0.2)
-    cfn-model (0.6.5)
+    cfn-model (0.6.6)
       kwalify (= 0.7.2)
       psych (~> 3)
     diff-lcs (1.4.4)

cfn-nag.gemspec

@@ -27,7 +27,7 @@ Gem::Specification.new do |s|
   # versus what we used to run tests in cfn-nag before publishing cfn-nag
   # they are coupled and we are doing a good bit of experimenting in cfn-model
   # i might consider collapsing them again....
-  s.add_runtime_dependency('cfn-model', '0.6.5')
+  s.add_runtime_dependency('cfn-model', '0.6.6')
   s.add_runtime_dependency('logging', '~> 2.2.2')
   s.add_runtime_dependency('netaddr', '~> 2.0.4')
   s.add_runtime_dependency('optimist', '~> 3.0.0')

lib/cfn-nag/cfn_nag.rb

@@ -95,7 +95,7 @@ def audit(cloudformation_string:, parameter_values_string: nil, condition_values
       )
 
       violations = filter_violations_by_deny_list_and_profile(violations)
-      violations = mark_line_numbers(violations, cfn_model)
+      violations = mark_line_numbers_and_element_types(violations, cfn_model)
     rescue RuleRepoException, Psych::SyntaxError, ParserError => fatal_error
       violations << Violation.fatal_violation(fatal_error.to_s)
     rescue JSON::ParserError => json_parameters_error
@@ -118,10 +118,11 @@ def render_results(aggregate_results:,
 
   private
 
-  def mark_line_numbers(violations, cfn_model)
+  def mark_line_numbers_and_element_types(violations, cfn_model)
     violations.each do |violation|
       violation.logical_resource_ids.each do |logical_resource_id|
         violation.line_numbers << cfn_model.line_numbers[logical_resource_id]
+        violation.element_types << cfn_model.element_types[logical_resource_id]
       end
     end
 

lib/cfn-nag/custom_rules/base.rb

@@ -22,12 +22,13 @@ def audit(cfn_model)
     violation(logical_resource_ids)
   end
 
-  def violation(logical_resource_ids, line_numbers = [])
+  def violation(logical_resource_ids, line_numbers = [], element_types = [])
     Violation.new(id: rule_id,
                   name: self.class.name,
                   type: rule_type,
                   message: rule_text,
                   logical_resource_ids: logical_resource_ids,
-                  line_numbers: line_numbers)
+                  line_numbers: line_numbers,
+                  element_types: element_types)
   end
 end

lib/cfn-nag/result_view/colored_stdout_results.rb

@@ -10,15 +10,16 @@ def message(message_type:,
               color:,
               message:,
               logical_resource_ids: nil,
-              line_numbers: [])
+              line_numbers: [],
+              element_types: [])
 
     logical_resource_ids = nil if logical_resource_ids == []
 
     60.times { print '-' }
     puts
     puts colorize(color, "| #{message_type.upcase}")
     puts colorize(color, '|')
-    puts colorize(color, "| Resources: #{logical_resource_ids}") unless logical_resource_ids.nil?
+    puts colorize(color, "| #{element_type(element_types)}: #{logical_resource_ids}") unless logical_resource_ids.nil?
     puts colorize(color, "| Line Numbers: #{line_numbers}") unless line_numbers.empty?
     puts colorize(color, '|') unless line_numbers.empty? && logical_resource_ids.nil?
     puts colorize(color, "| #{message}")
@@ -38,4 +39,12 @@ def color_code(color_symbol)
   def colorize(color_symbol, str)
     "\e[#{color_code(color_symbol)}m#{str}\e[0m"
   end
+
+  def element_type(element_types)
+    if element_types == [] || element_types.first.nil?
+      'Element'
+    elsif !element_types.first.nil?
+      element_types.first.capitalize
+    end
+  end
 end

lib/cfn-nag/result_view/simple_stdout_results.rb

@@ -11,18 +11,27 @@ def message(message_type:,
               message:,
               color:,
               logical_resource_ids: nil,
-              line_numbers: [])
+              line_numbers: [],
+              element_types: [])
 
     logical_resource_ids = nil if logical_resource_ids == []
 
     60.times { print '-' }
     puts
     puts "| #{message_type.upcase}"
     puts '|'
-    puts "| Resources: #{logical_resource_ids}" unless logical_resource_ids.nil?
+    puts "| #{element_type(element_types)}: #{logical_resource_ids}" unless logical_resource_ids.nil?
     puts "| Line Numbers: #{line_numbers}" unless line_numbers.empty?
     puts '|' unless line_numbers.empty? && logical_resource_ids.nil?
     puts "| #{message}"
   end
   # rubocop:enable Lint/UnusedMethodArgument
+
+  def element_type(element_types)
+    if element_types == [] || element_types.first.nil?
+      'Element'
+    elsif !element_types.first.nil?
+      element_types.first.capitalize
+    end
+  end
 end

lib/cfn-nag/result_view/stdout_results.rb

@@ -12,7 +12,8 @@ def message_violations(violations)
               color: color,
               message: violation.message,
               logical_resource_ids: violation.logical_resource_ids,
-              line_numbers: violation.line_numbers
+              line_numbers: violation.line_numbers,
+              element_types: violation.element_types
     end
   end
 

lib/cfn-nag/violation.rb

@@ -4,22 +4,24 @@
 
 # Rule definition for violations
 class Violation < RuleDefinition
-  attr_reader :logical_resource_ids, :line_numbers
+  attr_reader :logical_resource_ids, :line_numbers, :element_types
 
   # rubocop:disable Metrics/ParameterLists
   def initialize(id:,
                  name:,
                  type:,
                  message:,
                  logical_resource_ids: [],
-                 line_numbers: [])
+                 line_numbers: [],
+                 element_types: [])
     super id: id,
           name: name,
           type: type,
           message: message
 
     @logical_resource_ids = logical_resource_ids
     @line_numbers = line_numbers
+    @element_types = element_types
   end
   # rubocop:enable Metrics/ParameterLists
 
@@ -30,7 +32,8 @@ def to_s
   def to_h
     super.to_h.merge(
       logical_resource_ids: @logical_resource_ids,
-      line_numbers: @line_numbers
+      line_numbers: @line_numbers,
+      element_types: @element_types
     )
   end
 

spec/cfn_nag_integration/cfn_nag_cloudfront_distribution_spec.rb

@@ -18,9 +18,9 @@
           file_results: {
             failure_count: 0,
             violations: [
-              CloudFrontDistributionAccessLoggingRule.new.violation(%w[rDistribution2], [46]),
-              CloudfrontMinimumProtocolVersionRule.new.violation(["rDistribution1", "rDistribution2"], [4,46]),
-              MissingBucketPolicyRule.new.violation(%w[S3Bucket], [81])
+              CloudFrontDistributionAccessLoggingRule.new.violation(%w[rDistribution2], [46], ["resource"]),
+              CloudfrontMinimumProtocolVersionRule.new.violation(["rDistribution1", "rDistribution2"], [4,46], ["resource", "resource"]),
+              MissingBucketPolicyRule.new.violation(%w[S3Bucket], [81], ["resource"])
             ]
           }
         }

spec/cfn_nag_integration/cfn_nag_ec2_instance_spec.rb

@@ -19,7 +19,7 @@
           file_results: {
             failure_count: 0,
             violations: [
-              CloudFormationAuthenticationRule.new.violation(%w[EC2I4LBA1], [11])
+              CloudFormationAuthenticationRule.new.violation(%w[EC2I4LBA1], [11], ["resource"])
             ]
           }
         }

spec/cfn_nag_integration/cfn_nag_ec2_volume_spec.rb

@@ -18,8 +18,8 @@
           file_results: {
             failure_count: 2,
             violations: [
-              EbsVolumeEncryptionKeyRule.new.violation(%w[NewVolume1 NewVolume2], [4, 13]),
-              EbsVolumeHasSseRule.new.violation(%w[NewVolume1 NewVolume2], [4, 13])
+              EbsVolumeEncryptionKeyRule.new.violation(%w[NewVolume1 NewVolume2], [4, 13], ["resource", "resource"]),
+              EbsVolumeHasSseRule.new.violation(%w[NewVolume1 NewVolume2], [4, 13], ["resource", "resource"])
             ]
           }
         }
@@ -42,7 +42,7 @@
           file_results: {
             failure_count: 0,
             violations: [
-              EbsVolumeEncryptionKeyRule.new.violation(%w[NewVolume], [4])
+              EbsVolumeEncryptionKeyRule.new.violation(%w[NewVolume], [4], ["resource"])
             ]
           }
         }

spec/cfn_nag_integration/cfn_nag_elasticloadbalancing_loadbalancer_spec.rb

@@ -18,7 +18,7 @@
           file_results: {
             failure_count: 0,
             violations: [
-              ElasticLoadBalancerAccessLoggingRule.new.violation(%w[elb1 elb2], [4, 19])
+              ElasticLoadBalancerAccessLoggingRule.new.violation(%w[elb1 elb2], [4, 19], ["resource", "resource"])
             ]
           }
         }

spec/cfn_nag_integration/cfn_nag_iam_role_spec.rb

@@ -13,7 +13,7 @@
       template_name = 'yaml/iam_role/embedded_ref.yml'
 
       expected_violations = [
-        IamRoleWildcardResourceOnPermissionsPolicyRule.new.violation(%w[HelperRole], [7])
+        IamRoleWildcardResourceOnPermissionsPolicyRule.new.violation(%w[HelperRole], [7], ["resource"])
       ]
 
       actual_violations = @cfn_nag.audit(

spec/cfn_nag_integration/cfn_nag_iam_user_spec.rb

@@ -19,7 +19,7 @@
             # only increment this when Violation::FAILING (vs WARNING)
             failure_count: 1,
             violations: [
-              UserMissingGroupRule.new.violation(%w[myuser2], [4])
+              UserMissingGroupRule.new.violation(%w[myuser2], [4], ["resource"])
             ]
           }
         }

spec/cfn_nag_integration/cfn_nag_lambda_permission_spec.rb

@@ -18,11 +18,11 @@
           file_results: {
             failure_count: 3,
             violations: [
-              IamRolePassRoleWildcardResourceRule.new.violation(%w[LambdaExecutionRole], [50]),
-              IamRoleWildcardActionOnPermissionsPolicyRule.new.violation(%w[LambdaExecutionRole], [50]),
-              IamRoleWildcardResourceOnPermissionsPolicyRule.new.violation(%w[LambdaExecutionRole], [50]),
-              LambdaFunctionInsideVPCRule.new.violation(%w[AppendItemToListFunction], [4]),
-              LambdaPermissionWildcardPrincipalRule.new.violation(%w[lambdaPermission], [24])
+              IamRolePassRoleWildcardResourceRule.new.violation(%w[LambdaExecutionRole], [50], ["resource"]),
+              IamRoleWildcardActionOnPermissionsPolicyRule.new.violation(%w[LambdaExecutionRole], [50], ["resource"]),
+              IamRoleWildcardResourceOnPermissionsPolicyRule.new.violation(%w[LambdaExecutionRole], [50], ["resource"]),
+              LambdaFunctionInsideVPCRule.new.violation(%w[AppendItemToListFunction], [4], ["resource"]),
+              LambdaPermissionWildcardPrincipalRule.new.violation(%w[lambdaPermission], [24], ["resource"])
             ]
           }
         }

spec/cfn_nag_integration/cfn_nag_rds_instance_spec.rb

@@ -18,7 +18,7 @@
           file_results: {
             failure_count: 1,
             violations: [
-              RDSInstancePubliclyAccessibleRule.new.violation(%w[PublicDB], [4])
+              RDSInstancePubliclyAccessibleRule.new.violation(%w[PublicDB], [4], ["resource"])
             ]
           }
         }
@@ -39,8 +39,8 @@
           file_results: {
             failure_count: 2,
             violations: [
-              RDSDBInstanceMasterUserPasswordRule.new.violation(%w[BadDb2], [11]),
-              RDSInstancePubliclyAccessibleRule.new.violation(%w[BadDb2], [11])
+              RDSDBInstanceMasterUserPasswordRule.new.violation(%w[BadDb2], [11], ["resource"]),
+              RDSInstancePubliclyAccessibleRule.new.violation(%w[BadDb2], [11], ["resource"])
             ]
           }
         }
@@ -62,8 +62,8 @@
           file_results: {
             failure_count: 4,
             violations: [
-              RDSDBInstanceMasterUserPasswordRule.new.violation(%w[BadDb1 BadDb2], [14, 30]),
-              RDSDBInstanceMasterUsernameRule.new.violation(%w[BadDb1 BadDb2], [14, 30])
+              RDSDBInstanceMasterUserPasswordRule.new.violation(%w[BadDb1 BadDb2], [14, 30], ["resource", "resource"]),
+              RDSDBInstanceMasterUsernameRule.new.violation(%w[BadDb1 BadDb2], [14, 30], ["resource", "resource"])
             ]
           }
         }

spec/cfn_nag_integration/cfn_nag_s3_bucket_policy_spec.rb

@@ -18,8 +18,8 @@
           file_results: {
             failure_count: 3,
             violations: [
-              S3BucketPolicyWildcardActionRule.new.violation(%w[S3BucketPolicy S3BucketPolicy2], [61, 86]),
-              S3BucketPolicyWildcardPrincipalRule.new.violation(%w[S3BucketPolicy2], [86])
+              S3BucketPolicyWildcardActionRule.new.violation(%w[S3BucketPolicy S3BucketPolicy2], [61, 86], ["resource", "resource"]),
+              S3BucketPolicyWildcardPrincipalRule.new.violation(%w[S3BucketPolicy2], [86], ["resource"])
             ]
           }
         }

spec/cfn_nag_integration/cfn_nag_s3_bucket_spec.rb

@@ -18,9 +18,9 @@
           file_results: {
             failure_count: 1,
             violations: [
-              MissingBucketPolicyRule.new.violation(%w[S3BucketRead S3BucketReadWrite], [4, 24]),
-              S3BucketPublicReadAclRule.new.violation(%w[S3BucketRead], [4]),
-              S3BucketPublicReadWriteAclRule.new.violation(%w[S3BucketReadWrite], [24])
+              MissingBucketPolicyRule.new.violation(%w[S3BucketRead S3BucketReadWrite], [4, 24], ["resource", "resource"]),
+              S3BucketPublicReadAclRule.new.violation(%w[S3BucketRead], [4], ["resource"]),
+              S3BucketPublicReadWriteAclRule.new.violation(%w[S3BucketReadWrite], [24], ["resource"])
             ]
           }
         }

spec/cfn_nag_integration/cfn_nag_security_group_spec.rb

@@ -45,7 +45,7 @@
           file_results: {
             failure_count: 1,
             violations: [
-              SecurityGroupMissingEgressRule.new.violation(%w[sg], [4])
+              SecurityGroupMissingEgressRule.new.violation(%w[sg], [4], ["resource"])
             ]
           }
         }
@@ -67,10 +67,10 @@
           file_results: {
             failure_count: 2,
             violations: [
-              SecurityGroupIngressCidrNon32Rule.new.violation(%w[sg2], [18]),
-              SecurityGroupIngressOpenToWorldRule.new.violation(%w[sg2], [18]),
-              SecurityGroupIngressPortRangeRule.new.violation(%w[sg sg2], [4, 18]),
-              SecurityGroupMissingEgressRule.new.violation(%w[sg sg2], [4, 18])
+              SecurityGroupIngressCidrNon32Rule.new.violation(%w[sg2], [18], ["resource"]),
+              SecurityGroupIngressOpenToWorldRule.new.violation(%w[sg2], [18], ["resource"]),
+              SecurityGroupIngressPortRangeRule.new.violation(%w[sg sg2], [4, 18], ["resource", "resource"]),
+              SecurityGroupMissingEgressRule.new.violation(%w[sg sg2], [4, 18], ["resource", "resource"])
             ]
           }
         }
@@ -116,7 +116,7 @@
           file_results: {
             failure_count: 0,
             violations: [
-              SecurityGroupIngressCidrNon32Rule.new.violation(%w[sg], [9])
+              SecurityGroupIngressCidrNon32Rule.new.violation(%w[sg], [9], ["resource"])
             ]
           }
         }
@@ -140,7 +140,7 @@
           file_results: {
             failure_count: 0,
             violations: [
-              SecurityGroupIngressCidrNon32Rule.new.violation(%w[sg sg2], [9, 30])
+              SecurityGroupIngressCidrNon32Rule.new.violation(%w[sg sg2], [9, 30], ["resource", "resource"])
             ]
           }
         }

spec/cfn_nag_integration/cfn_nag_serverless_transform_spec.rb

@@ -21,9 +21,9 @@
           file_results: {
             failure_count: 0,
             violations: [
-              LambdaFunctionCloudWatchLogsRule.new.violation(%w[SomeFunction2], [34]),
-              LambdaFunctionInsideVPCRule.new.violation(["SomeFunction", "SomeFunction2"], [20, 34]),
-              LambdaFunctionReservedConcurrentExecutionsRule.new.violation(["SomeFunction","SomeFunction2"], [20, 34])
+              LambdaFunctionCloudWatchLogsRule.new.violation(%w[SomeFunction2], [34], ["resource"]),
+              LambdaFunctionInsideVPCRule.new.violation(["SomeFunction", "SomeFunction2"], [20, 34], ["resource", "resource"]),
+              LambdaFunctionReservedConcurrentExecutionsRule.new.violation(["SomeFunction","SomeFunction2"], [20, 34], ["resource", "resource"])
             ]
           }
         }

spec/cfn_nag_integration/cfn_nag_sns_policy_spec.rb

@@ -20,7 +20,7 @@
             # only increment this when Violation::FAILING (vs WARNING)
             failure_count: 4,
             violations: [
-              SnsTopicPolicyWildcardPrincipalRule.new.violation(%w[mysnspolicy0 mysnspolicy1 mysnspolicy2 mysnspolicy3], [11, 29, 55, 85])
+              SnsTopicPolicyWildcardPrincipalRule.new.violation(%w[mysnspolicy0 mysnspolicy1 mysnspolicy2 mysnspolicy3], [11, 29, 55, 85], ["resource", "resource", "resource", "resource"])
             ]
           }
         }