Merge pull request #60 from github/general-improvements

General improvements

Author: GrantBirki

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    hooks-ruby (0.3.1)
+    hooks-ruby (0.3.2)
       dry-schema (~> 1.14, >= 1.14.1)
       grape (~> 2.3)
       puma (~> 6.6)

lib/hooks/core/plugin_loader.rb

@@ -205,7 +205,11 @@ def load_custom_auth_plugin(file_path, auth_plugin_dir)
           require file_path
 
           # Get the class and validate it
-          auth_plugin_class = Object.const_get("Hooks::Plugins::Auth::#{class_name}")
+          auth_plugin_class = begin
+            Hooks::Plugins::Auth.const_get(class_name, false) # false = don't inherit from ancestors
+          rescue NameError
+            raise StandardError, "Auth plugin class not found in Hooks::Plugins::Auth namespace: #{class_name}"
+          end
           unless auth_plugin_class < Hooks::Plugins::Auth::Base
             raise StandardError, "Auth plugin class must inherit from Hooks::Plugins::Auth::Base: #{class_name}"
           end
@@ -239,8 +243,13 @@ def load_custom_handler_plugin(file_path, handler_plugin_dir)
           # Load the file
           require file_path
 
-          # Get the class and validate it
-          handler_class = Object.const_get(class_name)
+          # Get the class and validate it - use safe constant lookup
+          handler_class = begin
+            # Check if the constant exists in the global namespace for handlers
+            Object.const_get(class_name, false) # false = don't inherit from ancestors
+          rescue NameError
+            raise StandardError, "Handler class not found: #{class_name}"
+          end
           unless handler_class < Hooks::Plugins::Handlers::Base
             raise StandardError, "Handler class must inherit from Hooks::Plugins::Handlers::Base: #{class_name}"
           end
@@ -274,8 +283,12 @@ def load_custom_lifecycle_plugin(file_path, lifecycle_plugin_dir)
           # Load the file
           require file_path
 
-          # Get the class and validate it
-          lifecycle_class = Object.const_get(class_name)
+          # Get the class and validate it - use safe constant lookup
+          lifecycle_class = begin
+            Object.const_get(class_name, false) # false = don't inherit from ancestors
+          rescue NameError
+            raise StandardError, "Lifecycle plugin class not found: #{class_name}"
+          end
           unless lifecycle_class < Hooks::Plugins::Lifecycle
             raise StandardError, "Lifecycle plugin class must inherit from Hooks::Plugins::Lifecycle: #{class_name}"
           end
@@ -309,8 +322,12 @@ def load_custom_instrument_plugin(file_path, instruments_plugin_dir)
           # Load the file
           require file_path
 
-          # Get the class and validate it
-          instrument_class = Object.const_get(class_name)
+          # Get the class and validate it - use safe constant lookup
+          instrument_class = begin
+            Object.const_get(class_name, false) # false = don't inherit from ancestors
+          rescue NameError
+            raise StandardError, "Instrument plugin class not found: #{class_name}"
+          end
 
           # Determine instrument type based on inheritance
           if instrument_class < Hooks::Plugins::Instruments::StatsBase

lib/hooks/plugins/auth/shared_secret.rb

@@ -68,23 +68,21 @@ def self.valid?(payload:, headers:, config:)
           secret_header = validator_config[:header]
 
           # Find the secret header with case-insensitive matching
-          raw_secret = find_header_value(headers, secret_header)
+          provided_secret = find_header_value(headers, secret_header)
 
-          if raw_secret.nil? || raw_secret.empty?
+          if provided_secret.nil? || provided_secret.empty?
             log.warn("Auth::SharedSecret validation failed: Missing or empty secret header '#{secret_header}'")
             return false
           end
 
           # Validate secret format using shared validation
-          unless valid_header_value?(raw_secret, "Secret")
+          unless valid_header_value?(provided_secret, "Secret")
             log.warn("Auth::SharedSecret validation failed: Invalid secret format")
             return false
           end
 
-          stripped_secret = raw_secret.strip
-
           # Use secure comparison to prevent timing attacks
-          result = Rack::Utils.secure_compare(secret, stripped_secret)
+          result = Rack::Utils.secure_compare(secret, provided_secret)
           if result
             log.debug("Auth::SharedSecret validation successful for header '#{secret_header}'")
           else

lib/hooks/version.rb

@@ -4,5 +4,5 @@
 module Hooks
   # Current version of the Hooks webhook framework
   # @return [String] The version string following semantic versioning
