From 9e4456b7dffa4528be01bef149075a42c720190f Mon Sep 17 00:00:00 2001 From: waffle2k Date: Sun, 13 Sep 2026 17:41:48 -0700 Subject: [PATCH] Send the moderator DM after responding, not before The first real pre-block in production exposed this. Timeline from the logs: 00:24:54.53 Mastodon calls (signup form render) 00:24:55.33 ipapi.is answers (cold lookup, new range) 00:24:55.89 ip_blocks POST 200 <- block installed, 1.35s in 00:24:56.47 statuses POST 200 <- moderator DM, 582ms 00:24:56.53 Mastodon gives up at its 2.0s budget The block landed comfortably inside the budget and did its job -- the range is blocked and no account was created from it. But the answer never got back, because a moderator DM is a whole extra Mastodon round trip sitting on the synchronous response path. Nothing the caller acts on depends on that DM. It now goes out as a FastAPI BackgroundTask, which runs after the response is sent, so a cold lookup answers in ~1.35s instead of ~1.95s. Only classification and registering the block have to happen before we answer; telling a human can wait. reputation_check() takes a notify callable, defaulting to sending immediately so any non-HTTP caller keeps the old behaviour; the endpoint passes one that collects and defers. Tests assert the block is registered before the DM goes out, and that a DM that raises cannot stop a block being installed. --- app/main.py | 40 ++++++++++++++++++++++++++++++++-------- test_local.py | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 8 deletions(-) diff --git a/app/main.py b/app/main.py index 62d62a7..07e972d 100644 --- a/app/main.py +++ b/app/main.py @@ -1537,7 +1537,7 @@ def _external_lookup_allowed() -> bool: return True -def _reputation_ip(ip: str, source: str) -> dict | None: +def _reputation_ip(ip: str, source: str, notify) -> dict | None: """Classify one IP for the query API and install the block if it's flagged. Returns None when the IP side is switched off or the address doesn't parse. @@ -1587,7 +1587,7 @@ def _reputation_ip(ip: str, source: str) -> dict | None: log.warning("reputation query: pre-blocked ip=%s cidr=%s classification=%s org=%s source=%s", ip, cidr, classification, org, source or "-") if REPUTATION_API_DM_MODERATOR: - dm_moderator( + notify( f"🛡️ Pre-blocked {cidr} before signup — {classification} " f"({org or 'unknown org'})." ) @@ -1599,7 +1599,7 @@ def _reputation_ip(ip: str, source: str) -> dict | None: } -def _reputation_email(email: str, source: str) -> dict | None: +def _reputation_email(email: str, source: str, notify) -> dict | None: """Classify one signup email's *domain* for the query API and install the domain block if it's flagged. The full address is never stored or logged — same rule as the webhook path (see classify_email_domain). @@ -1640,7 +1640,7 @@ def _reputation_email(email: str, source: str) -> dict | None: log.warning("reputation query: pre-blocked domain=%s disposable=%s risk=%d source=%s", domain, is_disposable, risk, source or "-") if REPUTATION_API_DM_MODERATOR: - dm_moderator( + notify( f"🛡️ Pre-blocked email domain {domain} before signup " f"(disposable={is_disposable}, risk={risk})." ) @@ -1654,7 +1654,8 @@ def _reputation_email(email: str, source: str) -> dict | None: } -def reputation_check(ip: str = "", email: str = "", source: str = "") -> dict: +def reputation_check(ip: str = "", email: str = "", source: str = "", + notify=None) -> dict: """Classify an IP and/or email domain, install any block they earn, and report what happened. The write half of the bot's pre-emptive path. @@ -1668,10 +1669,19 @@ def reputation_check(ip: str = "", email: str = "", source: str = "") -> dict: as a result of this call", which is what the caller acts on: it is true both for a block installed just now and for one we installed earlier that is still in force. + + `notify` receives the moderator-alert text for each newly-installed block. + It defaults to sending immediately, but the HTTP endpoint passes one that + defers until after the response — a moderator DM is a whole extra Mastodon + round trip (measured at 582ms), and the caller is a live signup waiting on + a timeout. Only the classification and the block registration have to + happen before we answer; telling a human can wait. """ + if notify is None: + notify = dm_moderator result = { - "ip": _reputation_ip(ip.strip(), source), - "email": _reputation_email(email.strip(), source), + "ip": _reputation_ip(ip.strip(), source, notify), + "email": _reputation_email(email.strip(), source, notify), "checked_at": _now_iso(), } sides = [side for side in (result["ip"], result["email"]) if side] @@ -2031,6 +2041,7 @@ def _reputation_guard(key: str | None) -> None: @app.post("/api/reputation") def api_reputation( body: ReputationRequest, + background: BackgroundTasks, x_reputation_key: str | None = Header(default=None), ): """Look up an IP and/or email domain, record the answer, install whatever @@ -2040,11 +2051,24 @@ def api_reputation( Declared as a plain `def` on purpose: FastAPI runs those in a threadpool, so the blocking provider calls underneath never tie up the event loop and stall webhook deliveries. + + Moderator DMs are deferred to after the response. The caller is a live + signup on a ~2s budget, and a cold lookup already spends most of it on the + provider call plus registering the block; adding a Mastodon status POST on + top blew the budget in production the first time a real bad range showed + up (the block landed at 1.35s, the DM pushed the answer past 2.0s and the + caller timed out). Nothing the caller acts on depends on the DM. """ _reputation_guard(x_reputation_key) if not body.ip and not body.email: raise HTTPException(status_code=400, detail="ip or email is required") - return reputation_check(ip=body.ip, email=body.email, source=body.source) + + alerts: list[str] = [] + result = reputation_check(ip=body.ip, email=body.email, source=body.source, + notify=alerts.append) + for message in alerts: + background.add_task(dm_moderator, message) + return result @app.post("/webhook") diff --git a/test_local.py b/test_local.py index 31f61bd..0c3f077 100644 --- a/test_local.py +++ b/test_local.py @@ -1036,6 +1036,8 @@ def reputation_api_tests(): main.register_ip_block = ( lambda ip, acct, org, cidr="", comment="": ipblocks.append((ip, cidr, comment)) or True) + # TestClient runs BackgroundTasks to completion before returning, so the + # deferred moderator DMs are still observable in `dms` after a request. main.register_email_domain_block = ( lambda domain, acct: domain_blocks.append((domain, acct)) or True) @@ -1251,6 +1253,38 @@ def reputation_api_tests(): active = {e["cidr"] for e in client.get("/api/blocks/ip-ranges").json()["items"]} assert "203.0.113.0/24" not in active, "an expired block must not be published" + # N. The moderator DM is deferred until after the response. It costs a + # whole Mastodon round trip (582ms measured), and the caller is a live + # signup on a ~2s budget -- keeping it inline blew that budget the + # first time a real bad range showed up in production. + sent_order = [] + main.register_ip_block = ( + lambda ip, acct, org, cidr="", comment="": + sent_order.append("block") or ipblocks.append((ip, cidr, comment)) or True) + main.dm_moderator = lambda message: sent_order.append("dm") or dms.append(message) + classifications["198.51.103.7"] = ("datacenter", "Late DM Inc", True, "198.51.103.0/24") + + dms.clear() + r = ask({"ip": "198.51.103.7", "source": "defer-check"}) + assert r.json()["blocked"] is True, r.json() + assert sent_order == ["block", "dm"], \ + f"the block must be registered before the DM is sent, got {sent_order}" + assert any("198.51.103.0/24" in d for d in dms), "the DM still goes out, just later" + + # ...and the response body never depends on the DM having been sent. + main.dm_moderator = lambda message: (_ for _ in ()).throw( + RuntimeError("moderator DM must not be able to break a lookup")) + classifications["198.51.104.7"] = ("datacenter", "Broken DM Inc", True, "198.51.104.0/24") + try: + r = ask({"ip": "198.51.104.7", "source": "defer-check"}) + except RuntimeError: + # TestClient surfaces background-task errors; the important part is + # that the block was still installed and the answer computed first. + pass + assert main.reputation_block_installed("ip", "198.51.104.0/24"), \ + "a failing DM must not prevent the block being registered" + main.dm_moderator = lambda message: dms.append(message) + # Email domain blocks don't expire in Mastodon, so they carry none here # either: recorded with no expiry, they stay installed indefinitely. main.record_reputation_block("email", "rep-forever.test", "", "")