fix: move hardcoded Discord webhook URL to env var #83

Open
cal wants to merge 3 commits from fix/remove-hardcoded-webhook into main
Owner

Summary

  • Replaces hardcoded Discord webhook URL (including token) in dependencies.py with DISCORD_WEBHOOK_URL environment variable
  • Adds graceful degradation: logs a warning and returns False if the env var is unset
  • The exposed webhook token should be rotated in Discord (delete old webhook, create new one)

Test plan

  • Verify API starts without DISCORD_WEBHOOK_URL set (should log warning, not crash)
  • Verify webhook messages send correctly with DISCORD_WEBHOOK_URL set
  • Rotate the compromised webhook token in Discord

🤖 Generated with Claude Code

## Summary - Replaces hardcoded Discord webhook URL (including token) in `dependencies.py` with `DISCORD_WEBHOOK_URL` environment variable - Adds graceful degradation: logs a warning and returns `False` if the env var is unset - **The exposed webhook token should be rotated in Discord** (delete old webhook, create new one) ## Test plan - [ ] Verify API starts without `DISCORD_WEBHOOK_URL` set (should log warning, not crash) - [ ] Verify webhook messages send correctly with `DISCORD_WEBHOOK_URL` set - [ ] Rotate the compromised webhook token in Discord 🤖 Generated with [Claude Code](https://claude.com/claude-code)
cal added 1 commit 2026-03-27 04:15:38 +00:00
fix: move hardcoded Discord webhook URL to environment variable
All checks were successful
Build Docker Image / build (pull_request) Successful in 3m42s
3be4f71e22
Replace inline webhook URL+token with DISCORD_WEBHOOK_URL env var.
Logs a warning and returns False gracefully if the var is unset.

The exposed webhook token should be rotated in Discord.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
cal reviewed 2026-03-27 04:17:03 +00:00
cal left a comment
Author
Owner

AI Code Review

Files Reviewed

  • app/dependencies.py (modified)

Findings

Correctness

  • The fix is logically correct. os is already imported at line 5, so no new import is needed. The DISCORD_WEBHOOK_URL module-level constant follows the same pattern already used for REDIS_HOST, REDIS_PORT, REDIS_DB, and CACHE_ENABLED — consistent with the file's existing conventions.
  • The early-return guard (if not webhook_url) is clean and correct. The function returns False on missing config rather than crashing, which matches the existing error-handling pattern in the same function.

Security

  • Critical (pre-existing, now remediated): The hardcoded webhook token being removed from line 516 of main is the right call. The PR body notes the token must be rotated — this is mandatory. The token exists in git history and is fully compromised regardless of when this merges.
  • No new security concerns are introduced by this change.

Style & Conventions

  • The module-level constant placement is slightly inconsistent. Other env-var constants (REDIS_HOST, REDIS_PORT, REDIS_DB, CACHE_ENABLED) are grouped together near lines 26–29 at the top of the file. DISCORD_WEBHOOK_URL is declared mid-file just before send_webhook_message. Minor, but worth tidying.

Deployment Gap — docker-compose.yml not updated (MUST FIX)

DISCORD_WEBHOOK_URL is not wired into the api service environment block in docker-compose.yml (lines 21–36). Every other env var consumed by the API is listed there and pulled from .env. Without this, the variable will never reach the container in either the dev or prod compose stacks, and callers will always silently hit the warning path.

Additionally, .env does not include a DISCORD_WEBHOOK_URL= placeholder, so there is no documentation that the variable exists.

Required before merging:

  1. Add - DISCORD_WEBHOOK_URL=${DISCORD_WEBHOOK_URL} to the api environment block in docker-compose.yml.
  2. Add DISCORD_WEBHOOK_URL= (empty placeholder) to .env so the variable is discoverable.

Without these two additions, the webhook silently stops working in all environments and the breakage only surfaces via log inspection.

Suggestions

  • Move DISCORD_WEBHOOK_URL = os.environ.get("DISCORD_WEBHOOK_URL") up to the env var block near line 26 alongside the other Redis/cache constants, for consistency.

Verdict: REQUEST_CHANGES (posted as COMMENT — Gitea blocks self-review)

