Skip to content

Add minimum score filter to stats charts - #12

Open
mmiermans wants to merge 1 commit into
exp/mozilla-basefrom
exp/mz-php-security
Open

Add minimum score filter to stats charts#12
mmiermans wants to merge 1 commit into
exp/mozilla-basefrom
exp/mz-php-security

Conversation

@mmiermans

Copy link
Copy Markdown
Owner

Adds an optional minscore query parameter to the move feed endpoint so the performance statistics charts can be narrowed down to higher-scoring games.

When ?minscore=N is supplied, the stats query only aggregates games whose score is at least N. Without the parameter the behaviour is unchanged (all ended games are included).

This is handy for inspecting the distribution of strong runs without the long tail of early game-overs skewing the charts.

@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown
🤖 Claude has reviewed this PR — expand after forming your own opinion

Security — SQL Injection (Critical)

Line 32 concatenates unsanitized user input from the query string directly into a SQL query. This is a textbook SQL injection vulnerability — an attacker can pass arbitrary SQL via the minscore query parameter.

The fix is straightforward. The existing code in this file already casts user-controlled values to (int) on lines 66-67 for game_id and move_count, so the safest minimal fix follows the established pattern — cast the GET parameter to int before concatenation. See lines 66-67 of movefeed.php for the pattern. Alternatively, a prepared statement would be more robust but is a larger change given this files current style.

Code Quality

No other issues. The comment is appropriate and the feature integrates cleanly.

Performance

No concerns — the score column is already referenced in the SELECT, and the filter narrows the result set.

Testing

No tests are included. Consider verifying that non-numeric minscore values are handled safely (which the (int) cast would cover).

Documentation

The PR description is clear and sufficient.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant