Skip to content

fix(events): clamp page param to >= 1 so junk pagination params don't 500 - #2775

Open
mroderick wants to merge 1 commit into
masterfrom
fix/clamp-page-param
Open

fix(events): clamp page param to >= 1 so junk pagination params don't 500#2775
mroderick wants to merge 1 commit into
masterfrom
fix/clamp-page-param

Conversation

@mroderick

@mroderick mroderick commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

This PR is about saving maintainer attention by not throwing 500 errors when security scanners try to hit the application.

Problem

Rollbar item codebar-production/709: a security scanner hit /events/past?page=' OR '1'='1 and got a Pagy::OptionError: expected :page >= 1; got 0 500. (params[:page] || 1).to_i converts the probe string to 0, and Pagy::Offset.new rejects page < 1. The same junk input also 500s via ?page=, ?page=-3, and ?page[]=1 (the latter as NoMethodError on Array#to_i).

Fix

EventsController#paginated_events is the only manual Pagy::Offset path in the app — every other paginated endpoint uses the standard pagy helper, which clamps the page via Pagy::Request#resolve_page ([page.to_s.to_i, 1].max). This mirrors that exact clamp:

page = [1, params[:page].to_s.to_i].max

The .to_s also handles array params (?page[]=1). Junk input now renders page 1 (200) instead of a 500, so no more Rollbar notifications from these probes.

Tests

Added request specs covering page=0, SQL injection probe, empty, negative, array, and valid page params. All green; rubocop clean.

@mroderick
mroderick marked this pull request as ready for review August 3, 2026 07:28
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