Fix/2491 unclosed file handles

.github/workflows/run-tests-generate-output.yaml

@@ -35,7 +35,8 @@ jobs:
       - name: Checkout repository
         uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
         with:
-          ref: ${{ github.event.pull_request.head.sha }}
+          # use the commit that triggered this workflow to avoid any wildcard fetch
+          ref: ${{ github.sha }}
           repository: ${{ github.event.pull_request.head.repo.full_name }}
           persist-credentials: false
       # Set the pip environment up
@@ -173,7 +174,8 @@ jobs:
       - name: Checkout repository
         uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1
         with:
-          ref: ${{ github.event.pull_request.head.ref }}
+          # use the run's SHA rather than branch to avoid wildcard refspec
+          ref: ${{ github.sha }}
       - name: Download translation check report
         uses: actions/download-artifact@fa0a91b85d4f404e444e00e005971372dc801d16 # v4.1.8
         with:

copi.owasp.org/lib/copi_web/live/player_live/index.html.heex

@@ -1,4 +1,5 @@
 
+<%= if @player do %>
   <.live_component
     module={CopiWeb.PlayerLive.FormComponent}
     id={:new}
@@ -8,6 +9,7 @@
     client_ip={@client_ip}
     patch={~p"/games/#{@game.id}"}
   />
+<% end %>
 
 
 <!--

copi.owasp.org/lib/copi_web/live/player_live/show.ex

@@ -23,11 +23,11 @@ defmodule CopiWeb.PlayerLive.Show do
         {:noreply, socket |> assign(:game, game) |> assign(:player, player)}
       else
         {:error, _reason} ->
-          {:ok, redirect(socket, to: "/error")}
+          {:noreply, redirect(socket, to: "/error")}
       end
     else
       {:error, _reason} ->
-        {:ok, redirect(socket, to: "/error")}
+        {:noreply, redirect(socket, to: "/error")}
     end
   end
 
@@ -37,7 +37,7 @@ defmodule CopiWeb.PlayerLive.Show do
       {:noreply, socket |> assign(:game, updated_game) |> assign(:player, updated_player)}
     else
       {:error, _reason} ->
-        {:ok, redirect(socket, to: "/error")}
+        {:noreply, redirect(socket, to: "/error")}
     end
   end
 

copi.owasp.org/test/copi/cornucopia/dealt_card_test.exs

@@ -0,0 +1,24 @@
+defmodule Copi.Cornucopia.DealtCardTest do
+  use Copi.DataCase, async: true
+
+  alias Copi.Cornucopia.DealtCard
+
+  describe "changeset/2" do
+    test "returns a valid changeset with empty attrs" do
+      changeset = DealtCard.changeset(%DealtCard{}, %{})
+      assert changeset.valid?
+    end
+
+    test "casts played_in_round when provided" do
+      changeset = DealtCard.changeset(%DealtCard{}, %{played_in_round: 3})
+      # changeset/2 only casts [], so played_in_round is ignored but changeset is valid
+      assert changeset.valid?
+    end
+  end
+
+  describe "find/1" do
+    test "returns error for nonexistent dealt card id" do
+      assert {:error, :not_found} = DealtCard.find(-1)
+    end
+  end
+end

copi.owasp.org/test/copi/cornucopia/player_test.exs

@@ -0,0 +1,32 @@
+defmodule Copi.Cornucopia.PlayerTest do
+  use ExUnit.Case, async: true
+
+  alias Copi.Cornucopia.Player
+
+  describe "changeset/2" do
+    test "valid with name only" do
+      # game_id is not in validate_required, so name alone is enough
+      changeset = Player.changeset(%Player{}, %{name: "Alice"})
+      assert changeset.valid?
+    end
+
+    test "invalid without name" do
+      changeset = Player.changeset(%Player{}, %{})
+      refute changeset.valid?
+      assert %{name: ["can't be blank"]} = errors_on(changeset)
+    end
+
+    test "invalid when name too long" do
+      changeset = Player.changeset(%Player{}, %{name: String.duplicate("a", 51)})
+      refute changeset.valid?
+    end
+  end
+
+  defp errors_on(changeset) do
+    Ecto.Changeset.traverse_errors(changeset, fn {msg, opts} ->
+      Enum.reduce(opts, msg, fn {key, value}, acc ->
+        String.replace(acc, "%{#{key}}", to_string(value))
+      end)
+    end)
+  end
+end

copi.owasp.org/test/copi/cornucopia/vote_test.exs

@@ -0,0 +1,12 @@
+defmodule Copi.Cornucopia.VoteTest do
+  use ExUnit.Case, async: true
+
+  alias Copi.Cornucopia.Vote
+
+  describe "changeset/2" do
+    test "valid with empty attrs" do
+      changeset = Vote.changeset(%Vote{}, %{})
+      assert changeset.valid?
+    end
+  end
+end

