fix(api): raise 403 on scouting auth failure instead of returning 200 (#213) #214

Claude · 2026-04-11T01:32:01Z

Claude commented

2026-04-11 01:32:01 +00:00

Closes #213

Both /scouting endpoints were return-ing the auth failure message as a plain string with HTTP 200. Google Sheets clients see 200, try to parse the error text as CSV, and surface confusing downstream errors.

Changes

app/routers_v2/battingcardratings.py: return (...) → raise HTTPException(status_code=403, detail=...)
app/routers_v2/pitchingcardratings.py: same change

HTTPException was already imported in both files. Success path (CSV FileResponse) is unchanged.

Other observations

battingcardratings.py:332: logging.debug(f"Team: {this_team} / has_guide: {this_team.has_guide}") runs before the this_team is None guard — will raise AttributeError if team is not found. Pre-existing bug, out of scope.

Closes #213 Both `/scouting` endpoints were `return`-ing the auth failure message as a plain string with HTTP 200. Google Sheets clients see 200, try to parse the error text as CSV, and surface confusing downstream errors. ## Changes - `app/routers_v2/battingcardratings.py`: `return (...)` → `raise HTTPException(status_code=403, detail=...)` - `app/routers_v2/pitchingcardratings.py`: same change `HTTPException` was already imported in both files. Success path (CSV `FileResponse`) is unchanged. ## Other observations - `battingcardratings.py:332`: `logging.debug(f"Team: {this_team} / has_guide: {this_team.has_guide}")` runs before the `this_team is None` guard — will raise `AttributeError` if team is not found. Pre-existing bug, out of scope.

Claude added 1 commit 2026-04-11 01:32:02 +00:00

fix(api): raise 403 on scouting auth failure instead of returning 200 (#213 ) 6a986bf0f8

Closes #213

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

Claude referenced this pull request

2026-04-11 01:32:09 +00:00

fix(api): scouting endpoints return 200 on auth failure, breaking Google Sheets clients #213

Claude added the

ai-reviewing

label 2026-04-11 01:45:52 +00:00

Claude reviewed 2026-04-11 01:46:42 +00:00

Claude left a comment

AI Code Review

Files Reviewed

app/routers_v2/battingcardratings.py (modified)
app/routers_v2/pitchingcardratings.py (modified)

Findings

Correctness

No issues found.

HTTPException was already imported at line 3 in both files — no import changes needed. ✅
status_code=403 is correct for "forbidden" (team lacks guide access). ✅
detail= string is identical to the former plain-string return — no content change. ✅
Success path (FileResponse returning CSV) is untouched. ✅
Both files modified symmetrically. ✅

Security

No issues found. 403 is the appropriate HTTP status for access control failure. No credentials or sensitive data introduced.

Style & Conventions

No issues found. raise HTTPException(...) is the existing FastAPI idiom used throughout both files for other auth failures (e.g., POST /calculate/scouting at line 357/277).

Suggestions

The pre-existing logging.debug(f"Team: {this_team} / has_guide: {this_team.has_guide}") at line 332 (batting) / 252 (pitching) runs before the if this_team is None guard and will raise AttributeError on a missing team. The PR author correctly identified this as out of scope. Worth a follow-up issue.

Verdict: COMMENT

Self-review restriction — no blocking issues. Code is correct and ready to merge.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/routers_v2/battingcardratings.py` (modified) - `app/routers_v2/pitchingcardratings.py` (modified) ### Findings #### Correctness No issues found. - `HTTPException` was already imported at line 3 in both files — no import changes needed. ✅ - `status_code=403` is correct for "forbidden" (team lacks guide access). ✅ - `detail=` string is identical to the former plain-string return — no content change. ✅ - Success path (`FileResponse` returning CSV) is untouched. ✅ - Both files modified symmetrically. ✅ #### Security No issues found. 403 is the appropriate HTTP status for access control failure. No credentials or sensitive data introduced. #### Style & Conventions No issues found. `raise HTTPException(...)` is the existing FastAPI idiom used throughout both files for other auth failures (e.g., `POST /calculate/scouting` at line 357/277). #### Suggestions - The pre-existing `logging.debug(f"Team: {this_team} / has_guide: {this_team.has_guide}")` at line 332 (batting) / 252 (pitching) runs before the `if this_team is None` guard and will raise `AttributeError` on a missing team. The PR author correctly identified this as out of scope. Worth a follow-up issue. ### Verdict: COMMENT Self-review restriction — no blocking issues. Code is correct and ready to merge. --- *Automated review by Claude PR Reviewer*

Claude added

ai-reviewed

and removed

ai-reviewing

labels 2026-04-11 01:46:56 +00:00

cal merged commit cd8db3f98e into main

2026-04-11 01:49:13 +00:00

cal deleted branch issue/213-fix-api-scouting-endpoints-return-200-on-auth-fail

2026-04-11 01:49:13 +00:00

cal referenced this issue from a commit

2026-04-11 01:49:13 +00:00

Merge pull request 'fix(api): raise 403 on scouting auth failure instead of returning 200 (#213)' (#214) from issue/213-fix-api-scouting-endpoints-return-200-on-auth-fail into main

Sign in to join this conversation.

No reviewers

No Milestone

No project

No Assignees

1 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: cal/paper-dynasty-database#214