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

fix: scrub exposed credentials from docs #52

Merged
cal merged 5 commits from fix/scrub-exposed-credentials into main 2026-04-01 18:02:02 +00:00
Conversation 0 Commits 5 Files Changed 3 +7 -7
cal commented 2026-04-01 16:59:19 +00:00
Owner
Copy Link

Summary

  • Replace real API token value in docs/PD_CARDS_CLI_REFERENCE.md (line 421) with your-api-token-here placeholder
  • Confirmed no other tracked files contain the exposed token (.env is already gitignored)
  • No Supabase JWT or Ladies5-Monogamy-Charter references found in any tracked card-creation files

Context

The token Tp3aO3jhYve5NJF1IqOmJTmk was embedded in a shell example command in the CLI reference doc. While the .env file containing the same token is gitignored, the doc was tracked and publicly visible.

Closes paper-dynasty-database#9
Closes #50

## Summary - Replace real API token value in `docs/PD_CARDS_CLI_REFERENCE.md` (line 421) with `your-api-token-here` placeholder - Confirmed no other tracked files contain the exposed token (`.env` is already gitignored) - No Supabase JWT or `Ladies5-Monogamy-Charter` references found in any tracked card-creation files ## Context The token `Tp3aO3jhYve5NJF1IqOmJTmk` was embedded in a shell example command in the CLI reference doc. While the `.env` file containing the same token is gitignored, the doc was tracked and publicly visible. Closes paper-dynasty-database#9 Closes #50
cal added 4 commits 2026-04-01 16:59:20 +00:00 
Seven pre-implementation test cases covering: 108-sum invariant
preservation under profile-based boosts, D20 probability shift
magnitude at T4, pipeline collision risk between T4 rarity upgrade
and live-series post_player_updates, HoF rarity cap (non-contiguous
ID ladder), RP T1 achievability, SP/RP/batter T4 parity, and the
cross-season stat accumulation design decision that must be confirmed
before Phase 2 code is written.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add rarity name cross-reference table in Background section mapping PRD
  display names (Replacement/Reserve/Starter/All-Star/MVP/Hall of Fame) to
  codebase names (Common/Bronze/Silver/Gold/Diamond/HoF) with IDs
- Fix T4-2: correct Gold OPS threshold from 0.700 to 0.900 (confirmed in
  rarity_thresholds.py); add note that 0.700 is the Bronze floor
- Fix T4-1: restate truncation invariant as a single precise assertion —
  sum(columns) == 108 - truncated_amount — instead of two independent checks
  that can both pass while the sum is wrong for unrelated reasons

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
db_calls.py default timeouts raised from 3s to 30s across all methods
(db_get, url_get, db_patch, db_post, db_put). scouting_batters.py
fetch_data now passes timeout=120 for large card rating queries.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace real API token in PD_CARDS_CLI_REFERENCE.md example command
with placeholder value to prevent credential exposure in the repo.

Closes #50
Claude added the
ai-reviewing
 label 2026-04-01 17:17:59 +00:00
Claude reviewed 2026-04-01 17:19:25 +00:00
Claude left a comment
Collaborator
Copy Link

AI Code Review

Files Reviewed

  • docs/PD_CARDS_CLI_REFERENCE.md (modified) — credential scrub
  • db_calls.py (modified) — default timeout 3 → 30 for 5 async functions
  • scouting_batters.py (modified) — explicit timeout=120 in fetch_data()
  • docs/REFRACTOR_PHASE2_VALIDATION_SPEC.md (added) — 468-line Phase 2 validation spec

Findings

Correctness

  • Credential scrub is correct. API_TOKEN=Tp3aO3jhYve5NJF1IqOmJTmk replaced with your-api-token-here on the target line. No functional breakage.
  • Timeout defaults (3 → 30): Functionally sound. 3 seconds was very short for remote API calls, particularly batch card generation and scouting operations that fan out to many endpoints. PR #41 already wired the timeout param into aiohttp.ClientTimeout; this PR upgrades the safe default. No logic errors.
  • scouting_batters.py timeout=120: Appropriate for long-running scouting queries that pull large player sets.
  • REFRACTOR_PHASE2_VALIDATION_SPEC.md: Documentation-only. Content is internally consistent with the PRD chapters referenced.

Security

  • The exposed token Tp3aO3jhYve5NJF1IqOmJTmk is scrubbed from the tracked doc. ✓
  • No new credentials or secrets introduced.
  • Reminder (pre-existing, not introduced here): The AUTH_TOKEN bearer token is still hardcoded in db_calls.py line 7; that cleanup was noted in the PR #44 review but is out of scope here.

