fix: handle invalid dealt_card_id in PlayerLive toggle_vote gracefully (fixes #2843) by immortal71 · Pull Request #2865 · OWASP/cornucopia

immortal71 · 2026-04-23T19:35:51Z

Summary

toggle_vote crashed the PlayerLive process whenever a client sent a non-existent or invalid dealt_card_id. The bare pattern-match {:ok, dealt_card} = DealtCard.find(dealt_card_id) raised a MatchError on {:error, :not_found}, killing the LiveView process and disconnecting the user.

Root Cause

DealtCard.find/1 returns {:error, :not_found} for missing records. The match operator raised a MatchError, crashing the LiveView process.

Fix

Replaced the bare pattern-match with a case expression so that {:error, _} returns a flash error and keeps the socket alive, while the {:ok, dealt_card} path is fully unchanged in behaviour.

Changes

lib/copi_web/live/player_live/show.ex

Wrapped DealtCard.find/1 in a case expression
{:error, _} logs a warning and returns a flash error ("Invalid card selection") keeping the socket alive
{:ok, dealt_card} path is unchanged

test/copi_web/live/player_live/show_test.exs

Added describe "toggle_vote with invalid dealt_card_id" block
Test sends dealt_card_id = "999999999" (guaranteed non-existent) via phx-click
Asserts LiveView does not crash and renders "Invalid card selection" flash

Test Coverage — All 3 paths of toggle_vote

Path	Test
Non-existent `dealt_card_id` → flash error, no crash	New
Valid card from a different game → flash error	Existing
Valid card in current game → vote toggled	Existing

Before / After

Scenario	Before	After
Invalid ID sent	LiveView process crashes	Flash error shown, socket stays alive
Cross-game card ID	Flash error (already handled)	Unchanged
Valid card, same game	Vote toggled	Unchanged

Apply Copilot suggestion: use catalog.get('Categories') + isinstance(dict) check before accessing nested keys, preventing TypeError when Categories is null or a non-dict value. Add unit tests for all missing/malformed Categories edge cases: - Categories key absent - Categories is None - Categories is non-dict (e.g. list) - Category key missing from dict - Category value is not a list Closes OWASP#2487

…ies-2487 # Conflicts: # copi.owasp.org/lib/copi_web/live/player_live/show.ex # copi.owasp.org/test/copi/ip_helper_test.exs # copi.owasp.org/test/copi/rate_limiter_test.exs # copi.owasp.org/test/copi_web/controllers/card_controller_test.exs # copi.owasp.org/test/copi_web/live/game_live/show_test.exs # copi.owasp.org/test/copi_web/live/player_live/show_test.exs # copi.owasp.org/test/copi_web/plugs/rate_limiter_plug_test.exs # scripts/capec_map_enricher.py

…message in tests

Fixes OWASP#2843 Previously, oggle_vote called {:ok, dealt_card} = DealtCard.find(dealt_card_id), which raised a MatchError (crashing the LiveView process) whenever the ID did not exist in the database. Replace the bare pattern-match with a case expression so that an {:error, _} result returns a flash error message and leaves the socket alive, matching the expected behaviour described in the issue.

Copilot

Pull request overview

This PR primarily aims to prevent PlayerLive.Show from crashing when toggle_vote receives an invalid/non-existent dealt_card_id (issue #2843), while also adding a number of additional robustness/test-coverage changes across the repo (CAPEC enricher script/tests, rate limiter/IP helper tests, and some LiveView template/test additions).

Changes:

Updated toggle_vote to handle DealtCard.find/1 errors without crashing the LiveView, showing a flash instead.
Added/expanded ExUnit coverage for PlayerLive.Show, PlayerLive.Index, rate limiter plug, IP helper, and some schema changesets (plus new “pure” unit tests).
Hardened CAPEC enricher extraction logic and expanded Python unit tests around malformed Categories/Category sections.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
`copi.owasp.org/lib/copi_web/live/player_live/show.ex`	Handles `DealtCard.find/1` failures in `toggle_vote` instead of crashing.
`copi.owasp.org/test/copi_web/live/player_live/show_test.exs`	Adds LiveView test for invalid/non-existent `dealt_card_id` behavior.
`copi.owasp.org/lib/copi_web/live/player_live/index.html.heex`	Guards rendering of the player form component when `@player` is nil (index route).
`copi.owasp.org/test/copi_web/live/player_live_test.exs`	Adds more PlayerLive index/new form behavior coverage.
`copi.owasp.org/test/copi_web/plugs/rate_limiter_plug_test.exs`	Adds init passthrough test (currently duplicates an existing one).
`copi.owasp.org/test/copi/ip_helper_test.exs`	Adds more LiveView socket connect_info IP extraction cases.
`copi.owasp.org/test/copi/rate_limiter_test.exs`	Minor test structuring update (new describe section boundary).
`copi.owasp.org/test/copi_web/live/player_live/show_pure_test.exs`	New pure unit tests for helper functions in `PlayerLive.Show` + `FormComponent`.
`copi.owasp.org/test/copi_web/live/game_live/show_pure_test.exs`	New pure unit tests for helper functions in `GameLive.Show`.
`copi.owasp.org/test/copi/cornucopia/vote_test.exs`	New changeset unit test for `Vote`.
`copi.owasp.org/test/copi/cornucopia/player_test.exs`	New changeset unit tests for `Player`.
`copi.owasp.org/test/copi/cornucopia/dealt_card_test.exs`	New changeset/find unit tests for `DealtCard` (one test name is misleading).
`scripts/capec_map_enricher.py`	Makes CAPEC extraction more defensive and adds warnings for malformed sections.
`tests/scripts/capec_map_enricher_utest.py`	Adds Python unit tests covering malformed/missing `Categories` and `Category`.

Copilot · 2026-04-23T19:42:44Z

+<%= if @player do %>
  <.live_component
    module={CopiWeb.PlayerLive.FormComponent}
    id={:new}
    title={@page_title}
    action={@live_action}
    player={@player}
    client_ip={@client_ip}
    patch={~p"/games/#{@game.id}"}
  />
+<% end %>


This template change (guarding the form component when @player is nil on the index route) isn’t mentioned in the PR description, which focuses on toggle_vote. Please update the PR description to include this additional behavior change, or split it into a separate PR if it’s unrelated to #2843.

Copilot · 2026-04-23T19:42:45Z

+    case DealtCard.find(dealt_card_id) do
+      {:error, _reason} ->
+        Logger.warning("toggle_vote: dealt_card_id=#{dealt_card_id} not found for player_id=#{player.id}, game_id=#{game.id}")
+        {:noreply, socket |> put_flash(:error, "Invalid card selection")}


dealt_card_id comes from client params and is passed directly into DealtCard.find/1, which calls Repo.get/2 on an integer PK. For non-numeric IDs (e.g. "abc"), Repo.get/2 will raise a cast error rather than returning nil, so this code can still crash the LiveView. Consider validating/parsing dealt_card_id (e.g. Integer.parse/1) or rescuing cast errors inside DealtCard.find/1 and returning {:error, :invalid_id}.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

immortal71 · 2026-04-23T19:59:15Z

Superseded by #2866 — a clean branch rebased directly from upstream/master with no unrelated commits.

rewtd and others added 7 commits March 5, 2026 10:37

Merge branch 'master' into fix/capec-categories-2487

40ac90f

fix: use explicit key check for Categories to match expected warning …

d78a2b1

…message in tests

fix: unify Categories warning message to match all unit test assertions

9bce348

fix: resolve remaining conflict markers in Elixir test files

efcf26e

Copilot AI review requested due to automatic review settings April 23, 2026 19:35

immortal71 requested review from cw-owasp, rewtd and sydseter as code owners April 23, 2026 19:35

Copilot started reviewing on behalf of immortal71 April 23, 2026 19:36 View session

Copilot AI reviewed Apr 23, 2026

View reviewed changes

immortal71 and others added 4 commits April 23, 2026 12:52

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

8322fbf

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

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

c10edaf

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update copi.owasp.org/test/copi_web/live/player_live/show_test.exs

50f34be

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update copi.owasp.org/test/copi_web/plugs/rate_limiter_plug_test.exs

abe197d

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

immortal71 closed this Apr 23, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: handle invalid dealt_card_id in PlayerLive toggle_vote gracefully (fixes #2843)#2865

fix: handle invalid dealt_card_id in PlayerLive toggle_vote gracefully (fixes #2843)#2865
immortal71 wants to merge 11 commits intoOWASP:masterfrom
immortal71:fix/player-live-toggle-vote-invalid-card-id

immortal71 commented Apr 23, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Apr 23, 2026

Uh oh!

Copilot AI Apr 23, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

immortal71 commented Apr 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

immortal71 commented Apr 23, 2026

Summary

Root Cause

Fix

Changes

Test Coverage — All 3 paths of toggle_vote

Before / After

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

immortal71 commented Apr 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants