tests: hard-block outbound transports, and assert the block holds
A test run could message real subscribers, and would have reported green while
doing it.
Every send gate in notifications/ reads its config from ambient env at import
time, and conftest neutralised none of it -- it set four env vars, none touching
Discord, SMTP or VAPID. test_notify.py calls run_pass() seven times without
patching notifications.discord, and notify.py:358 reaches
discord.post_subscription_alert() for every notification it creates. The
subscription channel notifies real people.
The failure mode is silent: notify.py's send path and discord.py's _bot_post
swallow every exception, so a real send raises nothing and no assertion goes red.
An operator who had sourced /etc/thermograph.env to debug -- routine -- turned
`pytest tests/notifications` into a live broadcast. CI was safe only by omission:
it runs in a container with no credentials baked in, and one added -e removed
that.
Two layers:
1. Config blanked, so every enabled()/dm_enabled()/subscription_enabled() gate
reports False. This is the state CI already runs in, now guaranteed locally
instead of depending on the shell.
2. The transports themselves replaced with sentinels that raise. A test that
forgets to patch one fails loudly rather than sending.
Tests that need to exercise a send path patch these themselves, which is the
point: the escape hatch becomes explicit in the test that takes it. The idiom is
lifted from tests/test_warm_content.py, which already did this for one call.
test_send_safety.py asserts the guarantee, because a block like this fails in
only one direction -- weaken it and every other test still passes, the sole
symptom being a message delivered to somebody.
Full suite: 457 passed, 8 skipped. Nothing relied on a live transport.
This commit is contained in:
parent
e377c4de03
commit
35006466bf
2 changed files with 171 additions and 0 deletions
|
|
@ -53,6 +53,69 @@ audit.ACTIVITY_DIR = os.path.join(_TMP, "logs", "activity")
|
|||
audit.HEARTBEAT_DIR = os.path.join(_TMP, "logs", "heartbeat")
|
||||
|
||||
|
||||
# --- outbound transports: hard-blocked, not merely unconfigured ---------------
|
||||
#
|
||||
# Every send gate in notifications/ is read from ambient env AT IMPORT TIME, and
|
||||
# every send site swallows its exceptions (notify.py's _flush_sends,
|
||||
# discord.py's _bot_post). So before this block existed, an operator who had
|
||||
# sourced /etc/thermograph.env in the shell — routine when debugging — turned
|
||||
# `pytest tests/notifications` into a live broadcast to real subscribers, and it
|
||||
# reported GREEN because the exceptions never surfaced. test_notify.py alone
|
||||
# calls run_pass() seven times without patching notifications.discord.
|
||||
#
|
||||
# CI was safe only by omission: it runs the suite in a container with no
|
||||
# credentials baked in. One added `-e` removed that.
|
||||
#
|
||||
# Two layers, deliberately:
|
||||
# 1. Blank the config so every enabled()/dm_enabled()/subscription_enabled()
|
||||
# gate reports False — the same state CI runs in, now guaranteed locally.
|
||||
# 2. Replace the actual transports with sentinels that RAISE. A test that
|
||||
# forgets to patch one now fails loudly instead of sending silently.
|
||||
#
|
||||
# Tests that need to exercise a send path patch these themselves, which is the
|
||||
# point: the escape hatch is explicit and visible in the test that takes it.
|
||||
# The idiom is borrowed from tests/test_warm_content.py.
|
||||
import smtplib # noqa: E402
|
||||
|
||||
import indexnow # noqa: E402
|
||||
from notifications import discord, mailer, push # noqa: E402
|
||||
|
||||
|
||||
class OutboundBlocked(AssertionError):
|
||||
"""Raised when a test reaches a real outbound transport.
|
||||
|
||||
Seeing this is not a flaky test — it means the code under test tried to
|
||||
contact Discord, an SMTP host, a push endpoint or IndexNow for real. Patch
|
||||
the transport in the test, or assert the no-send path.
|
||||
"""
|
||||
|
||||
|
||||
def _blocked(what):
|
||||
def _raise(*a, **kw):
|
||||
raise OutboundBlocked(
|
||||
f"test tried to reach {what} for real. Patch it in the test "
|
||||
f"(see tests/conftest.py's outbound-transport block).")
|
||||
return _raise
|
||||
|
||||
|
||||
# 1. Config blanked: every gate reports False.
|
||||
discord.BOT_TOKEN = ""
|
||||
discord.WEBHOOK_URL = ""
|
||||
discord.SUBSCRIPTION_CHANNEL_ID = ""
|
||||
discord.WEATHER_CHANNEL_ID = ""
|
||||
mailer.BACKEND = "console"
|
||||
|
||||
# 2. Transports replaced with raisers.
|
||||
discord._client.post = _blocked("Discord")
|
||||
discord._client.patch = _blocked("Discord")
|
||||
discord._client.put = _blocked("Discord")
|
||||
discord._client.delete = _blocked("Discord")
|
||||
indexnow._client.post = _blocked("IndexNow")
|
||||
push.webpush = _blocked("a Web Push endpoint")
|
||||
smtplib.SMTP = _blocked("an SMTP host")
|
||||
smtplib.SMTP_SSL = _blocked("an SMTP host")
|
||||
|
||||
|
||||
def _years_before(d: datetime.date, years: int) -> datetime.date:
|
||||
"""`d` shifted back `years` years, same month/day (Feb 29 → Feb 28)."""
|
||||
try:
|
||||
|
|
|
|||
108
backend/tests/notifications/test_send_safety.py
Normal file
108
backend/tests/notifications/test_send_safety.py
Normal file
|
|
@ -0,0 +1,108 @@
|
|||
"""The guarantee that a test run cannot reach a real subscriber.
|
||||
|
||||
tests/conftest.py hard-blocks every outbound transport. That block is worth
|
||||
nothing unless something asserts it, because it fails in exactly one direction:
|
||||
if a future edit weakens it, every other test still passes and the only symptom
|
||||
is a real Discord message, a real email, or a real push to somebody's phone —
|
||||
delivered silently, because notify.py and discord.py swallow send exceptions.
|
||||
|
||||
So this file tests the test harness. If anything here fails, do not weaken the
|
||||
assertion; fix the block in conftest.py.
|
||||
"""
|
||||
import smtplib
|
||||
|
||||
import pytest
|
||||
|
||||
import indexnow
|
||||
from notifications import discord, mailer, push
|
||||
from tests.conftest import OutboundBlocked
|
||||
|
||||
|
||||
# --- layer 1: every send gate reports "not configured" ------------------------
|
||||
|
||||
def test_no_discord_surface_is_enabled():
|
||||
"""A stray THERMOGRAPH_DISCORD_* in the operator's shell must not arm the
|
||||
bot. These four gates are what stand between run_pass() and a real post to
|
||||
the subscription channel, which notifies real people."""
|
||||
assert discord.enabled() is False
|
||||
assert discord.dm_enabled() is False
|
||||
assert discord.subscription_enabled() is False
|
||||
assert discord.weather_enabled() is False
|
||||
|
||||
|
||||
def test_no_discord_credentials_are_live():
|
||||
assert discord.BOT_TOKEN == ""
|
||||
assert discord.WEBHOOK_URL == ""
|
||||
assert discord.SUBSCRIPTION_CHANNEL_ID == ""
|
||||
assert discord.WEATHER_CHANNEL_ID == ""
|
||||
|
||||
|
||||
def test_mail_never_goes_over_smtp():
|
||||
"""`console` prints; `smtp` hands the message to a real host. digest.py
|
||||
branches on exactly this value."""
|
||||
assert mailer.BACKEND == "console"
|
||||
assert mailer.enabled() is False
|
||||
|
||||
|
||||
# --- layer 2: the transports themselves raise --------------------------------
|
||||
#
|
||||
# Layer 1 is config, and config can be re-enabled by a test that monkeypatches a
|
||||
# token to exercise a branch. These are the backstop for that case.
|
||||
|
||||
def test_discord_transport_raises_rather_than_posting():
|
||||
with pytest.raises(OutboundBlocked):
|
||||
discord._client.post("https://discord.com/api/v10/channels/1/messages",
|
||||
json={"content": "should never leave the box"})
|
||||
|
||||
|
||||
def test_indexnow_transport_raises_rather_than_posting():
|
||||
with pytest.raises(OutboundBlocked):
|
||||
indexnow._client.post("https://api.indexnow.org/indexnow", json={})
|
||||
|
||||
|
||||
def test_push_transport_raises_rather_than_sending():
|
||||
with pytest.raises(OutboundBlocked):
|
||||
push.webpush(subscription_info={}, data="{}")
|
||||
|
||||
|
||||
def test_smtp_raises_rather_than_connecting():
|
||||
with pytest.raises(OutboundBlocked):
|
||||
smtplib.SMTP("127.0.0.1", 25)
|
||||
with pytest.raises(OutboundBlocked):
|
||||
smtplib.SMTP_SSL("127.0.0.1", 465)
|
||||
|
||||
|
||||
# --- the paths that used to be able to send silently -------------------------
|
||||
|
||||
def test_bot_post_cannot_reach_discord_even_with_a_token(monkeypatch):
|
||||
"""_bot_post is the funnel for DMs and channel posts. Re-arm the token the
|
||||
way a careless test might and confirm the transport still refuses.
|
||||
|
||||
Note _bot_post swallows exceptions by design, so it returns None rather than
|
||||
raising — the assertion that matters is that the sentinel ran at all, i.e.
|
||||
that no HTTP request was attempted against the real API.
|
||||
"""
|
||||
monkeypatch.setattr(discord, "BOT_TOKEN", "not-a-real-token")
|
||||
attempted = []
|
||||
monkeypatch.setattr(discord._client, "post",
|
||||
lambda *a, **kw: attempted.append(a) or (_ for _ in ()).throw(
|
||||
OutboundBlocked("blocked")))
|
||||
assert discord._bot_post("/channels/1/messages", {"content": "x"}) is None
|
||||
assert attempted, "_bot_post should have gone through _client.post"
|
||||
|
||||
|
||||
def test_run_pass_sends_nothing_when_nothing_is_configured():
|
||||
"""The regression this whole block exists for: notify.run_pass() reaches
|
||||
discord.post_subscription_alert() for every notification it creates, and
|
||||
test_notify.py calls it seven times without patching discord. With the gates
|
||||
closed it must complete without touching a transport."""
|
||||
from accounts import db
|
||||
from notifications import notify
|
||||
|
||||
# The app lifespan (which normally creates these) does not run under
|
||||
# TestClient-less tests, so build the schema the way test_notify.py does.
|
||||
db.Base.metadata.create_all(db.sync_engine)
|
||||
|
||||
# No subscriptions in the throwaway DB => no candidates, no sends. The point
|
||||
# is that this completes without raising OutboundBlocked.
|
||||
notify.run_pass()
|
||||
Loading…
Reference in a new issue