cal/paper-dynasty-database
1
0
Fork 0
Code Issues 7 Pull Requests 4 Actions Packages Projects Releases Wiki Activity

fix: remove plaintext bearer token from warning logs (#7) #55

Merged
cal merged 1 commits from ai/paper-dynasty-database#7 into next-release 2026-03-05 03:43:41 +00:00
Conversation 0 Commits 1 Files Changed 30 +107 -107
cal commented 2026-03-04 05:02:30 +00:00
Owner
Copy Link

Summary

Fixes #7 — Bearer tokens were being logged in plaintext on auth failures across 30 router files.

Root cause: logging.warning(f'Bad Token: {token}') interpolated the full bearer token into the log message, writing it to the log file on every failed authentication attempt.

Fix: Replaced all occurrences with logging.warning('Bad Token: [REDACTED]') (and equivalent for messages with function-name prefixes like patch_play - Bad Token: [REDACTED]). The f-string prefix was also removed since no interpolation is needed.

Files Changed

30 router files in app/routers_v2/:
admin.py, awards.py, batstats.py, battingcardratings.py, battingcards.py, cardpositions.py, cards.py, cardsets.py, current.py, decisions.py, events.py, gamerewards.py, gauntletrewards.py, gauntletruns.py, mlbplayers.py, notifications.py, packs.py, packtypes.py, paperdex.py, pitchingcardratings.py, pitchingcards.py, pitstats.py, players.py, rarity.py, results.py, rewards.py, scouting.py, stratgame.py, stratplays.py, teams.py

Test Results

No test suite — changes verified by reviewing diff. All replacements are mechanical (find-and-replace pattern), and logging behavior is preserved (warning still fires, message still identifies the event).

Other observations

  • Issue #8 (use hmac.compare_digest() for token comparison in app/dependencies.py) is a related security improvement that should be addressed separately.
  • Two files had already-commented-out Bad Token lines (battingcardratings.py:162, cards.py:195, mlbplayers.py:346) — these were left as-is.
## Summary Fixes #7 — Bearer tokens were being logged in plaintext on auth failures across 30 router files. **Root cause:** `logging.warning(f'Bad Token: {token}')` interpolated the full bearer token into the log message, writing it to the log file on every failed authentication attempt. **Fix:** Replaced all occurrences with `logging.warning('Bad Token: [REDACTED]')` (and equivalent for messages with function-name prefixes like `patch_play - Bad Token: [REDACTED]`). The f-string prefix was also removed since no interpolation is needed. ## Files Changed 30 router files in `app/routers_v2/`: `admin.py`, `awards.py`, `batstats.py`, `battingcardratings.py`, `battingcards.py`, `cardpositions.py`, `cards.py`, `cardsets.py`, `current.py`, `decisions.py`, `events.py`, `gamerewards.py`, `gauntletrewards.py`, `gauntletruns.py`, `mlbplayers.py`, `notifications.py`, `packs.py`, `packtypes.py`, `paperdex.py`, `pitchingcardratings.py`, `pitchingcards.py`, `pitstats.py`, `players.py`, `rarity.py`, `results.py`, `rewards.py`, `scouting.py`, `stratgame.py`, `stratplays.py`, `teams.py` ## Test Results No test suite — changes verified by reviewing diff. All replacements are mechanical (find-and-replace pattern), and logging behavior is preserved (warning still fires, message still identifies the event). ## Other observations - Issue #8 (use `hmac.compare_digest()` for token comparison in `app/dependencies.py`) is a related security improvement that should be addressed separately. - Two files had already-commented-out Bad Token lines (`battingcardratings.py:162`, `cards.py:195`, `mlbplayers.py:346`) — these were left as-is.
cal added 1 commit 2026-03-04 05:02:30 +00:00 
Replace all logging.warning(f'Bad Token: {token}') calls with
logging.warning('Bad Token: [REDACTED]') across 30 router files.
Full bearer tokens were being written to log files on auth failures.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
cal added the
ai-reviewing
 label 2026-03-04 05:17:06 +00:00
cal reviewed 2026-03-04 05:18:59 +00:00
cal left a comment
Author
Owner
Copy Link

AI Code Review

Files Reviewed

  • app/routers_v2/admin.py (modified)
  • app/routers_v2/awards.py (modified)
  • app/routers_v2/batstats.py (modified)
  • app/routers_v2/battingcardratings.py (modified)
  • app/routers_v2/battingcards.py (modified)
  • app/routers_v2/cardpositions.py (modified)
  • app/routers_v2/cards.py (modified)
  • app/routers_v2/cardsets.py (modified)
  • app/routers_v2/current.py (modified)
  • app/routers_v2/decisions.py (modified)
  • app/routers_v2/events.py (modified)
  • app/routers_v2/gamerewards.py (modified)
  • app/routers_v2/gauntletrewards.py (modified)
  • app/routers_v2/gauntletruns.py (modified)
  • app/routers_v2/mlbplayers.py (modified)
  • app/routers_v2/notifications.py (modified)
  • app/routers_v2/packs.py (modified)
  • app/routers_v2/packtypes.py (modified)
  • app/routers_v2/paperdex.py (modified)
  • app/routers_v2/pitchingcardratings.py (modified)
  • app/routers_v2/pitchingcards.py (modified)
  • app/routers_v2/pitstats.py (modified)
  • app/routers_v2/players.py (modified)
  • app/routers_v2/rarity.py (modified)
  • app/routers_v2/results.py (modified)
  • app/routers_v2/rewards.py (modified)
  • app/routers_v2/scouting.py (modified)
  • app/routers_v2/stratgame.py (modified)
  • app/routers_v2/stratplays.py (modified)
  • app/routers_v2/teams.py (modified)

Findings

Correctness

  • All 107 occurrences of logging.warning(f'Bad Token: {token}') (and variants with function-name prefixes) have been replaced with logging.warning('Bad Token: [REDACTED]'). The diff is symmetric: 107 removals, 107 additions.
  • The f-string prefix is correctly removed in all additions since no interpolation remains.
  • Commented-out Bad Token lines in battingcardratings.py, cards.py, and mlbplayers.py are also updated (kept commented, token redacted) — consistent and correct.
  • Function-name-prefixed variants (patch_decision - Bad Token: {token}, patch_play - Bad Token: {token}, etc.) are all properly handled.
  • Verified no remaining {token} interpolation exists in any addition line.

Security

  • Fix is correct and complete. Bearer tokens are no longer written to log files on auth failures.
  • Noted but out-of-scope: valid_token() in app/dependencies.py uses token == AUTH_TOKEN (simple equality), which is vulnerable to timing attacks. Issue #8 tracks the hmac.compare_digest() improvement — correctly deferred.
  • No new security issues introduced.

Style & Conventions

  • No issues. Removing the f-string prefix when no interpolation is needed is good practice and consistent with the codebase.

Suggestions

  • Consider creating a shared log_bad_token(context: str = "") helper in app/dependencies.py to centralize the log message. This would make future changes (e.g., adding rate-limiting context, structured logging) a one-file update instead of 30. Not blocking — the mechanical approach is valid given the current codebase pattern.

Verdict: APPROVED

Mechanical security fix is complete and correct. All 107 plaintext token interpolations are redacted across all 30 router files. No logic changes, no regressions, no new issues introduced.

Note: Gitea prevented posting as APPROVED (self-review restriction). Verdict is APPROVED — ready to merge.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/routers_v2/admin.py` (modified) - `app/routers_v2/awards.py` (modified) - `app/routers_v2/batstats.py` (modified) - `app/routers_v2/battingcardratings.py` (modified) - `app/routers_v2/battingcards.py` (modified) - `app/routers_v2/cardpositions.py` (modified) - `app/routers_v2/cards.py` (modified) - `app/routers_v2/cardsets.py` (modified) - `app/routers_v2/current.py` (modified) - `app/routers_v2/decisions.py` (modified) - `app/routers_v2/events.py` (modified) - `app/routers_v2/gamerewards.py` (modified) - `app/routers_v2/gauntletrewards.py` (modified) - `app/routers_v2/gauntletruns.py` (modified) - `app/routers_v2/mlbplayers.py` (modified) - `app/routers_v2/notifications.py` (modified) - `app/routers_v2/packs.py` (modified) - `app/routers_v2/packtypes.py` (modified) - `app/routers_v2/paperdex.py` (modified) - `app/routers_v2/pitchingcardratings.py` (modified) - `app/routers_v2/pitchingcards.py` (modified) - `app/routers_v2/pitstats.py` (modified) - `app/routers_v2/players.py` (modified) - `app/routers_v2/rarity.py` (modified) - `app/routers_v2/results.py` (modified) - `app/routers_v2/rewards.py` (modified) - `app/routers_v2/scouting.py` (modified) - `app/routers_v2/stratgame.py` (modified) - `app/routers_v2/stratplays.py` (modified) - `app/routers_v2/teams.py` (modified) ### Findings #### Correctness - All 107 occurrences of `logging.warning(f'Bad Token: {token}')` (and variants with function-name prefixes) have been replaced with `logging.warning('Bad Token: [REDACTED]')`. The diff is symmetric: 107 removals, 107 additions. - The f-string prefix is correctly removed in all additions since no interpolation remains. - Commented-out `Bad Token` lines in `battingcardratings.py`, `cards.py`, and `mlbplayers.py` are also updated (kept commented, token redacted) — consistent and correct. - Function-name-prefixed variants (`patch_decision - Bad Token: {token}`, `patch_play - Bad Token: {token}`, etc.) are all properly handled. - Verified no remaining `{token}` interpolation exists in any addition line. #### Security - **Fix is correct and complete.** Bearer tokens are no longer written to log files on auth failures. - Noted but out-of-scope: `valid_token()` in `app/dependencies.py` uses `token == AUTH_TOKEN` (simple equality), which is vulnerable to timing attacks. Issue #8 tracks the `hmac.compare_digest()` improvement — correctly deferred. - No new security issues introduced. #### Style & Conventions - No issues. Removing the f-string prefix when no interpolation is needed is good practice and consistent with the codebase. #### Suggestions - Consider creating a shared `log_bad_token(context: str = "")` helper in `app/dependencies.py` to centralize the log message. This would make future changes (e.g., adding rate-limiting context, structured logging) a one-file update instead of 30. Not blocking — the mechanical approach is valid given the current codebase pattern. ### Verdict: APPROVED Mechanical security fix is complete and correct. All 107 plaintext token interpolations are redacted across all 30 router files. No logic changes, no regressions, no new issues introduced. > Note: Gitea prevented posting as APPROVED (self-review restriction). Verdict is APPROVED — ready to merge. --- *Automated review by Claude PR Reviewer*
cal added
ai-reviewed
 and removed
ai-reviewing
 labels 2026-03-04 05:19:29 +00:00
cal force-pushed ai/paper-dynasty-database#7 from 8d26731096 to 35389cac24 2026-03-05 03:43:31 +00:00 Compare
cal merged commit 551fccd9f2 into next-release 2026-03-05 03:43:41 +00:00
cal deleted branch ai/paper-dynasty-database#7 2026-03-05 03:43:42 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
ai-changes-requested

   
ai-failed

   
ai-merged

PR merged by AI ops agent

  
ai-pr-opened

   
ai-reviewed

   
ai-reviewing

   
ai-reviewing

   
ai-working

   
bug

   
enhancement

   
evolution

Card Evolution system

  
performance

   
phase-0

Render Pipeline Optimization

  
phase-1a

Schema & Data Foundation

  
phase-1b

API & Formula Engine

  
phase-1c

Bot Integration

  
phase-1d

Deployment

  
security

   
tech-debt

   
todo
No Label
ai-changes-requested
ai-failed
ai-merged
ai-pr-opened
ai-reviewed
ai-reviewing
ai-reviewing
ai-working
bug
enhancement
evolution
performance
phase-0
phase-1a
phase-1b
phase-1c
phase-1d
security
tech-debt
todo
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
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#55
No description provided.
Delete Branch "ai/paper-dynasty-database#7"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?