-  VERSION = "0.3.1".freeze
+  VERSION = "0.3.2".freeze
 end

spec/unit/lib/hooks/core/plugin_loader_spec.rb

@@ -240,6 +240,28 @@ def self.valid?(payload:, headers:, config:)
       end
     end
 
+    it "raises error when auth plugin class is not found after loading" do
+      temp_auth_dir = File.join(temp_dir, "auth_missing_class")
+      FileUtils.mkdir_p(temp_auth_dir)
+
+      # Create plugin file that doesn't define the expected class
+      missing_file = File.join(temp_auth_dir, "missing_auth.rb")
+      File.write(missing_file, <<~RUBY)
+        # This file doesn't define MissingAuth class
+        module Hooks
+          module Plugins
+            module Auth
+              # Nothing here
+            end
+          end
+        end
+      RUBY
+
+      expect {
+        described_class.send(:load_custom_auth_plugin, missing_file, temp_auth_dir)
+      }.to raise_error(StandardError, /Auth plugin class not found in Hooks::Plugins::Auth namespace: MissingAuth/)
+    end
+
     describe "handler plugin loading failures" do
       it "raises error when handler plugin file fails to load" do
         temp_handler_dir = File.join(temp_dir, "handler_failures")
@@ -298,6 +320,23 @@ def call(payload:, headers:, env:, config:)
           described_class.send(:load_custom_handler_plugin, wrong_file, temp_handler_dir)
         }.to raise_error(StandardError, /Handler class must inherit from Hooks::Plugins::Handlers::Base/)
       end
+
+      it "raises error when handler plugin class is not found after loading" do
+        temp_handler_dir = File.join(temp_dir, "handler_missing_class")
+        FileUtils.mkdir_p(temp_handler_dir)
+
+        # Create plugin file that doesn't define the expected class
+        missing_file = File.join(temp_handler_dir, "missing_handler.rb")
+        File.write(missing_file, <<~RUBY)
+          # This file doesn't define MissingHandler class
+          class SomeOtherClass
+          end
+        RUBY
+
+        expect {
+          described_class.send(:load_custom_handler_plugin, missing_file, temp_handler_dir)
+        }.to raise_error(StandardError, /Handler class not found: MissingHandler/)
+      end
     end
 
     describe "lifecycle plugin loading failures" do
@@ -358,6 +397,23 @@ def on_request(env)
           described_class.send(:load_custom_lifecycle_plugin, wrong_file, temp_lifecycle_dir)
         }.to raise_error(StandardError, /Lifecycle plugin class must inherit from Hooks::Plugins::Lifecycle/)
       end
+
+      it "raises error when lifecycle plugin class is not found after loading" do
+        temp_lifecycle_dir = File.join(temp_dir, "lifecycle_missing_class")
+        FileUtils.mkdir_p(temp_lifecycle_dir)
+
+        # Create plugin file that doesn't define the expected class
+        missing_file = File.join(temp_lifecycle_dir, "missing_lifecycle.rb")
+        File.write(missing_file, <<~RUBY)
+          # This file doesn't define MissingLifecycle class
+          class SomeOtherClass
+          end
+        RUBY
+
+        expect {
+          described_class.send(:load_custom_lifecycle_plugin, missing_file, temp_lifecycle_dir)
+        }.to raise_error(StandardError, /Lifecycle plugin class not found: MissingLifecycle/)
+      end
     end
 
     describe "instrument plugin loading failures" do
@@ -418,6 +474,23 @@ def record(metric_name, value, tags = {})
           described_class.send(:load_custom_instrument_plugin, wrong_file, temp_instrument_dir)
         }.to raise_error(StandardError, /Instrument plugin class must inherit from StatsBase or FailbotBase/)
       end
+
+      it "raises error when instrument plugin class is not found after loading" do
+        temp_instrument_dir = File.join(temp_dir, "instrument_missing_class")
+        FileUtils.mkdir_p(temp_instrument_dir)
+
+        # Create plugin file that doesn't define the expected class
+        missing_file = File.join(temp_instrument_dir, "missing_instrument.rb")
+        File.write(missing_file, <<~RUBY)
+          # This file doesn't define MissingInstrument class
+          class SomeOtherClass
+          end
+        RUBY
+
+        expect {
+          described_class.send(:load_custom_instrument_plugin, missing_file, temp_instrument_dir)
+        }.to raise_error(StandardError, /Instrument plugin class not found: MissingInstrument/)
+      end
     end
   end
 end