feat: harden notification stack and switch logging selectors to icon grid
Notifications: - Add shared http_base, redact, and SSRF hardening modules - Refactor dispatcher, queue, receiver and per-provider clients (telegram, discord, email, matrix, ntfy, slack, webhook) to use the shared base, with bounded queue and redacted error logs - Tests for ssrf, redact, http_base, queue bounds, dispatcher aggregation, telegram media partition, email and matrix clients Frontend: - Settings: log level / log format selectors now use IconGridSelect with per-option icons and i18n descriptions - Minor providers page and entity-cache store updates Tooling: - Document code-review-graph MCP usage in CLAUDE.md - Ignore .code-review-graph/, register .mcp.json
This commit is contained in:
@@ -218,6 +218,19 @@ async def get_supported_locales(
|
||||
return locales or ["en"]
|
||||
|
||||
|
||||
@router.get("/external-url")
|
||||
async def get_external_url(
|
||||
user: User = Depends(get_current_user),
|
||||
session: AsyncSession = Depends(get_session),
|
||||
):
|
||||
"""Return the configured external base URL (available to all users).
|
||||
|
||||
Used by the UI to render absolute provider webhook URLs. Returns empty
|
||||
string when unset so the UI falls back to the relative path.
|
||||
"""
|
||||
return {"external_url": (await get_setting(session, "external_url")).rstrip("/")}
|
||||
|
||||
|
||||
async def _reregister_webhooks(
|
||||
session: AsyncSession, base_url: str, secret: str
|
||||
) -> None:
|
||||
|
||||
@@ -0,0 +1,46 @@
|
||||
"""Dispatcher result aggregation: per-receiver detail must survive."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from notify_bridge_core.notifications.dispatcher import NotificationDispatcher
|
||||
|
||||
|
||||
def test_aggregate_all_success() -> None:
|
||||
out = NotificationDispatcher._aggregate_results([
|
||||
{"success": True, "message_id": 1},
|
||||
{"success": True, "message_id": 2},
|
||||
])
|
||||
assert out["success"] is True
|
||||
assert out["receivers"] == 2
|
||||
assert out["successes"] == 2
|
||||
assert out["failures"] == 0
|
||||
|
||||
|
||||
def test_aggregate_partial() -> None:
|
||||
out = NotificationDispatcher._aggregate_results([
|
||||
{"success": True},
|
||||
{"success": False, "error": "boom"},
|
||||
])
|
||||
assert out["success"] is True # at least one succeeded
|
||||
assert out["successes"] == 1
|
||||
assert out["failures"] == 1
|
||||
assert "boom" in out["errors"]
|
||||
assert "results" in out
|
||||
|
||||
|
||||
def test_aggregate_all_fail_preserves_all_errors() -> None:
|
||||
out = NotificationDispatcher._aggregate_results([
|
||||
{"success": False, "error": "first"},
|
||||
{"success": False, "error": "second"},
|
||||
])
|
||||
assert out["success"] is False
|
||||
assert out["error"] == "first" # back-compat top-level field
|
||||
assert out["errors"] == ["first", "second"]
|
||||
# Per-receiver details survive — operator can see exactly what failed.
|
||||
assert len(out["results"]) == 2
|
||||
|
||||
|
||||
def test_aggregate_empty() -> None:
|
||||
out = NotificationDispatcher._aggregate_results([])
|
||||
assert out["success"] is False
|
||||
assert "error" in out
|
||||
@@ -0,0 +1,77 @@
|
||||
"""Email client header-injection / address-validation regression tests."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
|
||||
from notify_bridge_core.notifications.email.client import (
|
||||
EmailClient,
|
||||
SmtpConfig,
|
||||
_strip_header,
|
||||
_validate_email,
|
||||
_to_html,
|
||||
)
|
||||
|
||||
|
||||
def test_strip_header_removes_crlf() -> None:
|
||||
out = _strip_header("Subject\r\nBcc: attacker@example.com")
|
||||
assert "\r" not in out
|
||||
assert "\n" not in out
|
||||
# The injected "Bcc:" line is folded to a single header line; the SMTP
|
||||
# server will treat it as part of the subject text, not a header.
|
||||
assert "Bcc:" in out # value preserved as plain text
|
||||
|
||||
|
||||
def test_strip_header_removes_bare_lf() -> None:
|
||||
out = _strip_header("Hello\nWorld")
|
||||
assert "\n" not in out
|
||||
|
||||
|
||||
def test_strip_header_handles_non_string() -> None:
|
||||
assert _strip_header(None) == ""
|
||||
|
||||
|
||||
def test_validate_email_rejects_crlf() -> None:
|
||||
with pytest.raises(ValueError):
|
||||
_validate_email("user@example.com\r\nBcc: x@y")
|
||||
|
||||
|
||||
def test_validate_email_rejects_no_at() -> None:
|
||||
with pytest.raises(ValueError):
|
||||
_validate_email("not-an-email")
|
||||
|
||||
|
||||
def test_validate_email_rejects_empty() -> None:
|
||||
with pytest.raises(ValueError):
|
||||
_validate_email("")
|
||||
|
||||
|
||||
def test_validate_email_accepts_normal() -> None:
|
||||
assert _validate_email("user@example.com") == "user@example.com"
|
||||
|
||||
|
||||
def test_to_html_escapes_brackets() -> None:
|
||||
out = _to_html("<script>alert(1)</script>")
|
||||
assert "<script>" not in out
|
||||
assert "<script>" in out
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_returns_error_on_invalid_to() -> None:
|
||||
cfg = SmtpConfig(host="smtp.example.com", from_address="from@example.com")
|
||||
client = EmailClient(cfg)
|
||||
result = await client.send(
|
||||
to_email="user@example.com\r\nBcc: attacker@example.com",
|
||||
subject="hi",
|
||||
body_text="body",
|
||||
)
|
||||
assert result["success"] is False
|
||||
assert "Invalid email" in result["error"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_returns_error_on_no_host() -> None:
|
||||
cfg = SmtpConfig(host="", from_address="from@example.com")
|
||||
client = EmailClient(cfg)
|
||||
result = await client.send("u@x.com", "s", "b")
|
||||
assert result["success"] is False
|
||||
@@ -0,0 +1,53 @@
|
||||
"""HttpProviderClient + safe_headers tests."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
|
||||
from notify_bridge_core.notifications.http_base import safe_headers
|
||||
|
||||
|
||||
class TestSafeHeaders:
|
||||
def test_drops_hop_by_hop(self) -> None:
|
||||
out = safe_headers({
|
||||
"X-Custom": "ok",
|
||||
"Host": "evil.example.com",
|
||||
"Content-Length": "999",
|
||||
"Transfer-Encoding": "chunked",
|
||||
"Connection": "close",
|
||||
})
|
||||
assert out == {"X-Custom": "ok"}
|
||||
|
||||
def test_rejects_crlf_in_value(self) -> None:
|
||||
out = safe_headers({
|
||||
"X-Custom": "ok",
|
||||
"X-Bad": "value\r\nInjected: yes",
|
||||
})
|
||||
assert "X-Custom" in out
|
||||
assert "X-Bad" not in out
|
||||
|
||||
def test_rejects_crlf_in_name(self) -> None:
|
||||
out = safe_headers({
|
||||
"X-Custom": "ok",
|
||||
"X-Bad\r\nInject": "value",
|
||||
})
|
||||
assert out == {"X-Custom": "ok"}
|
||||
|
||||
def test_empty_input(self) -> None:
|
||||
assert safe_headers(None) == {}
|
||||
assert safe_headers({}) == {}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_http_base_returns_safe_error_on_invalid_url() -> None:
|
||||
"""An obviously-bad URL must not panic or leak the URL verbatim."""
|
||||
import aiohttp
|
||||
|
||||
from notify_bridge_core.notifications.http_base import HttpProviderClient
|
||||
|
||||
async with aiohttp.ClientSession() as sess:
|
||||
client = HttpProviderClient(sess, provider_name="test")
|
||||
# file:// is rejected by the SSRF guard before any HTTP call.
|
||||
result = await client.request("POST", "file:///etc/passwd", json={})
|
||||
assert result["success"] is False
|
||||
assert "Unsafe URL" in result["error"]
|
||||
@@ -0,0 +1,84 @@
|
||||
"""Matrix client validation: room_id format and quoting."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import aiohttp
|
||||
import pytest
|
||||
from aioresponses import aioresponses
|
||||
|
||||
from notify_bridge_core.notifications.matrix.client import MatrixClient
|
||||
|
||||
|
||||
HOMESERVER = "https://matrix.example.com"
|
||||
TOKEN = "secret-bearer-token-1234567890"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_rejects_path_injection_room_id() -> None:
|
||||
async with aiohttp.ClientSession() as sess:
|
||||
client = MatrixClient(sess, HOMESERVER, TOKEN)
|
||||
result = await client.send_message("!abc:host/../../etc/passwd", "hi")
|
||||
assert result["success"] is False
|
||||
assert "room_id" in result["error"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_rejects_empty_room_id() -> None:
|
||||
async with aiohttp.ClientSession() as sess:
|
||||
client = MatrixClient(sess, HOMESERVER, TOKEN)
|
||||
result = await client.send_message("", "hi")
|
||||
assert result["success"] is False
|
||||
assert "room_id" in result["error"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_rejects_unicode_control_chars_in_room_id() -> None:
|
||||
async with aiohttp.ClientSession() as sess:
|
||||
client = MatrixClient(sess, HOMESERVER, TOKEN)
|
||||
result = await client.send_message("!abc\x00:host", "hi")
|
||||
assert result["success"] is False
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_url_encodes_room_id_special_chars() -> None:
|
||||
"""``!`` and ``:`` must reach the server URL-encoded."""
|
||||
captured: list[str] = []
|
||||
|
||||
with aioresponses() as mocked:
|
||||
# Match any PUT under the rooms path; capture the URL we got.
|
||||
mocked.put(
|
||||
"https://matrix.example.com/_matrix/client/v3/rooms/%21abc%3Ahost.example/send/m.room.message",
|
||||
status=200, body='{}', repeat=True,
|
||||
)
|
||||
# aioresponses doesn't expose URL templates well, so use a regex mock.
|
||||
import re
|
||||
mocked.put(
|
||||
re.compile(r"https://matrix\.example\.com/_matrix/client/v3/rooms/[^/]+/send/m\.room\.message/.*"),
|
||||
status=200, body='{}', repeat=True,
|
||||
)
|
||||
|
||||
async with aiohttp.ClientSession() as sess:
|
||||
client = MatrixClient(sess, HOMESERVER, TOKEN)
|
||||
result = await client.send_message("!abc:host.example", "hi")
|
||||
|
||||
assert result["success"] is True
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_redacts_bearer_in_error() -> None:
|
||||
"""A 4xx response body must not echo the Authorization Bearer back to caller."""
|
||||
import re
|
||||
with aioresponses() as mocked:
|
||||
mocked.put(
|
||||
re.compile(r".*"),
|
||||
status=403,
|
||||
body='{"errcode": "M_FORBIDDEN", "Authorization": "Bearer ' + TOKEN + '"}',
|
||||
repeat=True,
|
||||
)
|
||||
|
||||
async with aiohttp.ClientSession() as sess:
|
||||
client = MatrixClient(sess, HOMESERVER, TOKEN)
|
||||
result = await client.send_message("!abc:host.example", "hi")
|
||||
|
||||
assert result["success"] is False
|
||||
assert TOKEN not in result["error"]
|
||||
@@ -0,0 +1,84 @@
|
||||
"""NotificationQueue bound + concurrency regression tests."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
from typing import Any
|
||||
|
||||
import pytest
|
||||
|
||||
from notify_bridge_core.notifications.queue import (
|
||||
DEFAULT_MAX_QUEUE_SIZE,
|
||||
NotificationQueue,
|
||||
)
|
||||
|
||||
|
||||
class _MemBackend:
|
||||
"""In-memory storage backend stub for tests."""
|
||||
|
||||
def __init__(self) -> None:
|
||||
self._data: dict[str, Any] | None = None
|
||||
|
||||
async def load(self) -> dict[str, Any] | None:
|
||||
return self._data
|
||||
|
||||
async def save(self, data: dict[str, Any]) -> None:
|
||||
self._data = data
|
||||
|
||||
async def remove(self) -> None:
|
||||
self._data = None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_load_with_garbage_falls_back_to_empty() -> None:
|
||||
backend = _MemBackend()
|
||||
backend._data = {"queue": "not-a-list"} # type: ignore[assignment]
|
||||
q = NotificationQueue(backend)
|
||||
await q.async_load()
|
||||
assert q.get_all() == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_enqueue_caps_at_max_size() -> None:
|
||||
backend = _MemBackend()
|
||||
q = NotificationQueue(backend, max_size=3)
|
||||
await q.async_load()
|
||||
for i in range(10):
|
||||
await q.async_enqueue({"i": i})
|
||||
items = q.get_all()
|
||||
assert len(items) == 3
|
||||
# FIFO drop: most recent three are kept (i=7..9).
|
||||
assert [it["params"]["i"] for it in items] == [7, 8, 9]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_all_returns_deep_copy() -> None:
|
||||
backend = _MemBackend()
|
||||
q = NotificationQueue(backend, max_size=10)
|
||||
await q.async_load()
|
||||
await q.async_enqueue({"key": "value"})
|
||||
snap = q.get_all()
|
||||
snap[0]["params"]["key"] = "MUTATED"
|
||||
snap2 = q.get_all()
|
||||
assert snap2[0]["params"]["key"] == "value"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_concurrent_enqueue_and_clear_no_corruption() -> None:
|
||||
backend = _MemBackend()
|
||||
q = NotificationQueue(backend, max_size=DEFAULT_MAX_QUEUE_SIZE)
|
||||
await q.async_load()
|
||||
|
||||
async def producer() -> None:
|
||||
for i in range(50):
|
||||
await q.async_enqueue({"i": i})
|
||||
|
||||
async def clearer() -> None:
|
||||
for _ in range(10):
|
||||
await asyncio.sleep(0)
|
||||
await q.async_clear()
|
||||
|
||||
await asyncio.gather(producer(), clearer())
|
||||
# No exceptions = no race-induced "dictionary changed size during iteration".
|
||||
items = q.get_all()
|
||||
assert isinstance(items, list)
|
||||
@@ -0,0 +1,74 @@
|
||||
"""Secret-redaction helper regression tests.
|
||||
|
||||
Locks in the patterns that surface from real provider error paths:
|
||||
Telegram bot URLs in aiohttp.ClientError messages, Authorization Bearer
|
||||
tokens in Matrix/ntfy responses, Discord/Slack webhook tokens, URL
|
||||
userinfo, and common ?token= query params.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
|
||||
from notify_bridge_core.notifications.redact import redact, redact_exc
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"raw,expected_substr,not_in",
|
||||
[
|
||||
(
|
||||
"Cannot connect to host api.telegram.org/bot1234567:AABBCC-secret-token/sendMessage",
|
||||
"api.telegram.org/bot***",
|
||||
"AABBCC-secret-token",
|
||||
),
|
||||
(
|
||||
"Authorization: Bearer ey.JhbGciOiJIUzI1NiJ9.payload.sig",
|
||||
"Bearer ***",
|
||||
"ey.JhbGciOiJIUzI1NiJ9",
|
||||
),
|
||||
(
|
||||
"POST https://discord.com/api/webhooks/12345/abcdefg-token failed",
|
||||
"discord.com/api/webhooks/12345/***",
|
||||
"abcdefg-token",
|
||||
),
|
||||
(
|
||||
"POST https://hooks.slack.com/services/T01/B02/zzzzz failed",
|
||||
"hooks.slack.com/services/T01/B02/***",
|
||||
"zzzzz",
|
||||
),
|
||||
(
|
||||
"fetch http://user:supersecret@example.com/foo",
|
||||
"http://***@example.com/foo",
|
||||
"supersecret",
|
||||
),
|
||||
(
|
||||
"https://api.example.com/x?token=mytoken123&extra=ok",
|
||||
"token=***",
|
||||
"mytoken123",
|
||||
),
|
||||
],
|
||||
)
|
||||
def test_redact_known_secrets(raw: str, expected_substr: str, not_in: str) -> None:
|
||||
out = redact(raw)
|
||||
assert expected_substr in out
|
||||
assert not_in not in out
|
||||
|
||||
|
||||
def test_redact_idempotent() -> None:
|
||||
once = redact("Bearer abcdefghij1234567890")
|
||||
twice = redact(once)
|
||||
assert once == twice
|
||||
|
||||
|
||||
def test_redact_exc_returns_str() -> None:
|
||||
err = RuntimeError("Bearer abcdefghij1234567890")
|
||||
out = redact_exc(err)
|
||||
assert isinstance(out, str)
|
||||
assert "Bearer ***" in out
|
||||
assert "abcdefghij1234567890" not in out
|
||||
|
||||
|
||||
def test_redact_handles_non_string() -> None:
|
||||
# Coercion path should not raise.
|
||||
out = redact(12345) # type: ignore[arg-type]
|
||||
assert out == "12345"
|
||||
@@ -0,0 +1,73 @@
|
||||
"""SSRF hardening regression tests.
|
||||
|
||||
Covers cases the original guard missed: IPv4-mapped IPv6, CGNAT,
|
||||
trailing-dot hostnames, IPv6 zone identifiers, and the safe-host repr
|
||||
used in error messages.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
|
||||
from notify_bridge_core.notifications.ssrf import (
|
||||
UnsafeURLError,
|
||||
PinnedResolver,
|
||||
avalidate_outbound_url_full,
|
||||
validate_outbound_url,
|
||||
)
|
||||
|
||||
|
||||
class TestBlockedRanges:
|
||||
@pytest.mark.parametrize(
|
||||
"url",
|
||||
[
|
||||
"http://[::ffff:127.0.0.1]/", # IPv4-mapped IPv6 → loopback
|
||||
"http://[::ffff:10.0.0.1]/", # IPv4-mapped IPv6 → RFC1918
|
||||
"http://100.64.0.1/", # CGNAT (RFC 6598)
|
||||
"http://0.0.0.0/", # unspecified
|
||||
],
|
||||
)
|
||||
def test_rejects_extra_ranges(self, url: str) -> None:
|
||||
with pytest.raises(UnsafeURLError):
|
||||
validate_outbound_url(url)
|
||||
|
||||
|
||||
class TestHostnameNormalization:
|
||||
def test_strips_trailing_dot(self) -> None:
|
||||
# ``localhost.`` should normalize to ``localhost`` and still resolve
|
||||
# to the loopback address — and be blocked.
|
||||
with pytest.raises(UnsafeURLError):
|
||||
validate_outbound_url("http://localhost./")
|
||||
|
||||
def test_rejects_bad_scheme_uppercase(self) -> None:
|
||||
with pytest.raises(UnsafeURLError):
|
||||
validate_outbound_url("FILE:///etc/passwd")
|
||||
|
||||
|
||||
class TestErrorMessages:
|
||||
def test_error_does_not_leak_long_hosts(self) -> None:
|
||||
with pytest.raises(UnsafeURLError) as ei:
|
||||
validate_outbound_url("http://" + "a" * 1024 + ".invalid/")
|
||||
# Truncated to 64 chars in the error string.
|
||||
assert len(str(ei.value)) < 256
|
||||
|
||||
|
||||
class TestPinnedResolverSync:
|
||||
def test_pin_returns_pinned_ip(self) -> None:
|
||||
resolver = PinnedResolver({"example.com": "93.184.216.34"})
|
||||
# Just exercise the dict path — full resolve runs in async tests.
|
||||
assert resolver._map["example.com"] == "93.184.216.34" # type: ignore[attr-defined]
|
||||
|
||||
|
||||
class TestAsyncFullValidator:
|
||||
@pytest.mark.asyncio
|
||||
async def test_returns_resolved_ip(self) -> None:
|
||||
# Literal IP — no DNS lookup; we still get back a ValidatedURL.
|
||||
result = await avalidate_outbound_url_full("http://8.8.8.8/")
|
||||
assert result.ip == "8.8.8.8"
|
||||
assert result.host == "8.8.8.8"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_rejects_blocked_literal(self) -> None:
|
||||
with pytest.raises(UnsafeURLError):
|
||||
await avalidate_outbound_url_full("http://[::ffff:127.0.0.1]/")
|
||||
@@ -0,0 +1,56 @@
|
||||
"""Telegram media-group mixed-type partitioning regression test.
|
||||
|
||||
Telegram rejects sendMediaGroup payloads that mix ``document`` with
|
||||
``photo``/``video``. The client must partition before chunking so a
|
||||
mixed input list still delivers all assets.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from notify_bridge_core.notifications.telegram.client import TelegramClient
|
||||
|
||||
|
||||
def test_partition_keeps_photo_video_together() -> None:
|
||||
parts = TelegramClient._partition_media_by_kind([
|
||||
{"type": "photo", "url": "p1"},
|
||||
{"type": "video", "url": "v1"},
|
||||
{"type": "photo", "url": "p2"},
|
||||
])
|
||||
assert len(parts) == 1
|
||||
assert [a["url"] for a in parts[0]] == ["p1", "v1", "p2"]
|
||||
|
||||
|
||||
def test_partition_separates_documents_from_media() -> None:
|
||||
parts = TelegramClient._partition_media_by_kind([
|
||||
{"type": "photo", "url": "p1"},
|
||||
{"type": "document", "url": "d1"},
|
||||
{"type": "video", "url": "v1"},
|
||||
])
|
||||
assert len(parts) == 3
|
||||
assert parts[0][0]["url"] == "p1"
|
||||
assert parts[1][0]["url"] == "d1"
|
||||
assert parts[2][0]["url"] == "v1"
|
||||
|
||||
|
||||
def test_partition_groups_consecutive_documents() -> None:
|
||||
parts = TelegramClient._partition_media_by_kind([
|
||||
{"type": "document", "url": "d1"},
|
||||
{"type": "document", "url": "d2"},
|
||||
{"type": "photo", "url": "p1"},
|
||||
])
|
||||
assert len(parts) == 2
|
||||
assert [a["url"] for a in parts[0]] == ["d1", "d2"]
|
||||
assert parts[1][0]["url"] == "p1"
|
||||
|
||||
|
||||
def test_partition_empty() -> None:
|
||||
assert TelegramClient._partition_media_by_kind([]) == []
|
||||
|
||||
|
||||
def test_partition_defaults_missing_type_to_photo() -> None:
|
||||
"""Items without an explicit type are treated as photos for grouping."""
|
||||
parts = TelegramClient._partition_media_by_kind([
|
||||
{"url": "x"}, # no type
|
||||
{"type": "video", "url": "v"},
|
||||
])
|
||||
assert len(parts) == 1
|
||||
Reference in New Issue
Block a user