feat(notifications): JSON-safe webhook template, SMTP timeout and implicit TLS, test-send buttons (#5064)
#5064 "Notifications that arrive": - render_webhook: alert.* is escaped to sit inside a JSON string (a quote or newline in a rule name no longer breaks the body); raw.* is the plain value for |tojson; a missing value reads "no data"; the template runs sandboxed and the body is checked to be JSON before it is sent. - SMTP: 15s timeout; port 465 connects with implicit TLS, otherwise the box means STARTTLS; a From address setting (falls back to the username). - A webhook error no longer carries the URL, which is a secret. - Settings → Notifications: "Send a test email" / "Send a test webhook" send a sample alert with the form's values (unsaved) and show the result and the body that went out; template help names alert.* and raw.*. - Notification audits go through audit_request; the test send is audited as settings.notification.tested, with the channel and outcome only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5.5
@@ -0,0 +1,152 @@
|
||||
"""Notification delivery (#5064): the webhook body is JSON whatever an alert
|
||||
holds, SMTP picks implicit TLS / STARTTLS / plain from port and box and never
|
||||
waits without a timeout, and a webhook error never carries the URL."""
|
||||
import json
|
||||
|
||||
import httpx
|
||||
import pytest
|
||||
|
||||
from steward.core import notifications as n
|
||||
|
||||
ALERT = {"rule_name": 'Disk "root" on\nweb01 \\ backslash', "state": "FIRING", "metric": "disk_used_pct",
|
||||
"value": 93.5, "threshold": 90.0, "resource": "web01", "source_module": "host_agent",
|
||||
"note": None, "timestamp": "2026-10-06T12:00:00+00:00"}
|
||||
|
||||
|
||||
# ── render_webhook ────────────────────────────────────────────────────────────
|
||||
|
||||
def test_alert_fields_are_escaped_into_json_strings():
|
||||
body = n.render_webhook('{"content": "{{ alert.state }} — {{ alert.rule_name }}"}', ALERT)
|
||||
assert json.loads(body) == {"content": 'FIRING — Disk "root" on\nweb01 \\ backslash'}
|
||||
|
||||
|
||||
def test_the_default_template_survives_quotes_in_a_name():
|
||||
from steward.core.settings import DEFAULTS
|
||||
body = json.loads(n.render_webhook(DEFAULTS["webhook.template"], ALERT))
|
||||
assert 'Disk "root" on\nweb01' in body["content"] and "93.5" in body["content"]
|
||||
|
||||
|
||||
def test_raw_fields_give_bare_json_values():
|
||||
tpl = '{"value": {{ raw.value | tojson }}, "name": {{ raw.rule_name | tojson }}}'
|
||||
assert json.loads(n.render_webhook(tpl, ALERT)) == {"value": 93.5, "name": ALERT["rule_name"]}
|
||||
assert json.loads(n.render_webhook(tpl, {**ALERT, "value": None}))["value"] is None
|
||||
|
||||
|
||||
def test_a_missing_value_reads_no_data():
|
||||
body = n.render_webhook('{"v": "{{ alert.value }}", "note": "{{ alert.note }}"}', {**ALERT, "value": None})
|
||||
assert json.loads(body) == {"v": "no data", "note": ""}
|
||||
|
||||
|
||||
@pytest.mark.parametrize("tpl, says", [
|
||||
('{"v": {{ alert.value }', "Template render failed"), # Jinja syntax error
|
||||
('{"v": {{ raw.rule_name }}}', "not valid JSON"), # unquoted string
|
||||
("{{ raw.__class__.__mro__ }}", "Template render failed"), # sandbox refuses
|
||||
])
|
||||
def test_a_bad_template_raises_valueerror_saying_why(tpl, says):
|
||||
with pytest.raises(ValueError, match=says):
|
||||
n.render_webhook(tpl, ALERT)
|
||||
|
||||
|
||||
# ── smtp_mode / _smtp_send_sync ───────────────────────────────────────────────
|
||||
|
||||
@pytest.mark.parametrize("port, tls, mode", [
|
||||
(465, True, "ssl"), (465, False, "ssl"), ("465", False, "ssl"),
|
||||
(587, True, "starttls"), (587, False, "plain"),
|
||||
(25, True, "starttls"), (25, False, "plain"),
|
||||
(None, True, "starttls"), (None, False, "plain"),
|
||||
])
|
||||
def test_smtp_mode(port, tls, mode):
|
||||
assert n.smtp_mode(port, tls) == mode
|
||||
|
||||
|
||||
class _FakeSMTP:
|
||||
made: list = []
|
||||
|
||||
def __init__(self, host, port, timeout=None, context=None):
|
||||
self.calls = [("connect", type(self).__name__, host, port, timeout)]
|
||||
_FakeSMTP.made.append(self)
|
||||
|
||||
def __enter__(self):
|
||||
return self
|
||||
|
||||
def __exit__(self, *exc):
|
||||
return False
|
||||
|
||||
def starttls(self, context=None):
|
||||
self.calls.append(("starttls",))
|
||||
|
||||
def login(self, user, password):
|
||||
self.calls.append(("login", user, password))
|
||||
|
||||
def send_message(self, msg):
|
||||
self.calls.append(("send", msg["From"]))
|
||||
|
||||
|
||||
class SMTP(_FakeSMTP):
|
||||
pass
|
||||
|
||||
|
||||
class SMTP_SSL(_FakeSMTP):
|
||||
pass
|
||||
|
||||
|
||||
@pytest.mark.parametrize("port, tls, expected", [
|
||||
(465, True, [("connect", "SMTP_SSL", "mx", 465, n.SMTP_TIMEOUT_S)]),
|
||||
(587, True, [("connect", "SMTP", "mx", 587, n.SMTP_TIMEOUT_S), ("starttls",)]),
|
||||
(25, False, [("connect", "SMTP", "mx", 25, n.SMTP_TIMEOUT_S)]),
|
||||
])
|
||||
def test_smtp_send_connects_by_mode_with_a_timeout(monkeypatch, port, tls, expected):
|
||||
monkeypatch.setattr(n.smtplib, "SMTP", SMTP)
|
||||
monkeypatch.setattr(n.smtplib, "SMTP_SSL", SMTP_SSL)
|
||||
_FakeSMTP.made = []
|
||||
msg = n.EmailMessage()
|
||||
msg["From"] = "steward@example.com"
|
||||
n._smtp_send_sync({"host": "mx", "port": port, "tls": tls, "username": "u", "password": "p"}, msg)
|
||||
(smtp,) = _FakeSMTP.made
|
||||
assert smtp.calls == expected + [("login", "u", "p"), ("send", "steward@example.com")]
|
||||
|
||||
|
||||
async def test_email_from_address_falls_back_to_username_then_localhost(monkeypatch):
|
||||
sent = []
|
||||
monkeypatch.setattr(n, "_smtp_send_sync", lambda cfg, msg: sent.append(msg["From"]))
|
||||
base = {"host": "mx", "port": 587, "recipients": ["a@example.com"]}
|
||||
for cfg in ({**base, "from": "alerts@example.com", "username": "u@example.com"},
|
||||
{**base, "from": "", "username": "u@example.com"},
|
||||
{**base, "from": "", "username": ""}):
|
||||
assert (await n._send_email(cfg, ALERT))["sent"]
|
||||
assert sent == ["alerts@example.com", "u@example.com", "steward@localhost"]
|
||||
|
||||
|
||||
def test_email_body_names_no_data_and_the_note():
|
||||
body = n.email_body({**ALERT, "value": None, "note": "no sample for 18m"})
|
||||
assert "Metric: disk_used_pct = no data\n" in body and "Note: no sample for 18m\n" in body
|
||||
assert "Note:" not in n.email_body(ALERT)
|
||||
|
||||
|
||||
# ── _send_webhook ─────────────────────────────────────────────────────────────
|
||||
|
||||
def _client_with(monkeypatch, handler):
|
||||
real = httpx.AsyncClient
|
||||
monkeypatch.setattr(n.httpx, "AsyncClient",
|
||||
lambda **kw: real(transport=httpx.MockTransport(handler), **kw))
|
||||
|
||||
|
||||
async def test_a_webhook_error_never_carries_the_url(monkeypatch):
|
||||
url = "https://discord.example/api/webhooks/123/s3cr3t-token"
|
||||
_client_with(monkeypatch, lambda req: httpx.Response(404))
|
||||
result = await n._send_webhook({"url": url, "template": ""}, ALERT)
|
||||
assert result["sent"] is False
|
||||
assert "s3cr3t-token" not in result["error"] and "<webhook URL>" in result["error"]
|
||||
|
||||
|
||||
async def test_a_webhook_posts_the_rendered_json(monkeypatch):
|
||||
seen = []
|
||||
|
||||
def handler(req):
|
||||
seen.append(json.loads(req.content))
|
||||
return httpx.Response(204)
|
||||
|
||||
_client_with(monkeypatch, handler)
|
||||
result = await n._send_webhook({"url": "https://hook.example/x", "template": ""}, ALERT)
|
||||
assert result == {"sent": True, "error": None}
|
||||
assert seen == [{"content": f'FIRING — {ALERT["rule_name"]}'}]
|
||||
@@ -0,0 +1,125 @@
|
||||
"""Integration: the test-send buttons on Settings → Notifications (#5064).
|
||||
|
||||
Each one sends a sample alert with the form's values, unsaved, and answers
|
||||
with what happened and the body that went out. The routes run for real
|
||||
against Postgres; only the delivery itself is replaced.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import json
|
||||
import os
|
||||
import time
|
||||
import uuid
|
||||
|
||||
import pytest
|
||||
|
||||
pytestmark = pytest.mark.integration
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def app():
|
||||
if not os.environ.get("STEWARD_DATABASE_URL"):
|
||||
pytest.skip("needs Postgres")
|
||||
from steward.app import create_app
|
||||
return create_app(testing=False)
|
||||
|
||||
|
||||
async def _admin_client(app):
|
||||
from steward.models.users import User, UserRole
|
||||
uid = str(uuid.uuid4())
|
||||
async with app.db_sessionmaker() as s:
|
||||
async with s.begin():
|
||||
s.add(User(id=uid, username=f"notify-{uid[:8]}", email=f"{uid}@test",
|
||||
password_hash="x", role=UserRole.admin, is_active=True))
|
||||
client = app.test_client()
|
||||
async with client.session_transaction() as sess:
|
||||
sess["user_id"] = uid
|
||||
sess["user_role"] = "admin"
|
||||
sess["username"] = f"notify-{uid[:8]}"
|
||||
sess["_csrf"] = "test-token"
|
||||
sess["login_at"] = time.time()
|
||||
return client, uid
|
||||
|
||||
|
||||
async def _audits(app, uid):
|
||||
from sqlalchemy import select
|
||||
from steward.models.audit import AuditEvent
|
||||
async with app.db_sessionmaker() as s:
|
||||
rows = (await s.execute(select(AuditEvent).where(AuditEvent.user_id == uid)
|
||||
.order_by(AuditEvent.timestamp))).scalars().all()
|
||||
return [(r.action, json.loads(r.detail_json)) for r in rows]
|
||||
|
||||
|
||||
def test_test_webhook_sends_the_form_template_and_shows_the_body(app, monkeypatch):
|
||||
from steward.core import notifications
|
||||
sent = []
|
||||
|
||||
async def fake_send(cfg, alert):
|
||||
sent.append((cfg, alert))
|
||||
return {"sent": False, "error": "Client error '404 Not Found' for url '<webhook URL>'"}
|
||||
|
||||
monkeypatch.setattr(notifications, "_send_webhook", fake_send)
|
||||
|
||||
async def _go():
|
||||
client, uid = await _admin_client(app)
|
||||
csrf = {"X-CSRF-Token": "test-token"}
|
||||
url = "https://hook.example/api/webhooks/1/s3cr3t"
|
||||
resp = await client.post("/settings/notifications/test-webhook", form={
|
||||
"webhook.url": url, "webhook.template": '{"text": "{{ alert.rule_name }}"}'}, headers=csrf)
|
||||
assert resp.status_code == 200
|
||||
page = await resp.get_data(as_text=True)
|
||||
assert "alert-error" in page and "404 Not Found" in page
|
||||
# The body shown is the one that would go out, and the quoted name kept it JSON.
|
||||
assert "Test alert \\"from\\" Steward" in page
|
||||
((cfg, alert),) = sent
|
||||
assert cfg["url"] == url and alert["rule_name"] == 'Test alert "from" Steward'
|
||||
|
||||
# A template that cannot make JSON is refused before anything is sent.
|
||||
resp = await client.post("/settings/notifications/test-webhook", form={
|
||||
"webhook.url": url, "webhook.template": '{"text": {{ raw.rule_name }}}'}, headers=csrf)
|
||||
page = await resp.get_data(as_text=True)
|
||||
assert "not valid JSON" in page and len(sent) == 1
|
||||
|
||||
audits = await _audits(app, uid)
|
||||
assert [a for a, _ in audits] == ["settings.notification.tested"] * 2
|
||||
assert all(d == {"channel": "webhook", "sent": False} for _, d in audits)
|
||||
assert "s3cr3t" not in json.dumps([d for _, d in audits])
|
||||
|
||||
asyncio.run(_go())
|
||||
|
||||
|
||||
def test_test_email_uses_the_form_settings_and_names_the_tls_mode(app, monkeypatch):
|
||||
from steward.core import notifications
|
||||
sent = []
|
||||
|
||||
async def fake_send(cfg, alert):
|
||||
sent.append(cfg)
|
||||
return {"sent": True, "error": None}
|
||||
|
||||
monkeypatch.setattr(notifications, "_send_email", fake_send)
|
||||
|
||||
async def _go():
|
||||
client, uid = await _admin_client(app)
|
||||
csrf = {"X-CSRF-Token": "test-token"}
|
||||
resp = await client.post("/settings/notifications/test-email", form={
|
||||
"smtp.host": "mx.example", "smtp.port": "465", "smtp.username": "u",
|
||||
"smtp.password": "pw", "smtp.from": "alerts@example.com",
|
||||
"smtp.recipients": "a@example.com, b@example.com"}, headers=csrf)
|
||||
page = await resp.get_data(as_text=True)
|
||||
assert "alert-success" in page
|
||||
assert "Sent to a@example.com, b@example.com via mx.example:465 (implicit TLS)." in page
|
||||
assert "Rule: Test alert" in page
|
||||
assert sent == [{"host": "mx.example", "port": 465, "tls": False, "username": "u",
|
||||
"password": "pw", "from": "alerts@example.com",
|
||||
"recipients": ["a@example.com", "b@example.com"]}]
|
||||
|
||||
resp = await client.post("/settings/notifications/test-email", form={
|
||||
"smtp.host": "", "smtp.port": "587", "smtp.recipients": ""}, headers=csrf)
|
||||
assert "Enter an SMTP host and at least one recipient." in await resp.get_data(as_text=True)
|
||||
assert len(sent) == 1
|
||||
|
||||
assert [d for _, d in await _audits(app, uid)] == [
|
||||
{"channel": "email", "sent": True}, {"channel": "email", "sent": False}]
|
||||
|
||||
asyncio.run(_go())
|
||||
Reference in New Issue
Block a user