Style & Conventions

  • Scope mismatch: The PR title/body describes only the credential scrub, but the diff contains three additional, unrelated changes: default timeout increases in db_calls.py, a timeout=120 call in scouting_batters.py, and a new 468-line design doc. None of these are mentioned in the PR description. The changes themselves are correct, but the undocumented scope makes the diff harder to audit and the PR harder to trace back to its motivation if something regresses.

Suggestions

  • Consider adding a line to the PR body noting the timeout default bump and the reason (scouting operations were timing out at 3s), so the commit history is self-documenting.
  • The REFRACTOR_PHASE2_VALIDATION_SPEC.md appears to have been merged already via the docs: refractor Phase 2 design validation spec (#51) commit (eaf4bdb on main). Verify this file isn't a duplicate before merge — the PR diff shows it as added, but the base branch SHA is eaf4bdb, which carries the same spec.

Verdict: COMMENT

The security fix is correct and should merge. The bundled timeout changes are sound. Flag the potential duplicate of REFRACTOR_PHASE2_VALIDATION_SPEC.md before merging — if it already landed in the base via #51, this PR will produce a no-op add or conflict.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `docs/PD_CARDS_CLI_REFERENCE.md` (modified) — credential scrub - `db_calls.py` (modified) — default timeout 3 → 30 for 5 async functions - `scouting_batters.py` (modified) — explicit `timeout=120` in `fetch_data()` - `docs/REFRACTOR_PHASE2_VALIDATION_SPEC.md` (added) — 468-line Phase 2 validation spec ### Findings #### Correctness - **Credential scrub is correct.** `API_TOKEN=Tp3aO3jhYve5NJF1IqOmJTmk` replaced with `your-api-token-here` on the target line. No functional breakage. - **Timeout defaults (3 → 30):** Functionally sound. 3 seconds was very short for remote API calls, particularly batch card generation and scouting operations that fan out to many endpoints. PR #41 already wired the `timeout` param into `aiohttp.ClientTimeout`; this PR upgrades the safe default. No logic errors. - **`scouting_batters.py` `timeout=120`:** Appropriate for long-running scouting queries that pull large player sets. - **`REFRACTOR_PHASE2_VALIDATION_SPEC.md`:** Documentation-only. Content is internally consistent with the PRD chapters referenced. #### Security - The exposed token `Tp3aO3jhYve5NJF1IqOmJTmk` is scrubbed from the tracked doc. ✓ - No new credentials or secrets introduced. - **Reminder (pre-existing, not introduced here):** The `AUTH_TOKEN` bearer token is still hardcoded in `db_calls.py` line 7; that cleanup was noted in the PR #44 review but is out of scope here. #### Style & Conventions - **Scope mismatch:** The PR title/body describes only the credential scrub, but the diff contains three additional, unrelated changes: default timeout increases in `db_calls.py`, a `timeout=120` call in `scouting_batters.py`, and a new 468-line design doc. None of these are mentioned in the PR description. The changes themselves are correct, but the undocumented scope makes the diff harder to audit and the PR harder to trace back to its motivation if something regresses. #### Suggestions - Consider adding a line to the PR body noting the timeout default bump and the reason (scouting operations were timing out at 3s), so the commit history is self-documenting. - The `REFRACTOR_PHASE2_VALIDATION_SPEC.md` appears to have been merged already via the `docs: refractor Phase 2 design validation spec (#51)` commit (`eaf4bdb` on `main`). Verify this file isn't a duplicate before merge — the PR diff shows it as added, but the base branch SHA is `eaf4bdb`, which carries the same spec. ### Verdict: COMMENT The security fix is correct and should merge. The bundled timeout changes are sound. Flag the potential duplicate of `REFRACTOR_PHASE2_VALIDATION_SPEC.md` before merging — if it already landed in the base via #51, this PR will produce a no-op add or conflict. --- *Automated review by Claude PR Reviewer*
Claude added
ai-reviewed
 and removed
ai-reviewing
 labels 2026-04-01 17:19:52 +00:00
Claude approved these changes 2026-04-01 18:01:55 +00:00
Claude left a comment
Collaborator
Copy Link

Approved via pd-pr

Approved via pd-pr
cal added 1 commit 2026-04-01 18:01:59 +00:00
cal merged commit c1f06eb9c7 into main 2026-04-01 18:02:02 +00:00
cal deleted branch fix/scrub-exposed-credentials 2026-04-01 18:02:02 +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#52
No description provided.
Delete Branch "fix/scrub-exposed-credentials"

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?