[Hardening] Validate campaign discord_webhook_url against a Discord host allowlist (SSRF) #97
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
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?
Context
Stored Discord webhook URLs are POSTed server-side from the Celery worker, so any non-Discord URL stored anywhere in the system is an SSRF primitive against the deployment's internal network (cloud metadata at 169.254.169.254, localhost ports, Redis/Postgres siblings, etc.).
Write-time validation exists for the campaign path:
_validate_webhook(webapp/backend/app/schemas/campaign.py:8-20) requires thehttps://discord.com/api/webhooks/prefix and is applied viafield_validatoron bothCampaignCreate(:60) andCampaignUpdate(:98), socampaign_service.py:35receives validated data. But coverage is incomplete:PUT /admin/settings/notifications(webapp/backend/app/routers/users.py:352-357) writesdata.discord_webhook_urlstraight into settings;NotificationSettingsRequest(users.py:52) has no validator. This URL is used for every campaign without its own webhook via_effective_webhook(webapp/backend/app/services/session_service.py:28-32). Unlike the admin Whisper/LLM/bot URLs, it never passesnormalize_service_url(users.py:434/446/465use it; the webhook write at:352does not).webapp/backend/app/notifications/discord.py:91-100(_post) andwebapp/backend/app/tasks/reminder_tasks.py:874(vote notification). Campaign rows written before the schema validator was introduced (or via any future path that bypasses the schema) are sent blind.Fix / Spec
webapp/backend/app/services/url_policy_service.py): parse withurllib.parse.urlsplit; require schemehttps, host in{discord.com, discordapp.com, ptb.discord.com, canary.discord.com}, and path starting with/api/webhooks/. Use real URL parsing, not string prefix matching, once multiple hosts are allowed._validate_webhookinschemas/campaign.pywith a call to the shared validator (behavioral change: the additional Discord hosts become accepted).field_validatoronNotificationSettingsRequest.discord_webhook_url(users.py:52) or a check in the handler beforeset_setting(users.py:352-357); return 422/400 with a clear message.discord.py._post(:91) and atreminder_tasks.py:874. On failure: log a clear warning identifying the campaign and skip the send — do not crash the task.Acceptance criteria
http://169.254.169.254/...,https://localhost:6379/...,https://evil.example/api/webhooks/x, andhttps://discord.com.evil.example/api/webhooks/xon: campaign create, campaign update, admin notification settings.https://discord.com/api/webhooks/<id>/<token>URL (and one alternate allowed host).References
webapp/backend/app/schemas/campaign.py:8-20,:60,:98(existing campaign validator)webapp/backend/app/routers/users.py:52,:352-357(unvalidated admin fallback write)webapp/backend/app/services/session_service.py:28-32(_effective_webhookfallback resolution)webapp/backend/app/notifications/discord.py:91-100,webapp/backend/app/tasks/reminder_tasks.py:874(send sites)webapp/backend/app/services/url_policy_service.py(existing URL policy home)Filed from the July 2026 full-project review.
Picking this up as part of a v3.3.0 push. Landing on branch
hardening/backendtogether with #88, #90, #106, and #109 (grouped by component to keep the diffs reviewable).Fixed on
main(commit01b3a13, merged via1c9c19f). One shared validator inurl_policy_service.py(urlsplit-based — real parsing, not prefix match): requireshttps, exact host in{discord.com, discordapp.com, ptb.discord.com, canary.discord.com}, no embedded credentials, no off-port target, path prefix/api/webhooks/. Applied at campaign create + update (_validate_webhook) and onNotificationSettingsRequest.discord_webhook_url(empty string preserved so the admin UI can still clear the fallback). Send-time revalidation added indiscord.py._postandsend_vote_notification— a non-Discord URL already in the DB is logged (with campaign + host) and skipped: no outbound request, no task crash. Tests (44) cover all four rejects incl. thehttps://discord.com.evil.example/...suffix trap, on create/update/admin-settings, acceptance of a real URL + one alternate host, and the legacy-row logged-skip. Backend suite green (358 passed), ruff clean.