From be9415a4a1ee4892a6869158c03551cb078bd0a7 Mon Sep 17 00:00:00 2001
From: Kevin Formsma <kevin.formsma@gmail.com>
Date: Mon, 12 Apr 2021 11:04:58 -0400
Subject: [PATCH] Use full path when loading file rules (#525)

Resolves #520
---
 .../rule_repos/file_based_rule_repo.rb        |   2 +-
 spec/rules_repos/file_base_rule_repo_spec.rb  | 107 ++++++++++++------
 2 files changed, 71 insertions(+), 38 deletions(-)

diff --git a/lib/cfn-nag/rule_repos/file_based_rule_repo.rb b/lib/cfn-nag/rule_repos/file_based_rule_repo.rb
index 52bbecc8..1f704668 100644
--- a/lib/cfn-nag/rule_repos/file_based_rule_repo.rb
+++ b/lib/cfn-nag/rule_repos/file_based_rule_repo.rb
@@ -52,7 +52,7 @@ def discover_rule_classes(rule_directory)
 
     rule_filenames = discover_rule_filenames(rule_directory)
     rule_filenames.each do |rule_filename|
-      require(rule_filename)
+      require(File.absolute_path(rule_filename))
       rule_classname = File.basename(rule_filename, '.rb')
 
       rule_classes << Object.const_get(rule_classname)
diff --git a/spec/rules_repos/file_base_rule_repo_spec.rb b/spec/rules_repos/file_base_rule_repo_spec.rb
index 25e1e942..38d28e02 100644
--- a/spec/rules_repos/file_base_rule_repo_spec.rb
+++ b/spec/rules_repos/file_base_rule_repo_spec.rb
@@ -6,52 +6,85 @@
 describe FileBasedRuleRepo do
   describe '#discover_rules' do
     context 'external rule directory' do
-      before(:each) do
-        fake_rule = <<RULE
-require 'cfn-nag/custom_rules/base'
-require 'cfn-nag/violation'
-
-class FakeRule < BaseRule
-  def rule_text
-    'this is fake rule text'
-  end
 
-  def rule_type
-    Violation::WARNING
-  end
+      let(:valid_rule_text) do
+        <<~RULE
+          require 'cfn-nag/custom_rules/base'
+          require 'cfn-nag/violation'
+          class ValidCustomRule < BaseRule
+            def rule_text
+              'this is fake rule text'
+            end
 
-  def rule_id
-    'W9933'
-  end
+            def rule_type
+              Violation::WARNING
+            end
 
-  def audit_impl(cfn_model)
-    %w(hardwired1 hardwired2)
-  end
-end
-RULE
-        @custom_rule_directory = Dir.mktmpdir(%w[custom_rule loader])
-        File.open(File.join(@custom_rule_directory,
-                            'FakeRule.rb'),
-                  'w+') { |file| file.write fake_rule }
-        @file_base_rule_repo = FileBasedRuleRepo.new(@custom_rule_directory)
+            def rule_id
+              'W9933'
+            end
+
+            def audit_impl(cfn_model)
+              %w(hardwired1 hardwired2)
+            end
+          end
+        RULE
       end
 
-      after(:each) do
-        FileUtils.rm_rf @custom_rule_directory
+      it 'includes external rule definition from absolute directories' do
+        Dir.mktmpdir(%w[custom_rule loader]) do |custom_rule_directory|
+          # Write out a valid rule
+          File.write(File.join(custom_rule_directory, 'ValidCustomRule.rb'), valid_rule_text)
+          # Write out a invalid rule
+          File.write(File.join(custom_rule_directory, 'InvalidRuleNotMatching.rb'), 'fake_rule')
+
+          core_rules_registry = FileBasedRuleRepo.new(nil).discover_rules
+          actual_rule_registry = FileBasedRuleRepo.new(custom_rule_directory).discover_rules
+
+          # Expect one additional rule loaded
+          expect(actual_rule_registry.rules.size).to eq(core_rules_registry.rules.size+1)
+
+          # Validate that the rule was loaded by id
+          expected_rule_definition = RuleDefinition.new id: 'W9933',
+                                                        message: 'this is fake rule text',
+                                                        type: RuleDefinition::WARNING
+
+          actual_rule_definition = actual_rule_registry.by_id 'W9933'
+          expect(actual_rule_definition).to eq expected_rule_definition
+
+          # Validate that the rule class was mapped correctly
+          expected_rule_classes = 'ValidCustomRule'
+          actual_rule_classes = actual_rule_registry.rule_classes.map { |rule_class| rule_class.name }
+          expect(actual_rule_classes.include?(expected_rule_classes)).to be true
+        end
       end
 
-      it 'includes external rule definition' do
-        actual_rule_registry = @file_base_rule_repo.discover_rules
-        expected_rule_definition = RuleDefinition.new id: 'W9933',
-                                                      message: 'this is fake rule text',
-                                                      type: RuleDefinition::WARNING
+      it 'includes external rule definition from relative' do
+        Dir.mktmpdir(%w[custom_rule loader], Dir.getwd) do |custom_rule_directory|
+          # Write out a valid rule
+          File.write(File.join(custom_rule_directory, 'ValidCustomRule.rb'), valid_rule_text)
+          # Write out a invalid rule
+          File.write(File.join(custom_rule_directory, 'InvalidRuleNotMatching.rb'), 'fake_rule')
+
+          core_rules_registry = FileBasedRuleRepo.new(nil).discover_rules
+          actual_rule_registry = FileBasedRuleRepo.new(File.basename(custom_rule_directory)).discover_rules
+
+          # Expect one additional rule loaded
+          expect(actual_rule_registry.rules.size).to eq(core_rules_registry.rules.size+1)
+
+          # Validate that the rule was loaded by id
+          expected_rule_definition = RuleDefinition.new id: 'W9933',
+                                                        message: 'this is fake rule text',
+                                                        type: RuleDefinition::WARNING
 
-        actual_rule_definition = actual_rule_registry.by_id 'W9933'
-        expect(actual_rule_definition).to eq expected_rule_definition
+          actual_rule_definition = actual_rule_registry.by_id 'W9933'
+          expect(actual_rule_definition).to eq expected_rule_definition
 
-        expected_rule_classes = 'FakeRule'
-        actual_rule_classes = actual_rule_registry.rule_classes.map { |rule_class| rule_class.name }
-        expect(actual_rule_classes.include?(expected_rule_classes)).to be true
+          # Validate that the rule class was mapped correctly
+          expected_rule_classes = 'ValidCustomRule'
+          actual_rule_classes = actual_rule_registry.rule_classes.map { |rule_class| rule_class.name }
+          expect(actual_rule_classes.include?(expected_rule_classes)).to be true
+        end
       end
     end
   end