Reject webhook/destination URLs with embedded credentials (#79)

[richardmhope]

Summary

A webhook/destination URL with userinfo — https://user:pass@host/hook — passed validation at create time, but two things went wrong afterwards:

1. Silent credential loss at send. The IP-pinning rebuild in send_pinned_request (app/webhooks.py) replaces the whole netloc with the pinned IP, so the user:pass@ was dropped and the destination returned confusing 401s (the original report).
2. Plaintext credential storage + exposure. AlertDestination.target is persisted in the DB and returned by GET /api/alerts/destinations (and rendered in the account UI), so a credential embedded in the URL was stored in the clear and handed back over the API — the "breaks secret storage" problem.

Per the issue's own preferred option, this rejects userinfo rather than trying to preserve it. validate_webhook_url now raises a clear WebhookValidationError for any URL containing a username or password, which surfaces as a 422 at create/update and — because validation runs again inside send_pinned_request — is also refused at send time, for every sender kind (webhook/Slack/Teams/Jira/ServiceNow). This matches how the outbound-proxy URL already rejects userinfo (validate_proxy_settings); credentials belong in an env-only mechanism (a sender secret_ref, #37), never in the target URL.

Closes #79

Test evidence

tests/test_alerts.py tests/test_senders.py tests/test_proxy.py tests/test_code_review_medium.py
  -> 203 passed
ruff check app tests            -> All checks passed!
ruff format --check             -> ok
mypy app                        -> Success: no issues found in 60 source files

New coverage (tests/test_alerts.py):

• test_create_destination_rejects_embedded_credentials — API returns 422 for user:pass@, user@, and :pass@ forms, and nothing is stored (the listing never contains the target).
• test_validate_webhook_url_rejects_userinfo — direct unit contract that userinfo never validates, covering the send-time path shared by all kinds.

Checklist

• The four CI gates pass locally: pytest, ruff check + ruff format --check, mypy app, bandit -c pyproject.toml -r app
• Tests added for the new rejection (API + unit)
• CHANGELOG.md updated under the unreleased Security section
• Architecture/structure documented — the validate_webhook_url bullet in .claude/rules/architecture.md now records the userinfo rejection
• No user-facing behaviour beyond the rejection, so help.html needs no change
• No dependency change, so no uv lock needed
• No secrets, credentials, or internal hostnames in the diff or logs

---

Generated by Claude Code

[icebergai-review-bot]

IcebergAutoReview

Verdict: approve

The change correctly rejects credential-bearing webhook URLs through the shared validator used at create/update and send time. It is secure, scoped, documented, and adequately tested.

Findings

• No blocking findings.

Validation

• Inspected complete main...HEAD diff and affected route/sender context
• git diff --check main...HEAD passed
• Direct userinfo rejection probe passed, including percent-encoded credentials
• Targeted pytest could not start because the workspace prevents creating .venv; current-head CI is green

Residual risks / optional notes

• None identified.

Automated review by Codex 2d3d6d51c934 using IcebergAutoReview.