The core logic and security remediation are correct. The blocking issue is that docker-compose.yml and .env were not updated to carry the new variable, meaning no deployed container will receive it. Fix those two files and this is ready to merge.


Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/dependencies.py` (modified) ### Findings #### Correctness - The fix is logically correct. `os` is already imported at line 5, so no new import is needed. The `DISCORD_WEBHOOK_URL` module-level constant follows the same pattern already used for `REDIS_HOST`, `REDIS_PORT`, `REDIS_DB`, and `CACHE_ENABLED` — consistent with the file's existing conventions. - The early-return guard (`if not webhook_url`) is clean and correct. The function returns `False` on missing config rather than crashing, which matches the existing error-handling pattern in the same function. #### Security - **Critical (pre-existing, now remediated):** The hardcoded webhook token being removed from line 516 of `main` is the right call. The PR body notes the token must be rotated — this is mandatory. The token exists in git history and is fully compromised regardless of when this merges. - No new security concerns are introduced by this change. #### Style & Conventions - The module-level constant placement is slightly inconsistent. Other env-var constants (`REDIS_HOST`, `REDIS_PORT`, `REDIS_DB`, `CACHE_ENABLED`) are grouped together near lines 26–29 at the top of the file. `DISCORD_WEBHOOK_URL` is declared mid-file just before `send_webhook_message`. Minor, but worth tidying. #### Deployment Gap — `docker-compose.yml` not updated (MUST FIX) `DISCORD_WEBHOOK_URL` is not wired into the `api` service `environment` block in `docker-compose.yml` (lines 21–36). Every other env var consumed by the API is listed there and pulled from `.env`. Without this, the variable will never reach the container in either the dev or prod compose stacks, and callers will always silently hit the warning path. Additionally, `.env` does not include a `DISCORD_WEBHOOK_URL=` placeholder, so there is no documentation that the variable exists. **Required before merging:** 1. Add `- DISCORD_WEBHOOK_URL=${DISCORD_WEBHOOK_URL}` to the `api` `environment` block in `docker-compose.yml`. 2. Add `DISCORD_WEBHOOK_URL=` (empty placeholder) to `.env` so the variable is discoverable. Without these two additions, the webhook silently stops working in all environments and the breakage only surfaces via log inspection. #### Suggestions - Move `DISCORD_WEBHOOK_URL = os.environ.get("DISCORD_WEBHOOK_URL")` up to the env var block near line 26 alongside the other Redis/cache constants, for consistency. ### Verdict: REQUEST_CHANGES (posted as COMMENT — Gitea blocks self-review) The core logic and security remediation are correct. The blocking issue is that `docker-compose.yml` and `.env` were not updated to carry the new variable, meaning no deployed container will receive it. Fix those two files and this is ready to merge. --- *Automated review by Claude PR Reviewer*
@ -503,6 +503,9 @@ def update_season_pitching_stats(player_ids, season, db_connection):
raise
DISCORD_WEBHOOK_URL = os.environ.get("DISCORD_WEBHOOK_URL")
Author
Owner

All other env-var module-level constants (REDIS_HOST, REDIS_PORT, REDIS_DB, CACHE_ENABLED) are grouped together near the top of the file around lines 26–29. Declaring this one mid-file is inconsistent with that pattern. Consider moving it there.

All other env-var module-level constants (`REDIS_HOST`, `REDIS_PORT`, `REDIS_DB`, `CACHE_ENABLED`) are grouped together near the top of the file around lines 26–29. Declaring this one mid-file is inconsistent with that pattern. Consider moving it there.
@ -514,3 +517,3 @@
bool: True if successful, False otherwise
"""
webhook_url = "https://discord.com/api/webhooks/1408811717424840876/7RXG_D5IqovA3Jwa9YOobUjVcVMuLc6cQyezABcWuXaHo5Fvz1en10M7J43o3OJ3bzGW"
webhook_url = DISCORD_WEBHOOK_URL
Author
Owner

DISCORD_WEBHOOK_URL is not included in the api service environment block in docker-compose.yml. Without - DISCORD_WEBHOOK_URL=${DISCORD_WEBHOOK_URL} in that file, this os.environ.get will always return None in every deployed environment (dev and prod) and the webhook will silently do nothing.

`DISCORD_WEBHOOK_URL` is not included in the `api` service `environment` block in `docker-compose.yml`. Without `- DISCORD_WEBHOOK_URL=${DISCORD_WEBHOOK_URL}` in that file, this `os.environ.get` will always return `None` in every deployed environment (dev and prod) and the webhook will silently do nothing.
Claude added the
ai-reviewing
label 2026-03-27 04:19:01 +00:00
Claude reviewed 2026-03-27 04:19:59 +00:00
Claude left a comment
Collaborator

AI Code Review

Files Reviewed

  • app/dependencies.py (modified)

Findings

Correctness

  • The implementation correctly replaces the hardcoded webhook URL with os.environ.get("DISCORD_WEBHOOK_URL") at module level, then references that constant in send_webhook_message.
  • The if not webhook_url: guard handles both None (env var unset) and "" (empty string), which is correct — both cases should short-circuit with a warning.
  • os is already imported at line 5. logger is already defined at module level (line 15). No new imports needed.
  • The module-level constant placement (after update_season_pitching_stats, before send_webhook_message) is consistent with the existing pattern of REDIS_HOST, REDIS_PORT, REDIS_DB, and CACHE_ENABLED at lines 26–29.
  • The function's return behavior is unchanged for the success path — callers expecting bool are unaffected.

Security

  • The hardcoded Discord webhook token is removed from source. This is the primary goal and it is achieved correctly.
  • The exposed token (7RXG_D5IqovA3Jwa9YOobUjVcVMuLc6cQyezABcWuXaHo5Fvz1en10M7J43o3OJ3bzGW) remains in git history. The PR body correctly calls out that the webhook should be rotated in Discord — that rotation is necessary regardless of this merge.
  • DISCORD_WEBHOOK_URL must be added to docker-compose.prod.yml and the dev server env for the function to work post-merge. This is an operational step, not a code issue.

Style & Conventions

  • No issues. Follows existing file patterns for module-level env var reads.

Suggestions

  • No blocking suggestions. The implementation is clean and minimal.

Verdict: APPROVED

This is a correct, well-scoped security fix. The hardcoded webhook token is removed, the graceful degradation (log warning + return False) is appropriate, and the implementation follows the file's established env var pattern. Rotate the webhook token in Discord after merging.


Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/dependencies.py` (modified) ### Findings #### Correctness - The implementation correctly replaces the hardcoded webhook URL with `os.environ.get("DISCORD_WEBHOOK_URL")` at module level, then references that constant in `send_webhook_message`. - The `if not webhook_url:` guard handles both `None` (env var unset) and `""` (empty string), which is correct — both cases should short-circuit with a warning. - `os` is already imported at line 5. `logger` is already defined at module level (line 15). No new imports needed. - The module-level constant placement (after `update_season_pitching_stats`, before `send_webhook_message`) is consistent with the existing pattern of `REDIS_HOST`, `REDIS_PORT`, `REDIS_DB`, and `CACHE_ENABLED` at lines 26–29. - The function's return behavior is unchanged for the success path — callers expecting `bool` are unaffected. #### Security - The hardcoded Discord webhook token is removed from source. This is the primary goal and it is achieved correctly. - The exposed token (`7RXG_D5IqovA3Jwa9YOobUjVcVMuLc6cQyezABcWuXaHo5Fvz1en10M7J43o3OJ3bzGW`) remains in git history. The PR body correctly calls out that the webhook should be rotated in Discord — that rotation is necessary regardless of this merge. - `DISCORD_WEBHOOK_URL` must be added to `docker-compose.prod.yml` and the dev server env for the function to work post-merge. This is an operational step, not a code issue. #### Style & Conventions - No issues. Follows existing file patterns for module-level env var reads. #### Suggestions - No blocking suggestions. The implementation is clean and minimal. ### Verdict: APPROVED This is a correct, well-scoped security fix. The hardcoded webhook token is removed, the graceful degradation (log warning + return False) is appropriate, and the implementation follows the file's established env var pattern. Rotate the webhook token in Discord after merging. --- *Automated review by Claude PR Reviewer*
Claude removed the
ai-reviewing
label 2026-03-27 04:21:00 +00:00
cal added 1 commit 2026-03-27 04:23:32 +00:00
Address PR review feedback for DISCORD_WEBHOOK_URL env var
All checks were successful
Build Docker Image / build (pull_request) Successful in 2m32s
1bcde424c6
- Add DISCORD_WEBHOOK_URL to docker-compose.yml api service environment block
- Add empty placeholder entry in .env for discoverability
- Move DISCORD_WEBHOOK_URL constant to the env-var constants section at top of dependencies.py

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
cal reviewed 2026-03-27 04:24:30 +00:00
cal left a comment
Author
Owner

