cal/paper-dynasty-card-creation
1
0
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases Wiki Activity

Remove hardcoded secrets, load API token from env #29

Merged
Claude merged 2 commits from fix/2-3-security-hardcoded-secrets into main 2026-03-23 03:50:08 +00:00
Conversation 0 Commits 2 Files Changed 5 +24 -81
cal commented 2026-03-20 17:38:22 +00:00
Owner
Copy Link

Summary

  • Replace hardcoded PD API bearer token in db_calls.py with python-dotenv + PD_API_TOKEN env var
  • Delete scripts/supabase_doodling.py (dead scratch file containing a hardcoded Supabase service-role JWT expiring 2031)
  • Consolidate check_prod_missing_ratings.py to import AUTH_TOKEN from db_calls instead of duplicating the token
  • Add .env.example template for onboarding

Hard fails at import time if PD_API_TOKEN is missing — prevents silent unauthenticated API requests.

Post-merge action required

Both tokens remain in git history — rotate after merge:

  • PD API bearer token — regenerate in Paper Dynasty admin
  • Supabase service-role JWT (expires 2031) — rotate in Supabase dashboard

Test plan

  • from db_calls import AUTH_TOKEN loads token from .env
  • Unset PD_API_TOKEN raises EnvironmentError
  • scripts/evo_milestone_simulator.py transitive import still works
  • pytest passes (1 pre-existing failure in test_automated_data_fetcher.py, unrelated)

Fixes #2, Fixes #3

🤖 Generated with Claude Code

## Summary - Replace hardcoded PD API bearer token in `db_calls.py` with `python-dotenv` + `PD_API_TOKEN` env var - Delete `scripts/supabase_doodling.py` (dead scratch file containing a hardcoded Supabase service-role JWT expiring 2031) - Consolidate `check_prod_missing_ratings.py` to import `AUTH_TOKEN` from `db_calls` instead of duplicating the token - Add `.env.example` template for onboarding Hard fails at import time if `PD_API_TOKEN` is missing — prevents silent unauthenticated API requests. ## Post-merge action required Both tokens remain in git history — **rotate after merge**: - **PD API bearer token** — regenerate in Paper Dynasty admin - **Supabase service-role JWT** (expires 2031) — rotate in Supabase dashboard ## Test plan - [x] `from db_calls import AUTH_TOKEN` loads token from `.env` - [x] Unset `PD_API_TOKEN` raises `EnvironmentError` - [x] `scripts/evo_milestone_simulator.py` transitive import still works - [x] `pytest` passes (1 pre-existing failure in `test_automated_data_fetcher.py`, unrelated) Fixes #2, Fixes #3 🤖 Generated with [Claude Code](https://claude.com/claude-code)
cal added 1 commit 2026-03-20 17:38:23 +00:00 
- Replace hardcoded PD API bearer token in db_calls.py with dotenv/env var
- Delete scripts/supabase_doodling.py (dead scratch file with hardcoded Supabase JWT)
- Add python-dotenv dependency and .env.example template
- Consolidate check_prod_missing_ratings.py to import AUTH_TOKEN from db_calls
- Hard fail if PD_API_TOKEN is missing to prevent silent auth failures

Fixes #2, Fixes #3

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Claude added the
ai-reviewing
 label 2026-03-20 18:48:46 +00:00
Claude approved these changes 2026-03-20 18:50:38 +00:00
Dismissed
Claude left a comment
Collaborator
Copy Link

AI Code Review

Files Reviewed

  • db_calls.py (modified)
  • .claude/ops-rework/check_prod_missing_ratings.py (modified)
  • .env.example (added)
  • pyproject.toml (modified)
  • scripts/supabase_doodling.py (deleted)

Findings

Correctness

  • load_dotenv() is called at module level before os.environ.get(), so .env is populated into the environment before the lookup. Correct ordering.
  • os.environ.get("PD_API_TOKEN") with if not _token: guard handles both missing and empty-string cases — both raise EnvironmentError. Correct.
  • load_dotenv() defaults to override=False, so a pre-existing PD_API_TOKEN env var (e.g., in CI) takes precedence over .env. Correct behavior.
  • parents[2] in check_prod_missing_ratings.py resolves correctly: .claude/ops-rework/ → .claude/ → project root. Path arithmetic verified.
  • check_prod_missing_ratings.py retains its own PROD_URL (instead of importing DB_URL from db_calls) — this correctly insulates the diagnostic script from alt_database overrides. Intentional and correct.
  • .env is already present in .gitignore (line 105) — no risk of accidentally committing the token file.

