From 35006466bf588f6da36052598841806d17c6dfd7 Mon Sep 17 00:00:00 2001 From: Emi Griffith Date: Sat, 25 Jul 2026 01:36:12 -0700 Subject: [PATCH] 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. --- backend/tests/conftest.py | 63 ++++++++++ .../tests/notifications/test_send_safety.py | 108 ++++++++++++++++++ 2 files changed, 171 insertions(+) create mode 100644 backend/tests/notifications/test_send_safety.py diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index f4f92fb..a4200da 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -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: diff --git a/backend/tests/notifications/test_send_safety.py b/backend/tests/notifications/test_send_safety.py new file mode 100644 index 0000000..24c5fee --- /dev/null +++ b/backend/tests/notifications/test_send_safety.py @@ -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()