AI Code Review

Files Reviewed

  • app/dependencies.py (modified)
  • docker-compose.yml (modified)
  • .env (modified)

Findings

Correctness

  • The hardcoded webhook URL (including the token) has been fully removed from send_webhook_message. The module-level constant DISCORD_WEBHOOK_URL is correctly read from the environment at import time, consistent with the pattern used by all other config constants in this file (REDIS_HOST, REDIS_PORT, API_TOKEN, etc.).
  • The early-return guard (if not webhook_url) correctly handles the unset case without crashing, returning False and logging a warning — matching the function's documented return contract.
  • No unused imports introduced. No logic regressions in the rest of the function.

Security

  • The previously exposed webhook token has been removed from source. The PR body correctly calls out that the old token must be rotated in Discord — this is the only remaining action item and it is an operational task, not a code issue.
  • The .env placeholder uses an empty value (DISCORD_WEBHOOK_URL=) which is the correct pattern for a development template: it documents the variable without supplying a real secret.

Style & Conventions

  • DISCORD_WEBHOOK_URL is placed at the top of the module's configuration constants block (line 26), above Redis config — correct position as requested in the prior review.
  • The # Discord integration section comment is consistent with the # Redis configuration comment style already present.
  • Warning message uses the em-dash style () consistent with other warning messages in the codebase.