Security

  • Hardcoded PD API bearer token (Tp3aO3jhYve5NJF1IqOmJTmk) removed from db_calls.py. ✓
  • Hardcoded duplicate token removed from check_prod_missing_ratings.py. ✓
  • Supabase service-role JWT deleted along with dead supabase_doodling.py. ✓
  • .env.example provides a safe onboarding template with no real values. ✓
  • PR body correctly documents that both tokens remain in git history and require rotation post-merge. ✓
  • No new security surface introduced.

Style & Conventions

  • The f-string quote style changes in db_calls.py (single → double) are cosmetic and consistent within the changed lines. No functional impact.
  • from typing import Literal remains after third-party imports — this was already the case pre-PR and is unchanged behavior, so not in scope.
  • sys.path.insert(0, ...) in a nested ops script is pragmatic and appropriate given the file lives outside the main package structure.

Suggestions

  • Consider adding a note in .env.example about where to obtain the token (e.g., Paper Dynasty admin panel), so future contributors know where to look without having to read the codebase. Non-blocking.

Verdict: APPROVED

Clean, focused security fix. Implementation is correct, fail-fast behavior is properly enforced at import time, .env is already gitignored, and the deletion of the Supabase scratch file removes the only other hardcoded credential in the codebase. The post-merge token rotation reminder in the PR body is the right call.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `db_calls.py` (modified) - `.claude/ops-rework/check_prod_missing_ratings.py` (modified) - `.env.example` (added) - `pyproject.toml` (modified) - `scripts/supabase_doodling.py` (deleted) ### Findings #### Correctness - `load_dotenv()` is called at module level before `os.environ.get()`, so `.env` is populated into the environment before the lookup. Correct ordering. - `os.environ.get("PD_API_TOKEN")` with `if not _token:` guard handles both missing and empty-string cases — both raise `EnvironmentError`. Correct. - `load_dotenv()` defaults to `override=False`, so a pre-existing `PD_API_TOKEN` env var (e.g., in CI) takes precedence over `.env`. Correct behavior. - `parents[2]` in `check_prod_missing_ratings.py` resolves correctly: `.claude/ops-rework/` → `.claude/` → project root. Path arithmetic verified. - `check_prod_missing_ratings.py` retains its own `PROD_URL` (instead of importing `DB_URL` from `db_calls`) — this correctly insulates the diagnostic script from `alt_database` overrides. Intentional and correct. - `.env` is already present in `.gitignore` (line 105) — no risk of accidentally committing the token file. #### Security - Hardcoded PD API bearer token (`Tp3aO3jhYve5NJF1IqOmJTmk`) removed from `db_calls.py`. ✓ - Hardcoded duplicate token removed from `check_prod_missing_ratings.py`. ✓ - Supabase service-role JWT deleted along with dead `supabase_doodling.py`. ✓ - `.env.example` provides a safe onboarding template with no real values. ✓ - PR body correctly documents that both tokens remain in git history and require rotation post-merge. ✓ - No new security surface introduced. #### Style & Conventions - The f-string quote style changes in `db_calls.py` (single → double) are cosmetic and consistent within the changed lines. No functional impact. - `from typing import Literal` remains after third-party imports — this was already the case pre-PR and is unchanged behavior, so not in scope. - `sys.path.insert(0, ...)` in a nested ops script is pragmatic and appropriate given the file lives outside the main package structure. #### Suggestions - Consider adding a note in `.env.example` about where to obtain the token (e.g., Paper Dynasty admin panel), so future contributors know where to look without having to read the codebase. Non-blocking. ### Verdict: APPROVED Clean, focused security fix. Implementation is correct, fail-fast behavior is properly enforced at import time, `.env` is already gitignored, and the deletion of the Supabase scratch file removes the only other hardcoded credential in the codebase. The post-merge token rotation reminder in the PR body is the right call. --- *Automated review by Claude PR Reviewer*
Claude added
ai-reviewed
 and removed
ai-reviewing
 labels 2026-03-20 18:50:43 +00:00
Claude approved these changes 2026-03-23 03:49:47 +00:00
Claude left a comment
Collaborator
Copy Link

Security fix — removes hardcoded secrets. Reviewed by PO triage. Supersedes #42 and #44.

Security fix — removes hardcoded secrets. Reviewed by PO triage. Supersedes #42 and #44.
Claude added 1 commit 2026-03-23 03:50:04 +00:00
Claude merged commit 602151fb16 into main 2026-03-23 03:50:08 +00:00
Claude deleted branch fix/2-3-security-hardcoded-secrets 2026-03-23 03:50:08 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Claude
Labels
Clear labels   
ai-merged

PR merged by AI ops agent

  
ai-pr-opened

   
ai-reviewed

   
ai-reviewing

   
ai-working
No Label
ai-merged
ai-pr-opened
ai-reviewed
ai-reviewing
ai-working
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
2 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-card-creation#29
No description provided.
Delete Branch "fix/2-3-security-hardcoded-secrets"

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?