Remove all dynamic plugin loading for fully static boot-time only approach

Co-authored-by: GrantBirki <[email protected]>

Author: Copilot

lib/hooks/app/auth/auth.rb

@@ -1,6 +1,5 @@
 # frozen_string_literal: true
 
-require "pathname"
 require_relative "../../core/plugin_loader"
 
 module Hooks
@@ -29,27 +28,11 @@ def validate_auth!(payload, headers, endpoint_config, global_config = {})
           error!("authentication configuration missing or invalid", 500)
         end
 
-        # Get auth plugin from loaded plugins registry first
+        # Get auth plugin from loaded plugins registry (boot-time loaded only)
         begin
           auth_class = Core::PluginLoader.get_auth_plugin(auth_type)
         rescue => e
-          # If not found in registry and auth_plugin_dir is provided, fall back to dynamic loading
-          if global_config[:auth_plugin_dir]
-            begin
-              auth_class = load_auth_plugin_dynamically(auth_type, global_config[:auth_plugin_dir])
-            rescue => fallback_error
-              # Preserve specific error messages for better debugging
-              if fallback_error.message.include?("not found") ||
-                 fallback_error.message.include?("invalid auth plugin") ||
-                 fallback_error.message.include?("must inherit from")
-                error!(fallback_error.message, 500)
-              else
-                error!("unsupported auth type '#{auth_type}'", 400)
-              end
-            end
-          else
-            error!("unsupported auth type '#{auth_type}'", 400)
-          end
+          error!("unsupported auth type '#{auth_type}'", 400)
         end
 
         unless auth_class.valid?(
@@ -60,70 +43,6 @@ def validate_auth!(payload, headers, endpoint_config, global_config = {})
           error!("authentication failed", 401)
         end
       end
-
-      private
-
-      # Load auth plugin class dynamically (fallback for backward compatibility)
-      #
-      # @param auth_type [String] The auth type/plugin name
-      # @param auth_plugin_dir [String] The directory containing auth plugin files
-      # @return [Class] The loaded auth plugin class
-      # @raise [StandardError] If the auth plugin cannot be loaded
-      def load_auth_plugin_dynamically(auth_type, auth_plugin_dir)
-        # Convert auth_type to CamelCase class name
-        auth_plugin_class_name = auth_type.split("_").map(&:capitalize).join("")
-
-        # Validate the converted class name before attempting to load
-        unless valid_auth_plugin_class_name?(auth_plugin_class_name)
-          raise StandardError, "invalid auth plugin type '#{auth_type}'"
-        end
-
-        # Convert class name to file name (e.g., SomeCoolAuthPlugin -> some_cool_auth_plugin.rb)
-        file_name = auth_plugin_class_name.gsub(/([A-Z])/, '_\1').downcase.sub(/^_/, "") + ".rb"
-        file_path = File.join(auth_plugin_dir, file_name)
-
-        # Security: Ensure the file path doesn't escape the auth plugin directory
-        normalized_auth_plugin_dir = Pathname.new(File.expand_path(auth_plugin_dir))
-        normalized_file_path = Pathname.new(File.expand_path(file_path))
-        unless normalized_file_path.descend.any? { |path| path == normalized_auth_plugin_dir }
-          raise StandardError, "auth plugin path outside of auth plugin directory"
-        end
-
-        if File.exist?(file_path)
-          require file_path
-          auth_plugin_class = Object.const_get("Hooks::Plugins::Auth::#{auth_plugin_class_name}")
-
-          # Security: Ensure the loaded class inherits from the expected base class
-          unless auth_plugin_class < Hooks::Plugins::Auth::Base
-            raise StandardError, "auth plugin class must inherit from Hooks::Plugins::Auth::Base"
-          end
-
-          auth_plugin_class
-        else
-          raise StandardError, "Auth plugin #{auth_plugin_class_name} not found at #{file_path}"
-        end
-      end
-
-      # Validate that an auth plugin class name is safe to load
-      #
-      # @param class_name [String] The class name to validate
-      # @return [Boolean] true if the class name is safe, false otherwise
-      def valid_auth_plugin_class_name?(class_name)
-        # Must be a string
-        return false unless class_name.is_a?(String)
-
-        # Must not be empty or only whitespace
-        return false if class_name.strip.empty?
-
-        # Must match a safe pattern: alphanumeric + underscore, starting with uppercase
-        # Examples: MyAuthPlugin, SomeCoolAuthPlugin, CustomAuth
-        return false unless class_name.match?(/\A[A-Z][a-zA-Z0-9_]*\z/)
-
-        # Must not be a system/built-in class name
-        return false if Hooks::Security::DANGEROUS_CLASSES.include?(class_name)
-
-        true
-      end
     end
   end
 end

lib/hooks/app/helpers.rb

@@ -65,69 +65,19 @@ def parse_payload(raw_body, headers, symbolize: true)
       # Load handler class
       #
       # @param handler_class_name [String] The name of the handler class to load
-      # @param handler_dir [String] The directory containing handler files (fallback for dynamic loading)
+      # @param handler_dir [String] The directory containing handler files (unused - kept for compatibility)
       # @return [Object] An instance of the loaded handler class
       # @raise [StandardError] If handler cannot be found
       def load_handler(handler_class_name, handler_dir = nil)
-        # Try to get handler class from loaded plugins registry first
+        # Get handler class from loaded plugins registry (boot-time loaded only)
         begin
           handler_class = Core::PluginLoader.get_handler_plugin(handler_class_name)
           return handler_class.new
         rescue => e
-          # If not found in registry and handler_dir is provided, fall back to dynamic loading
-          if handler_dir
-            return load_handler_dynamically(handler_class_name, handler_dir)
-          else
-            error!("failed to get handler '#{handler_class_name}': #{e.message}", 500)
-          end
+          error!("failed to get handler '#{handler_class_name}': #{e.message}", 500)
         end
       end
 
-      private
-
-      # Load handler class dynamically (fallback for backward compatibility)
-      #
-      # @param handler_class_name [String] The name of the handler class to load
-      # @param handler_dir [String] The directory containing handler files
-      # @return [Object] An instance of the loaded handler class
-      # @raise [LoadError] If the handler file or class cannot be found
-      # @raise [StandardError] Halts with error if handler cannot be loaded
-      def load_handler_dynamically(handler_class_name, handler_dir)
-        # Security: Validate handler class name to prevent arbitrary class loading
-        unless valid_handler_class_name?(handler_class_name)
-          error!("invalid handler class name: #{handler_class_name}", 400)
-        end
-
-        # Convert class name to file name (e.g., Team1Handler -> team1_handler.rb)
-        # E.g.2: GithubHandler -> github_handler.rb
-        # E.g.3: GitHubHandler -> git_hub_handler.rb
-        file_name = handler_class_name.gsub(/([A-Z])/, '_\1').downcase.sub(/^_/, "") + ".rb"
-        file_path = File.join(handler_dir, file_name)
-
-        # Security: Ensure the file path doesn't escape the handler directory
-        normalized_handler_dir = Pathname.new(File.expand_path(handler_dir))
-        normalized_file_path = Pathname.new(File.expand_path(file_path))
-        unless normalized_file_path.descend.any? { |path| path == normalized_handler_dir }
-          error!("handler path outside of handler directory", 400)
-        end
-
-        if File.exist?(file_path)
-          require file_path
-          handler_class = Object.const_get(handler_class_name)
-
-          # Security: Ensure the loaded class inherits from the expected base class
-          unless handler_class < Hooks::Plugins::Handlers::Base
-            error!("handler class must inherit from Hooks::Plugins::Handlers::Base", 400)
-          end
-
-          handler_class.new
-        else
-          raise LoadError, "Handler #{handler_class_name} not found at #{file_path}"
-        end
-      rescue => e
-        error!("failed to load handler: #{e.message}", 500)
-      end
-
       public
 
       # Load auth plugin class (DEPRECATED - plugins are now loaded at boot time)
@@ -143,48 +93,6 @@ def load_auth_plugin(auth_plugin_class_name, auth_plugin_dir)
 
       private
 
-      # Validate that a handler class name is safe to load
-      #
-      # @param class_name [String] The class name to validate
-      # @return [Boolean] true if the class name is safe, false otherwise
-      def valid_handler_class_name?(class_name)
-        # Must be a string
-        return false unless class_name.is_a?(String)
-
-        # Must not be empty or only whitespace
-        return false if class_name.strip.empty?
-
-        # Must match a safe pattern: alphanumeric + underscore, starting with uppercase
-        # Examples: MyHandler, GitHubHandler, Team1Handler
-        return false unless class_name.match?(/\A[A-Z][a-zA-Z0-9_]*\z/)
-
-        # Must not be a system/built-in class name
-        return false if Hooks::Security::DANGEROUS_CLASSES.include?(class_name)
-
-        true
-      end
-
-      # Validate that an auth plugin class name is safe to load
-      #
-      # @param class_name [String] The class name to validate
-      # @return [Boolean] true if the class name is safe, false otherwise
-      def valid_auth_plugin_class_name?(class_name)
-        # Must be a string
-        return false unless class_name.is_a?(String)
-
-        # Must not be empty or only whitespace
-        return false if class_name.strip.empty?
-
-        # Must match a safe pattern: alphanumeric + underscore, starting with uppercase
-        # Examples: MyAuthPlugin, SomeCoolAuthPlugin, CustomAuth
-        return false unless class_name.match?(/\A[A-Z][a-zA-Z0-9_]*\z/)
-
-        # Must not be a system/built-in class name
-        return false if Hooks::Security::DANGEROUS_CLASSES.include?(class_name)
-
-        true
-      end
-
       # Determine HTTP error code from exception
       #
       # @param exception [Exception] The exception to map to an HTTP status code

spec/unit/lib/hooks/app/auth/custom_auth_plugin_spec.rb

@@ -30,25 +30,28 @@ def error!(message, code)
     before do
       # Create temporary directory for custom auth plugins
       FileUtils.mkdir_p(custom_auth_plugin_dir)
+      # Clear plugin registries before each test
+      Hooks::Core::PluginLoader.clear_plugins
     end
 
     after do
       # Clean up
       FileUtils.rm_rf(custom_auth_plugin_dir) if Dir.exist?(custom_auth_plugin_dir)
+      # Clear plugin registries after each test
+      Hooks::Core::PluginLoader.clear_plugins
     end
 
-    context "when custom auth plugin is configured but directory not set" do
-      it "falls back to POC error message" do
+    context "when custom auth plugin is configured but not loaded at boot time" do
+      it "returns unsupported auth type error" do
         endpoint_config = { auth: { type: "some_cool_auth_plugin" } }
-        empty_global_config = {}
 
         expect do
-          instance.validate_auth!(payload, headers, endpoint_config, empty_global_config)
+          instance.validate_auth!(payload, headers, endpoint_config, global_config)
         end.to raise_error(StandardError, /unsupported auth type/)
       end
     end
 
-    context "when custom auth plugin exists and is valid" do
+    context "when custom auth plugin exists and is loaded at boot time" do
       let(:plugin_file_content) do
         <<~RUBY
           module Hooks
@@ -68,9 +71,11 @@ def self.valid?(payload:, headers:, config:)
 
       before do
         File.write(File.join(custom_auth_plugin_dir, "some_cool_auth_plugin.rb"), plugin_file_content)
+        # Load plugins at boot time as would happen in real application
+        Hooks::Core::PluginLoader.load_all_plugins(global_config)
       end
 
-      it "loads and uses the custom auth plugin successfully" do
+      it "uses the custom auth plugin successfully" do
         endpoint_config = { auth: { type: "some_cool_auth_plugin" } }
 
         expect do
@@ -99,6 +104,8 @@ def self.valid?(payload:, headers:, config:)
 
       before do
         File.write(File.join(custom_auth_plugin_dir, "failing_auth_plugin.rb"), plugin_file_content)
+        # Load plugins at boot time as would happen in real application
+        Hooks::Core::PluginLoader.load_all_plugins(global_config)
       end
 
       it "returns authentication failed error" do
@@ -110,67 +117,17 @@ def self.valid?(payload:, headers:, config:)
       end
     end
 
-    context "when custom auth plugin file does not exist" do
-      it "returns custom plugin loading error" do
+    context "when custom auth plugin file does not exist at boot time" do
+      it "returns unsupported auth type error" do
         endpoint_config = { auth: { type: "nonexistent_plugin" } }
 
         expect do
           instance.validate_auth!(payload, headers, endpoint_config, global_config)
-        end.to raise_error(StandardError, /Auth plugin NonexistentPlugin not found/)
-      end
-    end
-
-    context "when custom auth plugin has security issues" do
-      context "with invalid class name" do
-        it "converts lowercase plugin name and fails to find file" do
-          endpoint_config = { auth: { type: "lowercase_plugin" } }
-
-          expect do
-            instance.validate_auth!(payload, headers, endpoint_config, global_config)
-          end.to raise_error(StandardError, /Auth plugin LowercasePlugin not found/)
-        end
-
-        it "rejects plugin with special characters" do
-          endpoint_config = { auth: { type: "plugin$bad" } }
-
-          expect do
-            instance.validate_auth!(payload, headers, endpoint_config, global_config)
-          end.to raise_error(StandardError, /invalid auth plugin type/)
-        end
-      end
-
-      context "with plugin that doesn't inherit from Base" do
-        let(:bad_plugin_file_content) do
-          <<~RUBY
-            module Hooks
-              module Plugins
-                module Auth
-                  class BadPlugin
-                    def self.valid?(payload:, headers:, config:)
-                      true
-                    end
-                  end
-                end
-              end
-            end
-          RUBY
-        end
-
-        before do
-          File.write(File.join(custom_auth_plugin_dir, "bad_plugin.rb"), bad_plugin_file_content)
-        end
-
-        it "rejects plugin that doesn't inherit from Base" do
-          endpoint_config = { auth: { type: "bad_plugin" } }
-
-          expect do
-            instance.validate_auth!(payload, headers, endpoint_config, global_config)
-          end.to raise_error(StandardError, /auth plugin class must inherit from/)
-        end
+        end.to raise_error(StandardError, /unsupported auth type/)
       end
     end
 
-    context "with complex plugin names" do
+    context "with complex plugin names loaded at boot time" do
       let(:plugin_file_content) do
         <<~RUBY
           module Hooks
@@ -189,6 +146,8 @@ def self.valid?(payload:, headers:, config:)
 
       before do
         File.write(File.join(custom_auth_plugin_dir, "git_hub_o_auth2_plugin.rb"), plugin_file_content)
+        # Load plugins at boot time as would happen in real application
+        Hooks::Core::PluginLoader.load_all_plugins(global_config)
       end
 
       it "handles complex CamelCase names correctly" do