cal/major-domo-database
1
0
Fork 0
Code Issues 25 Pull Requests 25 Actions Packages Projects Releases Wiki Activity

fix: remove hardcoded Discord webhook URL from dependencies.py (#19) #56

Open
cal wants to merge 1 commits from ai/major-domo-database-19 into next-release
pull from: ai/major-domo-database-19
merge into: cal:next-release
Conversation 0 Commits 1 Files Changed 1 +143 -106
cal commented 2026-03-06 02:02:26 +00:00
Owner
Copy Link

Summary

  • Removed hardcoded Discord webhook URL (including private token) from send_webhook_message() in app/dependencies.py:484
  • Replaced with os.environ.get("DISCORD_WEBHOOK_URL") — os was already imported
  • Added early-return guard: if the env var is unset, logs an error and returns False instead of crashing at request time

Files Changed

  • app/dependencies.py — line 484: hardcoded URL → env var read

Test Results

No test suite. Verified fix by reading back the modified file — the webhook URL literal is gone and the function correctly falls back to False when the env var is absent.

Other observations

  • The auto-formatter (ruff/black) reformatted the entire file on save — cosmetic whitespace/quote changes throughout the diff are formatter noise, not intentional edits.
  • DISCORD_WEBHOOK_URL will need to be added to docker-compose.prod.yml and the dev server env for send_webhook_message() to function.
## Summary - Removed hardcoded Discord webhook URL (including private token) from `send_webhook_message()` in `app/dependencies.py:484` - Replaced with `os.environ.get("DISCORD_WEBHOOK_URL")` — `os` was already imported - Added early-return guard: if the env var is unset, logs an error and returns `False` instead of crashing at request time ## Files Changed - `app/dependencies.py` — line 484: hardcoded URL → env var read ## Test Results No test suite. Verified fix by reading back the modified file — the webhook URL literal is gone and the function correctly falls back to `False` when the env var is absent. ## Other observations - The auto-formatter (ruff/black) reformatted the entire file on save — cosmetic whitespace/quote changes throughout the diff are formatter noise, not intentional edits. - `DISCORD_WEBHOOK_URL` will need to be added to `docker-compose.prod.yml` and the dev server env for `send_webhook_message()` to function.
cal added 1 commit 2026-03-06 02:02:27 +00:00
fix: remove hardcoded Discord webhook URL from dependencies.py (#19)
All checks were successful
Build Docker Image / build (pull_request) Successful in 2m14s
Details
900fda6e27 
Replace hardcoded Discord webhook URL (including secret token) in
send_webhook_message() with DISCORD_WEBHOOK_URL environment variable.
Returns False with an error log if the variable is not set.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
cal added the
ai-reviewing
 label 2026-03-06 02:15:36 +00:00
cal reviewed 2026-03-06 02:16:33 +00:00
cal left a comment
Author
Owner
Copy Link

AI Code Review

Files Reviewed

  • app/dependencies.py (modified)

Findings

Correctness

  • The fix is correct. os.environ.get("DISCORD_WEBHOOK_URL") replaces the hardcoded URL; os was already imported at module level.
  • The early-return guard (if not webhook_url: ... return False) correctly handles both None (env var unset) and "" (set to empty string) — both are falsy.
  • logger is module-level (logger = logging.getLogger("discord_app"), line 14) and in scope at the call site.
  • The return type bool remains consistent — all paths return True or False.
  • The try/except structure and the rest of send_webhook_message are unchanged.

Security

  • Fix confirmed: The hardcoded Discord webhook URL (including the private token 7RXG_D5IqovA3Jwa9YOobUjVcVMuLc6cQyezABcWuXaHo5Fvz1en10M7J43o3OJ3bzGW) is removed from HEAD. No credentials remain in the source.
  • Suggestion (non-blocking): The webhook token is still present in git history. You should invalidate/regenerate the webhook in Discord's developer portal — rotating it costs nothing and removes any exposure risk from the git log.

Style & Conventions

  • Bulk of the diff is ruff/black formatter noise (single→double quotes, trailing commas, line wrapping). No intentional logic changes elsewhere. Consistent with other PRs in this repo.

Suggestions

  • After merge, add DISCORD_WEBHOOK_URL to docker-compose.prod.yml and the dev server .env so send_webhook_message() is functional (noted in PR body — just confirming it's the right call).

Verdict: APPROVED

Clean security fix. The credential is correctly moved to an environment variable with an appropriate fail-fast guard. Rotate the Discord webhook token to fully close the exposure from git history.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/dependencies.py` (modified) ### Findings #### Correctness - The fix is correct. `os.environ.get("DISCORD_WEBHOOK_URL")` replaces the hardcoded URL; `os` was already imported at module level. - The early-return guard (`if not webhook_url: ... return False`) correctly handles both `None` (env var unset) and `""` (set to empty string) — both are falsy. - `logger` is module-level (`logger = logging.getLogger("discord_app")`, line 14) and in scope at the call site. - The return type `bool` remains consistent — all paths return `True` or `False`. - The try/except structure and the rest of `send_webhook_message` are unchanged. #### Security - **Fix confirmed**: The hardcoded Discord webhook URL (including the private token `7RXG_D5IqovA3Jwa9YOobUjVcVMuLc6cQyezABcWuXaHo5Fvz1en10M7J43o3OJ3bzGW`) is removed from HEAD. No credentials remain in the source. - **Suggestion (non-blocking)**: The webhook token is still present in git history. You should invalidate/regenerate the webhook in Discord's developer portal — rotating it costs nothing and removes any exposure risk from the git log. #### Style & Conventions - Bulk of the diff is ruff/black formatter noise (single→double quotes, trailing commas, line wrapping). No intentional logic changes elsewhere. Consistent with other PRs in this repo. #### Suggestions - After merge, add `DISCORD_WEBHOOK_URL` to `docker-compose.prod.yml` and the dev server `.env` so `send_webhook_message()` is functional (noted in PR body — just confirming it's the right call). ### Verdict: APPROVED Clean security fix. The credential is correctly moved to an environment variable with an appropriate fail-fast guard. Rotate the Discord webhook token to fully close the exposure from git history. --- *Automated review by Claude PR Reviewer*
cal added
ai-reviewed
 and removed
ai-reviewing
 labels 2026-03-06 02:16:56 +00:00
cal changed target branch from main to next-release 2026-03-07 07:32:47 +00:00
All checks were successful
Build Docker Image / build (pull_request) Successful in 2m14s
Details
This pull request has changes conflicting with the target branch.
  • app/dependencies.py
View command line instructions.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin ai/major-domo-database-19:ai/major-domo-database-19
git checkout ai/major-domo-database-19
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
ai-changes-requested

   
ai-pr-opened

   
ai-reviewed

   
ai-reviewing

   
ai-reviewing

   
ai-working
No Label
ai-changes-requested
ai-pr-opened
ai-reviewed
ai-reviewing
ai-reviewing
ai-working
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/major-domo-database#56
No description provided.
Delete Branch "ai/major-domo-database-19"

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?