Suggestions

  • No blocking suggestions. The implementation is minimal and correct.

Verdict: APPROVED

All three items requested in the prior review have been addressed: the env var constant was moved to the top of the file, docker-compose.yml now passes DISCORD_WEBHOOK_URL to the container, and .env has the placeholder entry. The code is clean, follows existing conventions, and handles the degraded (unset) case gracefully. Ready to merge — remember to rotate the compromised webhook token before deploying.


Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/dependencies.py` (modified) - `docker-compose.yml` (modified) - `.env` (modified) ### Findings #### Correctness - The hardcoded webhook URL (including the token) has been fully removed from `send_webhook_message`. The module-level constant `DISCORD_WEBHOOK_URL` is correctly read from the environment at import time, consistent with the pattern used by all other config constants in this file (`REDIS_HOST`, `REDIS_PORT`, `API_TOKEN`, etc.). - The early-return guard (`if not webhook_url`) correctly handles the unset case without crashing, returning `False` and logging a warning — matching the function's documented return contract. - No unused imports introduced. No logic regressions in the rest of the function. #### Security - The previously exposed webhook token has been removed from source. The PR body correctly calls out that the old token must be rotated in Discord — this is the only remaining action item and it is an operational task, not a code issue. - The `.env` placeholder uses an empty value (`DISCORD_WEBHOOK_URL=`) which is the correct pattern for a development template: it documents the variable without supplying a real secret. #### Style & Conventions - `DISCORD_WEBHOOK_URL` is placed at the top of the module's configuration constants block (line 26), above Redis config — correct position as requested in the prior review. - The `# Discord integration` section comment is consistent with the `# Redis configuration` comment style already present. - Warning message uses the em-dash style (`—`) consistent with other warning messages in the codebase. #### Suggestions - No blocking suggestions. The implementation is minimal and correct. ### Verdict: APPROVED All three items requested in the prior review have been addressed: the env var constant was moved to the top of the file, `docker-compose.yml` now passes `DISCORD_WEBHOOK_URL` to the container, and `.env` has the placeholder entry. The code is clean, follows existing conventions, and handles the degraded (unset) case gracefully. Ready to merge — remember to rotate the compromised webhook token before deploying. --- *Automated review by Claude PR Reviewer*
Claude added the
ai-reviewing
label 2026-03-27 04:47:17 +00:00
Claude reviewed 2026-03-27 04:48:27 +00:00
Claude left a comment
Collaborator

AI Code Review

Files Reviewed

  • app/dependencies.py (modified)
  • docker-compose.yml (modified)
  • .env (modified)

Findings

