fix: prevent AlreadySentError in BruteForceProtection plug

The plug was attempting to register a before_send callback after
blocking a banned IP and sending a 403 response. This caused
Plug.Conn.AlreadySentError in production.

Fixed by checking conn.halted before attempting to register the
404 tracking callback. If the request was already blocked and sent,
we skip the callback registration.

Also updated the test to verify proper behavior without the
workaround for AlreadySentError.

lib/towerops_web/plugs/brute_force_protection.ex

@@ -40,9 +40,15 @@ defmodule ToweropsWeb.Plugs.BruteForceProtection do
     ip_address = get_remote_ip(conn)
 
     # Check whitelist and ban status before routing
-    conn
-    |> check_and_block(ip_address)
-    |> maybe_track_404(ip_address)
+    conn = check_and_block(conn, ip_address)
+
+    # Only register 404 tracking if we didn't already halt the request
+    # (halted requests have already been sent, can't register before_send)
+    if conn.halted do
+      conn
+    else
+      maybe_track_404(conn, ip_address)
+    end
   end
 
   defp check_and_block(conn, ip_address) do

test/towerops_web/plugs/brute_force_protection_test.exs

@@ -87,37 +87,20 @@ defmodule ToweropsWeb.Plugs.BruteForceProtectionTest do
   end
 
   describe "call/2 with banned IPs" do
-    # NOTE: The BruteForceProtection plug has a design issue where block_request/2
-    # sends the response (setting conn.state to :sent) and then maybe_track_404/2
-    # attempts register_before_send/2 which raises AlreadySentError on sent conns.
-    #
-    # This test verifies the blocking behavior by catching the AlreadySentError and
-    # inspecting the conn state after the block_request step completed.
     test "blocks requests from banned IPs with 403 and retry-after header" do
       ip = "198.51.100.#{[:positive] |> System.unique_integer() |> rem(255)}"
 
       # Create a ban for the IP (5 minute ban for first offense)
       {:ok, _block} = BruteForce.create_or_escalate_ban(ip, "Test ban")
 
-      # The plug will send 403, then raise AlreadySentError when trying to
-      # register before_send on the already-sent conn.
       conn =
-        try do
-          build_conn()
-          |> put_req_header("x-forwarded-for", ip)
-          |> BruteForceProtection.call([])
-        rescue
-          Plug.Conn.AlreadySentError ->
-            # When AlreadySentError is raised, the response has been sent but
-            # we lose the conn reference. Verify via BruteForce context instead.
-            nil
-        end
+        build_conn()
+        |> put_req_header("x-forwarded-for", ip)
+        |> BruteForceProtection.call([])
 
-      if conn do
-        # If no error was raised (future fix), verify directly
-        assert conn.halted
-        assert conn.status == 403
-      end
+      assert conn.halted
+      assert conn.status == 403
+      assert get_resp_header(conn, "retry-after") != []
 
       # Verify the IP is actually banned via the context
       assert {:blocked, _banned_until} = BruteForce.check_ban_status(ip)