fix: validate sort_by parameter with Literal type in views.py (#36) #44

cal · 2026-03-05T17:03:37Z

cal commented

2026-03-05 17:03:37 +00:00

Summary

Changed sort_by: str to sort_by: Literal[...] in both get_season_batting_stats and get_season_pitching_stats endpoints in app/routers_v3/views.py
Batting allows: pa, ab, run, hit, double, triple, homerun, rbi, bb, so, bphr, bpfo, bp1b, bplo, gidp, hbp, sac, ibb, avg, obp, slg, ops, woba, k_pct, sb, cs
Pitching allows: tbf, outs, games, gs, win, loss, hold, saves, bsave, ir, irs, ab, run, e_run, hits, double, triple, homerun, bb, so, hbp, sac, ibb, gidp, sb, cs, bphr, bpfo, bp1b, bplo, wp, balk, wpa, era, whip, avg, obp, slg, ops, woba, hper9, kper9, bbper9, kperbb, lob_2outs, rbipercent, re24
FastAPI will now reject requests with invalid sort_by values with a 422 Unprocessable Entity response before reaching the getattr call

Files Changed

app/routers_v3/views.py

Tests

No test suite exists for this repo. Changes verified by reading back modified file — both endpoints now use Literal type constraints that match the exact column names defined on SeasonBattingStats and SeasonPitchingStats models in db_engine.py.

Other observations

The project's ruff linter auto-reformatted the file on save (quote styles, import grouping, trailing commas). The diff is larger than the functional change because of this reformatting.
Several pre-existing Pyright warnings exist for StratPlay/StratGame attribute access and unused imports — not introduced by this change.

## Summary - Changed `sort_by: str` to `sort_by: Literal[...]` in both `get_season_batting_stats` and `get_season_pitching_stats` endpoints in `app/routers_v3/views.py` - Batting allows: `pa, ab, run, hit, double, triple, homerun, rbi, bb, so, bphr, bpfo, bp1b, bplo, gidp, hbp, sac, ibb, avg, obp, slg, ops, woba, k_pct, sb, cs` - Pitching allows: `tbf, outs, games, gs, win, loss, hold, saves, bsave, ir, irs, ab, run, e_run, hits, double, triple, homerun, bb, so, hbp, sac, ibb, gidp, sb, cs, bphr, bpfo, bp1b, bplo, wp, balk, wpa, era, whip, avg, obp, slg, ops, woba, hper9, kper9, bbper9, kperbb, lob_2outs, rbipercent, re24` - FastAPI will now reject requests with invalid `sort_by` values with a 422 Unprocessable Entity response before reaching the `getattr` call ## Files Changed - `app/routers_v3/views.py` ## Tests No test suite exists for this repo. Changes verified by reading back modified file — both endpoints now use `Literal` type constraints that match the exact column names defined on `SeasonBattingStats` and `SeasonPitchingStats` models in `db_engine.py`. ## Other observations - The project's ruff linter auto-reformatted the file on save (quote styles, import grouping, trailing commas). The diff is larger than the functional change because of this reformatting. - Several pre-existing Pyright warnings exist for `StratPlay`/`StratGame` attribute access and unused imports — not introduced by this change.

cal added 1 commit 2026-03-05 17:03:39 +00:00

fix: validate sort_by parameter with Literal type in views.py (#36 )

Build Docker Image / build (pull_request) Successful in 2m6s

Details

2d9462cf42

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

cal referenced this pull request

2026-03-05 17:03:49 +00:00

Validate `sort_by` parameter against allowed field names in `views.py` #36

cal added the

ai-reviewing

label 2026-03-05 17:15:12 +00:00

cal reviewed 2026-03-05 17:16:53 +00:00

cal left a comment

AI Code Review

Files Reviewed

app/routers_v3/views.py (modified)

Findings

Correctness

Verified all Literal values against SeasonBattingStats and SeasonPitchingStats field definitions in app/db_engine.py — every value in both Literal unions maps exactly to a real model field. No omissions or typos found.
Default values ("woba" for batting, "era" for pitching) are present in the Literal unions and unchanged from before.
The getattr fallback in get_top_hitters/get_top_pitchers is now effectively dead code for the FastAPI path (FastAPI validates before the call), but it's harmless and lives in db_engine.py outside this PR's scope.

Security

This change is a meaningful validation improvement. Previously sort_by: str accepted arbitrary values; they were passed to getattr with a safe default fallback, so not directly exploitable, but the explicit Literal constraint is cleaner defense-in-depth.
FastAPI will now return 422 Unprocessable Entity for invalid sort_by values before any ORM code runs.

Style & Conventions

The ruff reformatting (quote style, import grouping, trailing commas) is consistent and expected for this project.
Minor: sort_order.lower() is now redundant since sort_order is constrained to Literal["asc", "desc"] — both values are already lowercase. Not a blocker, and this pattern predates this PR.
Pre-existing unused imports (List, Query, pydantic) are not introduced by this change and are out of scope.

Suggestions

The redundant .lower() on sort_order could be cleaned up in a follow-up, but it's harmless.

Verdict: APPROVED

The Literal type constraints are correct, complete, and directly fix the issue. The diff is larger than the functional change due to ruff reformatting, but the logic is unchanged. Clean fix.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/routers_v3/views.py` (modified) ### Findings #### Correctness - Verified all `Literal` values against `SeasonBattingStats` and `SeasonPitchingStats` field definitions in `app/db_engine.py` — every value in both Literal unions maps exactly to a real model field. No omissions or typos found. - Default values (`"woba"` for batting, `"era"` for pitching) are present in the Literal unions and unchanged from before. - The `getattr` fallback in `get_top_hitters`/`get_top_pitchers` is now effectively dead code for the FastAPI path (FastAPI validates before the call), but it's harmless and lives in `db_engine.py` outside this PR's scope. #### Security - This change is a meaningful validation improvement. Previously `sort_by: str` accepted arbitrary values; they were passed to `getattr` with a safe default fallback, so not directly exploitable, but the explicit Literal constraint is cleaner defense-in-depth. - FastAPI will now return 422 Unprocessable Entity for invalid `sort_by` values before any ORM code runs. #### Style & Conventions - The ruff reformatting (quote style, import grouping, trailing commas) is consistent and expected for this project. - Minor: `sort_order.lower()` is now redundant since `sort_order` is constrained to `Literal["asc", "desc"]` — both values are already lowercase. Not a blocker, and this pattern predates this PR. - Pre-existing unused imports (`List`, `Query`, `pydantic`) are not introduced by this change and are out of scope. #### Suggestions - The redundant `.lower()` on `sort_order` could be cleaned up in a follow-up, but it's harmless. ### Verdict: APPROVED The Literal type constraints are correct, complete, and directly fix the issue. The diff is larger than the functional change due to ruff reformatting, but the logic is unchanged. Clean fix. --- *Automated review by Claude PR Reviewer*

cal added the

ai-reviewed

label 2026-03-05 17:17:00 +00:00

cal changed target branch from main to next-release

2026-03-07 07:32:38 +00:00

cal merged commit 6cbe529a82 into next-release

2026-03-10 14:41:31 +00:00

cal deleted branch ai/major-domo-database-36

2026-03-10 14:41:31 +00:00

cal referenced this issue from a commit

2026-03-10 14:41:32 +00:00

Merge pull request 'fix: validate sort_by parameter with Literal type in views.py (#36)' (#44) from ai/major-domo-database-36 into next-release