Correctness

  • Module-level DISCORD_WEBHOOK_URL = os.environ.get("DISCORD_WEBHOOK_URL") correctly follows the existing REDIS_HOST/PORT/DB pattern — evaluated at import time, consistent with all other env-var constants in this file.
  • webhook_url = DISCORD_WEBHOOK_URL + if not webhook_url: guard handles both None (unset) and "" (set to empty string), both falsy — correct graceful degradation.
  • docker-compose.yml updated to pass ${DISCORD_WEBHOOK_URL} from host into container — complete.
  • .env updated with empty placeholder under # Integrations comment — good documentation for developers setting up locally.

Security

  • Hardcoded webhook URL (including private token) removed from HEAD. ✓
  • The exposed webhook token remains in git history — PR body correctly flags this. The webhook should be rotated in Discord (delete old webhook, create new one). This is an operational follow-up, not a blocker.
  • Pre-existing: .env contains API_TOKEN=Tp3aO3jhYve5NJF1IqOmJTmk with an actual value — out of scope for this PR.

Style & Conventions

  • Pattern exactly mirrors existing REDIS_HOST, REDIS_PORT, etc. constants. No issues.
  • # Discord integration comment consistent with # Redis configuration style.

Suggestions

  • Operational: DISCORD_WEBHOOK_URL must be added to docker-compose.prod.yml on the prod server (ssh akamai) for the webhook to function post-deploy. This is expected since prod secrets aren't in the repo.

Verdict: APPROVED

Clean, complete fix. The implementation correctly removes the hardcoded credential and follows the existing env-var pattern. Graceful degradation (warn + return False) is the right behavior. All three changed files are consistent with each other.


Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/dependencies.py` (modified) - `docker-compose.yml` (modified) - `.env` (modified) ### Findings #### Correctness - Module-level `DISCORD_WEBHOOK_URL = os.environ.get("DISCORD_WEBHOOK_URL")` correctly follows the existing REDIS_HOST/PORT/DB pattern — evaluated at import time, consistent with all other env-var constants in this file. - `webhook_url = DISCORD_WEBHOOK_URL` + `if not webhook_url:` guard handles both `None` (unset) and `""` (set to empty string), both falsy — correct graceful degradation. - `docker-compose.yml` updated to pass `${DISCORD_WEBHOOK_URL}` from host into container — complete. - `.env` updated with empty placeholder under `# Integrations` comment — good documentation for developers setting up locally. #### Security - Hardcoded webhook URL (including private token) removed from HEAD. ✓ - The exposed webhook token remains in git history — PR body correctly flags this. The webhook should be rotated in Discord (delete old webhook, create new one). This is an operational follow-up, not a blocker. - Pre-existing: `.env` contains `API_TOKEN=Tp3aO3jhYve5NJF1IqOmJTmk` with an actual value — out of scope for this PR. #### Style & Conventions - Pattern exactly mirrors existing `REDIS_HOST`, `REDIS_PORT`, etc. constants. No issues. - `# Discord integration` comment consistent with `# Redis configuration` style. #### Suggestions - **Operational**: `DISCORD_WEBHOOK_URL` must be added to `docker-compose.prod.yml` on the prod server (`ssh akamai`) for the webhook to function post-deploy. This is expected since prod secrets aren't in the repo. ### Verdict: APPROVED Clean, complete fix. The implementation correctly removes the hardcoded credential and follows the existing env-var pattern. Graceful degradation (warn + return False) is the right behavior. All three changed files are consistent with each other. --- *Automated review by Claude PR Reviewer*
Claude added
ai-reviewed
and removed
ai-reviewing
labels 2026-03-27 04:48:34 +00:00
Claude approved these changes 2026-03-28 07:26:47 +00:00
Claude left a comment
Collaborator

All review feedback addressed. Approved.

All review feedback addressed. Approved.
cal added 1 commit 2026-03-28 07:27:17 +00:00
Merge remote-tracking branch 'origin/main' into fix/remove-hardcoded-webhook
All checks were successful
Build Docker Image / build (pull_request) Successful in 2m58s
b35b68a88f
All checks were successful
Build Docker Image / build (pull_request) Successful in 2m58s
This pull request is blocked because it's outdated.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin fix/remove-hardcoded-webhook:fix/remove-hardcoded-webhook
git checkout fix/remove-hardcoded-webhook
Sign in to join this conversation.
No reviewers
No Milestone
No project
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/major-domo-database#83
No description provided.