copi.owasp.org/test/copi/ip_helper_test.exs

@@ -128,10 +128,37 @@ defmodule Copi.IPHelperTest do
       socket = %Phoenix.Socket{transport_pid: self()}
       Process.put(:peer, {{10, 0, 0, 5}, 12345})
       assert IPHelper.get_ip_from_socket(socket) == {10, 0, 0, 5}
-      
+
       socket2 = %Phoenix.Socket{transport_pid: nil}
       assert IPHelper.get_ip_from_socket(socket2) == {127, 0, 0, 1}
     end
+
+    test "falls back to localhost for LiveView socket with empty map connect_info" do
+      # connect_info is a map with no headers and no peer_data
+      socket = %Phoenix.LiveView.Socket{private: %{connect_info: %{}}}
+      assert IPHelper.get_ip_from_socket(socket) == {127, 0, 0, 1}
+    end
+
+    test "skips atom-key req_header tuple and falls back to binary-keyed one" do
+      socket = %Phoenix.LiveView.Socket{
+        private: %{connect_info: %{req_headers: [{:some_key, "ignored"}, {"x-forwarded-for", "10.0.2.5"}]}}
+      }
+      assert IPHelper.get_ip_from_socket(socket) == {10, 0, 2, 5}
+    end
+
+    test "falls back to peer_data when connect_info map x_headers has no x-forwarded-for" do
+      socket = %Phoenix.LiveView.Socket{
+        private: %{connect_info: %{x_headers: [{"other-header", "val"}], peer_data: %{address: {10, 0, 2, 6}}}}
+      }
+      assert IPHelper.get_ip_from_socket(socket) == {10, 0, 2, 6}
+    end
+
+    test "returns localhost when connect_info map has no peer_data and no useful headers" do
+      socket = %Phoenix.LiveView.Socket{
+        private: %{connect_info: %{x_headers: []}}
+      }
+      assert IPHelper.get_ip_from_socket(socket) == {127, 0, 0, 1}
+    end
   end
 
   describe "get_ip_from_connect_info/1" do
@@ -170,9 +197,39 @@ defmodule Copi.IPHelperTest do
     test "handles malformed extract_first_ip inputs" do
       info = %{x_headers: [{"x-forwarded-for", "invalid"}]}
       assert IPHelper.get_ip_from_connect_info(info) == nil
-      
+
       info2 = %{x_headers: [{"other", "10.0.0.1"}]}
       assert IPHelper.get_ip_from_connect_info(info2) == nil
     end
+
+    test "extracts IP from x_headers binary string" do
+      info = %{x_headers: "10.0.1.1"}
+      assert IPHelper.get_ip_from_connect_info(info) == {10, 0, 1, 1}
+    end
+
+    test "handles x_headers as non-map/non-list/non-binary (true->nil branch)" do
+      info = %{x_headers: 9999}
+      assert IPHelper.get_ip_from_connect_info(info) == nil
+    end
+
+    test "extracts IP from x_headers atom key tuple list" do
+      info = %{x_headers: [{:"x-forwarded-for", "10.0.1.2"}]}
+      assert IPHelper.get_ip_from_connect_info(info) == {10, 0, 1, 2}
+    end
+
+    test "returns nil for non-matching atom key in x_headers" do
+      info = %{x_headers: [{:other_header, "10.0.0.1"}]}
+      assert IPHelper.get_ip_from_connect_info(info) == nil
+    end
+
+    test "extracts IP from x_headers list of binary strings" do
+      info = %{x_headers: ["10.0.1.3"]}
+      assert IPHelper.get_ip_from_connect_info(info) == {10, 0, 1, 3}
+    end
+
+    test "returns nil for non-tuple elements in req_headers list" do
+      info = %{req_headers: [123, 456]}
+      assert IPHelper.get_ip_from_connect_info(info) == nil
+    end
   end
 end

copi.owasp.org/test/copi/rate_limiter_test.exs

@@ -241,13 +241,38 @@ defmodule Copi.RateLimiterTest do
       # Should still work even with weird input
       assert {:ok, _} = RateLimiter.check_rate("invalid-ip", :game_creation)
     end
+
+    test "normalize_ip fallback: passes non-tuple non-binary values through", %{ip: _ip} do
+      # nil is neither binary nor tuple, hits normalize_ip(ip), do: ip
+      assert {:ok, _} = RateLimiter.check_rate(nil, :game_creation)
+    end
+  end
+
+  describe "prod env bypass" do
+    test "bypasses rate limit for localhost in prod env" do
+      Application.put_env(:copi, :env, :prod)
+
+      try do
+        assert {:ok, :unlimited} = RateLimiter.check_rate({127, 0, 0, 1}, :game_creation)
+      after
+        Application.put_env(:copi, :env, :test)
+      end
+    end
   end
 
   describe "cleanup process" do
     test "rate limiter process is alive" do
       assert Process.whereis(Copi.RateLimiter) != nil
     end
 
+    test "cleanup message is handled without crashing" do
+      pid = Process.whereis(Copi.RateLimiter)
+      assert pid != nil
+      send(pid, :cleanup)
+      :timer.sleep(50)
+      assert Process.alive?(pid)
+    end
+
     test "can make requests after clearing IP", %{ip: ip} do
       config = RateLimiter.get_config()
       limit = config.limits.connection

copi.owasp.org/test/copi_web/controllers/card_controller_test.exs

@@ -43,5 +43,12 @@ defmodule CopiWeb.CardControllerTest do
     end
   end
 
+  describe "format_capec/1" do
+    test "returns the input list unchanged" do
+      assert CopiWeb.CardController.format_capec(["123", "456"]) == ["123", "456"]
+      assert CopiWeb.CardController.format_capec([]) == []
+    end
+  end
+
 
 end

copi.owasp.org/test/copi_web/live/game_live/show_pure_test.exs

@@ -0,0 +1,30 @@
+defmodule CopiWeb.GameLive.ShowPureTest do
+  use ExUnit.Case, async: true
+
+  alias CopiWeb.GameLive.Show
+
+  test "display_game_session returns expected labels" do
+    assert Show.display_game_session("webapp") == "Cornucopia Web Session:"
+    assert Show.display_game_session("mobileapp") == "Cornucopia Mobile Session:"
+    assert Show.display_game_session("mlsec") == "Elevation of MLSec Session:"
+    assert Show.display_game_session("unknown") == "EoP Session:"
+  end
+
+  test "latest_version returns expected versions" do
+    assert Show.latest_version("webapp") == "2.2"
+    assert Show.latest_version("ecommerce") == "1.22"
+    assert Show.latest_version("eop") == "5.1"
+    assert Show.latest_version("other") == "1.0"
+  end
+
+  test "card_played_in_round finds card or returns nil" do
+    cards = [%{played_in_round: 1, id: "a"}, %{played_in_round: 3, id: "b"}]
+    assert Show.card_played_in_round(cards, 3) == %{played_in_round: 3, id: "b"}
+    assert Show.card_played_in_round(cards, 2) == nil
+  end
+
+  test "topic builds topic string" do
+    assert Show.topic(42) == "game:42"
+    assert Show.topic("abc") == "game:abc"
+  end
+end

copi.owasp.org/test/copi_web/live/game_live/show_test.exs

@@ -128,5 +128,48 @@ defmodule CopiWeb.GameLive.ShowTest do
       alias CopiWeb.GameLive.Show
       assert Show.card_played_in_round([], 1) == nil
     end
+
+    test "card_played_in_round/2 returns the matching card", %{conn: _conn, game: _game} do
+      alias CopiWeb.GameLive.Show
+
+      cards = [%{played_in_round: 1, id: "a"}, %{played_in_round: 2, id: "b"}]
+      assert Show.card_played_in_round(cards, 2) == %{played_in_round: 2, id: "b"}
+    end
+
+    test "topic/1 builds topic strings", %{conn: _conn, game: _game} do
+      alias CopiWeb.GameLive.Show
+      assert Show.topic(1) == "game:1"
+      assert Show.topic("xyz") == "game:xyz"
+    end
+
+    test "handle_info ignores update for a different game id", %{conn: conn, game: game} do
+      {:ok, show_live, _html} = live(conn, "/games/#{game.id}")
+
+      {:ok, other_game} = Copi.Cornucopia.create_game(%{name: "other"})
+      {:ok, other_game_loaded} = Copi.Cornucopia.Game.find(other_game.id)
+
+      send(show_live.pid, %{
+        topic: "game:#{game.id}",
+        event: "game:updated",
+        payload: other_game_loaded
+      })
+
+      :timer.sleep(50)
+      assert render(show_live) =~ game.name
+    end
+
+    test "handle_params sets current_round to rounds_played when game is finished", %{conn: conn, game: game} do
+      {:ok, updated_game} = Copi.Cornucopia.update_game(game, %{
+        finished_at: DateTime.truncate(DateTime.utc_now(), :second),
+        rounds_played: 3
+      })
+      {:ok, _view, html} = live(conn, "/games/#{updated_game.id}?round=2")
+      assert is_binary(html)
+    end
+
+    test "handle_params redirects to /error for nonexistent game_id", %{conn: conn, game: _game} do
+      assert {:error, {:redirect, %{to: "/error"}}} =
+               live(conn, "/games/00000000000000000000000000")
+    end
   end
 end