Improve rack spoof handling and split integration setup

Author: ndbroadbent

lib/log_struct/integrations.rb

@@ -38,11 +38,25 @@ def self.setup_initializers(railtie)
       end
     end
 
-    sig { void }
-    def self.setup_integrations
+    sig { params(stage: Symbol).void }
+    def self.setup_integrations(stage: :all)
       config = LogStruct.config
 
-      # Set up each integration with consistent configuration pattern
+      case stage
+      when :non_middleware
+        setup_non_middleware_integrations(config)
+      when :middleware
+        setup_middleware_integrations(config)
+      when :all
+        setup_non_middleware_integrations(config)
+        setup_middleware_integrations(config)
+      else
+        raise ArgumentError, "Unknown integration stage: #{stage}"
+      end
+    end
+
+    sig { params(config: LogStruct::Configuration).void }
+    def self.setup_non_middleware_integrations(config)
       Integrations::Lograge.setup(config) if config.integrations.enable_lograge
       Integrations::ActionMailer.setup(config) if config.integrations.enable_actionmailer
       Integrations::ActiveJob.setup(config) if config.integrations.enable_activejob
@@ -51,8 +65,6 @@ def self.setup_integrations
       Integrations::GoodJob.setup(config) if config.integrations.enable_goodjob
       Integrations::Ahoy.setup(config) if config.integrations.enable_ahoy
       Integrations::ActiveModelSerializers.setup(config) if config.integrations.enable_active_model_serializers
-      Integrations::HostAuthorization.setup(config) if config.integrations.enable_host_authorization
-      Integrations::RackErrorHandler.setup(config) if config.integrations.enable_rack_error_handler
       Integrations::Shrine.setup(config) if config.integrations.enable_shrine
       Integrations::ActiveStorage.setup(config) if config.integrations.enable_activestorage
       Integrations::CarrierWave.setup(config) if config.integrations.enable_carrierwave
@@ -62,5 +74,13 @@ def self.setup_integrations
       end
       Integrations::Puma.setup(config) if config.integrations.enable_puma
     end
+
+    sig { params(config: LogStruct::Configuration).void }
+    def self.setup_middleware_integrations(config)
+      Integrations::HostAuthorization.setup(config) if config.integrations.enable_host_authorization
+      Integrations::RackErrorHandler.setup(config) if config.integrations.enable_rack_error_handler
+    end
+
+    private_class_method :setup_non_middleware_integrations, :setup_middleware_integrations
   end
 end

lib/log_struct/integrations/rack_error_handler.rb

@@ -19,9 +19,9 @@ def self.setup(config)
         return nil unless config.integrations.enable_rack_error_handler
 
         # Add structured logging middleware for security violations and errors
-        # Need to insert after ShowExceptions to catch IP spoofing errors
-        ::Rails.application.middleware.insert_after(
-          ::ActionDispatch::ShowExceptions,
+        # Need to insert before RemoteIp to catch IP spoofing errors it raises
+        ::Rails.application.middleware.insert_before(
+          ::ActionDispatch::RemoteIp,
           Integrations::RackErrorHandler::Middleware
         )
 

lib/log_struct/integrations/rack_error_handler/middleware.rb

@@ -55,8 +55,14 @@ def initialize(app)
         def call(env)
           return @app.call(env) unless LogStruct.enabled?
 
-          # Try to process the request
+          request = ::ActionDispatch::Request.new(env)
+
           begin
+            # Trigger the same spoofing checks that ActionDispatch::RemoteIp performs after
+            # it is initialized in the middleware stack. We run this manually because we
+            # execute before that middleware and still want spoofing attacks to surface here.
+            perform_remote_ip_check!(request)
+
             @app.call(env)
           rescue ::ActionDispatch::RemoteIp::IpSpoofAttackError => ip_spoof_error
             # Create a security log for IP spoofing
@@ -65,7 +71,7 @@ def call(env)
               http_method: env["REQUEST_METHOD"],
               user_agent: env["HTTP_USER_AGENT"],
               referer: env["HTTP_REFERER"],
-              request_id: env["action_dispatch.request_id"],
+              request_id: request.request_id,
               message: ip_spoof_error.message,
               client_ip: env["HTTP_CLIENT_IP"],
               x_forwarded_for: env["HTTP_X_FORWARDED_FOR"],
@@ -74,13 +80,7 @@ def call(env)
 
             ::Rails.logger.warn(security_log)
 
-            # Report the error
-            context = extract_request_context(env)
-            LogStruct.handle_exception(ip_spoof_error, source: Source::Security, context: context)
-
-            # If handle_exception raised an exception then Rails will deal with it (e.g. config.exceptions_app)
-            # If we are only logging or reporting these security errors, then return a default response
-            [FORBIDDEN_STATUS, IP_SPOOF_HEADERS, [IP_SPOOF_HTML]]
+            [FORBIDDEN_STATUS, IP_SPOOF_HEADERS.dup, [IP_SPOOF_HTML]]
           rescue ::ActionController::InvalidAuthenticityToken => invalid_auth_token_error
             # Create a security log for CSRF error
             request = ::ActionDispatch::Request.new(env)
@@ -102,7 +102,7 @@ def call(env)
 
             # If handle_exception raised an exception then Rails will deal with it (e.g. config.exceptions_app)
             # If we are only logging or reporting these security errors, then return a default response
-            [FORBIDDEN_STATUS, CSRF_HEADERS, [CSRF_HTML]]
+            [FORBIDDEN_STATUS, CSRF_HEADERS.dup, [CSRF_HTML]]
           rescue => error
             # Extract request context for error reporting
             context = extract_request_context(env)
@@ -119,6 +119,23 @@ def call(env)
 
         private
 
+        sig { params(request: ::ActionDispatch::Request).void }
+        def perform_remote_ip_check!(request)
+          action_dispatch_config = ::Rails.application.config.action_dispatch
+
+          remote_ip_middleware = ::ActionDispatch::RemoteIp.new(
+            ->(_env) { [200, {}, []] },
+            action_dispatch_config.ip_spoofing_check,
+            action_dispatch_config.trusted_proxies
+          )
+
+          return unless remote_ip_middleware.check_ip
+
+          ::ActionDispatch::RemoteIp::GetIp
+            .new(request, remote_ip_middleware.check_ip, remote_ip_middleware.proxies)
+            .to_s
+        end
+
         sig { params(env: T::Hash[String, T.untyped]).returns(T::Hash[Symbol, T.untyped]) }
         def extract_request_context(env)
           request = ::ActionDispatch::Request.new(env)

lib/log_struct/railtie.rb

@@ -25,12 +25,21 @@ class Railtie < ::Rails::Railtie
       # Merge Rails filter parameters into our filters
       LogStruct.merge_rails_filter_parameters!
 
-      # Set up all integrations
-      Integrations.setup_integrations
+      # Set up non-middleware integrations first
+      Integrations.setup_integrations(stage: :non_middleware)
 
       # Note: Host allowances are managed by the test app itself.
     end
 
+    # Setup middleware integrations during Rails configuration (before middleware stack is built)
+    # Must be done in the Railtie class body, not in an initializer
+    initializer "logstruct.configure_middleware", before: :build_middleware_stack do |app|
+      # This runs before middleware stack is frozen, so we can configure it
+      next unless LogStruct.enabled?
+
+      Integrations.setup_integrations(stage: :middleware)
+    end
+
     # Emit Puma lifecycle logs when running `rails server`
     initializer "logstruct.puma_lifecycle", after: "logstruct.configure_logger" do
       is_server = ::LogStruct.server_mode?