Skip to content

fix: Validate sort_by parameter against allowlist in repositories#1128

Merged
daveearley merged 1 commit intodevelopfrom
fix/validate-sort-by-parameters
Mar 28, 2026
Merged

fix: Validate sort_by parameter against allowlist in repositories#1128
daveearley merged 1 commit intodevelopfrom
fix/validate-sort-by-parameters

Conversation

@daveearley
Copy link
Copy Markdown
Contributor

@daveearley daveearley commented Mar 28, 2026

Summary

  • Multiple repository classes were passing the user-supplied sort_by query parameter directly to Eloquent's orderBy() without validation, enabling SQL injection (PostgreSQL supports stacked queries, so this is particularly spicy)
  • Added validateSortColumn() and validateSortDirection() helpers to BaseRepository that check against each domain object's existing getAllowedSorts() whitelist
  • Applied validation across all 11 affected repository files — invalid values silently fall back to defaults
  • Added IsSortable to ProductCategoryDomainObject (the one domain object that was missing it)

The allowlist pattern was already used correctly in the admin endpoints (getAllEventsForAdmin, getAllOrdersForAdmin) — this just applies the same approach everywhere else.

Test plan

  • All 350 unit tests pass
  • Verify sorting still works on attendees, orders, events, promo codes, etc.
  • Confirm invalid sort_by values fall back to defaults instead of erroring

Reported by @tikket1

Apply getAllowedSorts() whitelist validation to all repository orderBy()
calls, preventing SQL injection via the sort_by query parameter.

Reported by @tikket1

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@daveearley daveearley changed the base branch from main to develop March 28, 2026 14:02
@daveearley daveearley merged commit 01e1aee into develop Mar 28, 2026
4 checks passed
@daveearley daveearley deleted the fix/validate-sort-by-parameters branch March 28, 2026 14:03
@github-actions github-actions bot locked and limited conversation to collaborators Mar 28, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant