diff --git a/.env.example b/.env.example index dcac911..56b00a0 100644 --- a/.env.example +++ b/.env.example @@ -179,3 +179,22 @@ SUSPICIOUS_DRY_RUN=false # section. Without it the sweep 403s per-account (logged as an error, that # account is retried next sweep) but nothing else in the bot is affected. # Reuses ABUSE_SKIP_PRIVILEGED and ABUSE_ALLOWLIST above — no separate vars. + +# --- Combined-signal immediate suspend (app/main.py, process_signup) --- +# The sweep above acts on EITHER signal (IP or email) after +# SUSPICIOUS_GRACE_HOURS of inactivity. A signup flagged by BOTH signals at +# once is a stronger indicator, so it's auto-actioned immediately at signup +# time instead of waiting for the hourly sweep. Additive — doesn't change +# the OR-based sweep's handling of single-signal flags. Reuses +# ABUSE_ALLOWLIST above (staff can't be brand-new signups, but checked for +# defense in depth) — no separate allowlist var. +SUSPICIOUS_COMBINED_ENABLED=true + +# Moderation action taken immediately on a combined-signal signup: "suspend" +# (agreed default, same reasoning as SUSPICIOUS_ACTION) or "silence". +SUSPICIOUS_COMBINED_ACTION=suspend + +# Rollout safety switch — ships "true" (dry-run) here, unlike +# SUSPICIOUS_DRY_RUN's by-design "false" default above, since this is a +# brand-new action path. Flip to "false" once the moderator DMs look right. +SUSPICIOUS_COMBINED_DRY_RUN=true diff --git a/app/main.py b/app/main.py index d5596b6..3b0c972 100644 --- a/app/main.py +++ b/app/main.py @@ -183,6 +183,18 @@ SUSPICIOUS_ACTION = os.environ.get("SUSPICIOUS_ACTION", "suspend").lower() # this feature. Flip to "true" to pause without redeploying. SUSPICIOUS_DRY_RUN = os.environ.get("SUSPICIOUS_DRY_RUN", "false").lower() in ("1", "true", "yes") +# --- Combined-signal immediate suspend (both IP AND email flagged) --- +# The sweep above acts on EITHER signal after SUSPICIOUS_GRACE_HOURS of no +# activity. A signup flagged by BOTH signals at once is a stronger indicator +# than either alone, so it's acted on immediately in process_signup() instead +# of waiting for the hourly sweep — this is additive, it doesn't change the +# OR-based sweep's behavior for single-signal flags. +SUSPICIOUS_COMBINED_ENABLED = os.environ.get("SUSPICIOUS_COMBINED_ENABLED", "true").lower() in ("1", "true", "yes") +SUSPICIOUS_COMBINED_ACTION = os.environ.get("SUSPICIOUS_COMBINED_ACTION", "suspend").lower() +# Rollout safety switch — ships dry-run-first (unlike SUSPICIOUS_DRY_RUN's +# by-design "false" default above) since this is a brand-new action path. +SUSPICIOUS_COMBINED_DRY_RUN = os.environ.get("SUSPICIOUS_COMBINED_DRY_RUN", "true").lower() in ("1", "true", "yes") + if not WEBHOOK_SECRET: log.warning("WEBHOOK_SECRET is empty — signature verification will reject all requests.") if not BOT_ACCESS_TOKEN: @@ -601,9 +613,18 @@ def process_signup(account_id: str, acct: str, ip: str, email: str = "", Admin::Account.approved state at signup time: if it's already true (open registration, never queued), account.approved will never be delivered for it, so the hold is lifted immediately instead of waiting forever. + + A signup flagged by BOTH signals at once (see SUSPICIOUS_COMBINED_ENABLED) + is auto-actioned immediately here rather than falling through to the + hold/welcome path — no welcome, no suspicious-watch entry, since the + account is already suspended. This is additive to the existing hourly + sweep (app/suspicious_sweep.py), which still separately handles + single-signal flags after SUSPICIOUS_GRACE_HOURS of inactivity. """ reasons: list[str] = [] hold = False + ip_flagged = False + email_flagged = False if IP_SCRUTINY_ENABLED and ip: classification, org, ip_flagged = classify_signup_ip(ip) @@ -635,6 +656,37 @@ def process_signup(account_id: str, acct: str, ip: str, email: str = "", reasons.append(f"{prefix}email domain {domain} (disposable={is_disposable}, risk={risk})") hold = hold or (CHECK_MAIL_HOLD_WELCOME and not CHECK_MAIL_DRY_RUN) + if ip_flagged and email_flagged and SUSPICIOUS_COMBINED_ENABLED: + allowlisted = (acct.split("@")[0].lower() in ABUSE_ALLOWLIST + or acct.lower() in ABUSE_ALLOWLIST) + if not allowlisted: + note = ( + f"Auto-{SUSPICIOUS_COMBINED_ACTION} at signup: flagged by BOTH " + f"IP-scrutiny and email-domain scrutiny ({'; '.join(reasons)})." + ) + if SUSPICIOUS_COMBINED_DRY_RUN: + log.warning("[DRY-RUN] would %s acct=%s immediately (IP+email combined signal)", + SUSPICIOUS_COMBINED_ACTION, acct) + dm_moderator( + f"[DRY-RUN] would {SUSPICIOUS_COMBINED_ACTION} @{acct} immediately — " + f"flagged by BOTH IP and email signals ({'; '.join(reasons)})." + ) + else: + try: + apply_action(account_id, SUSPICIOUS_COMBINED_ACTION, note) + except httpx.HTTPError as exc: + log.error("acct=%s: failed to immediately %s on combined signal, " + "falling back to the normal flagged-signup path: %s", + acct, SUSPICIOUS_COMBINED_ACTION, exc) + else: + log.warning("auto-%sd acct=%s immediately — combined IP+email signup signal", + SUSPICIOUS_COMBINED_ACTION, acct) + dm_moderator( + f"🚨 Auto-{SUSPICIOUS_COMBINED_ACTION}d @{acct} immediately — " + f"flagged by BOTH IP and email signals ({'; '.join(reasons)})." + ) + return + if not reasons: send_welcome(account_id, acct) return diff --git a/test_local.py b/test_local.py index fc5d764..1c1209b 100644 --- a/test_local.py +++ b/test_local.py @@ -404,6 +404,96 @@ def email_scrutiny_tests(): assert domain_blocks == [], domain_blocks +def combined_signal_tests(): + """Drive process_signup's combined IP+email immediate-suspend path + (SUSPICIOUS_COMBINED_*), independent of the OR-based hourly sweep.""" + sent = [] + dms = [] + actions = [] + ipblocks = [] + domain_blocks = [] + main.send_welcome = lambda account_id, acct: sent.append((account_id, acct)) + main.dm_moderator = lambda message: dms.append(message) + main.apply_action = lambda target_id, action, text: actions.append((target_id, action)) + main.register_ip_block = lambda ip, acct, org: ipblocks.append((ip, acct, org)) + main.register_email_domain_block = lambda domain, acct: domain_blocks.append((domain, acct)) + main.fetch_account_counts = lambda account_id: (0, 0) + + main.IP_SCRUTINY_ENABLED = True + main.IP_SCRUTINY_DRY_RUN = False + main.IP_SCRUTINY_HOLD_WELCOME = True + main.IP_SCRUTINY_AUTO_IPBLOCK = True + main.CHECK_MAIL_ENABLED = True + main.CHECK_MAIL_API_KEY = "test-key" + main.CHECK_MAIL_DRY_RUN = False + main.CHECK_MAIL_HOLD_WELCOME = True + main.CHECK_MAIL_AUTO_DOMAIN_BLOCK = True + main.SUSPICIOUS_COMBINED_ENABLED = True + main.SUSPICIOUS_COMBINED_ACTION = "suspend" + main.SUSPICIOUS_COMBINED_DRY_RUN = False + main.ABUSE_ALLOWLIST = {"trustedstaff"} + + ip_classifications = { + "198.51.100.40": ("datacenter", "Example Cloud Hosting Inc", True), + "198.51.100.41": ("datacenter", "Example Cloud Hosting Inc", True), + } + main.classify_signup_ip = lambda ip: ip_classifications[ip] + email_classifications = { + "temp-mail.org": (True, 99), + "gmail.com": (False, 5), + } + main.classify_email_domain = lambda domain: email_classifications[domain] + + # A. both signals flagged, live -> suspended immediately, no welcome, no + # suspicious-watch entry (already actioned, nothing left to sweep). + # The individual ip/email auto-blocks still fire independently. + main.process_signup("501", "bothbad", "198.51.100.40", "bothbad@temp-mail.org") + assert ("501", "suspend") in actions, actions + assert ("501", "bothbad") not in sent, "must not welcome an immediately-suspended signup" + assert any("bothbad" in d and "BOTH" in d for d in dms), dms + with main._db() as conn: + row = conn.execute( + "SELECT 1 FROM suspicious_watch WHERE account_id = ?", ("501",), + ).fetchone() + assert row is None, "an immediately-suspended signup must not also enter the hourly watch" + assert ("198.51.100.40", "bothbad", "Example Cloud Hosting Inc") in ipblocks, ipblocks + assert ("temp-mail.org", "bothbad") in domain_blocks, domain_blocks + + # B. only IP flagged -> untouched by the combined path, falls through to + # the normal single-signal hold/welcome behavior. + actions.clear(); sent.clear(); dms.clear() + main.process_signup("502", "iponly", "198.51.100.41", "iponly@gmail.com") + assert ("502", "suspend") not in actions, actions + assert ("502", "iponly") not in sent, "single-signal flag should still be held" + + # C. both flagged, but allowlisted acct -> combined path skipped; falls + # through to the normal (still-held) path like any other flagged signup. + actions.clear(); sent.clear(); dms.clear() + main.process_signup("503", "trustedstaff", "198.51.100.40", "trustedstaff@temp-mail.org") + assert ("503", "suspend") not in actions, "allowlisted acct must not be auto-suspended" + + # D. both flagged, SUSPICIOUS_COMBINED_DRY_RUN -> DM only, no real + # suspend; falls through to the normal held-welcome path. + actions.clear(); sent.clear(); dms.clear() + main.SUSPICIOUS_COMBINED_DRY_RUN = True + main.process_signup("504", "drybothbad", "198.51.100.40", "drybothbad@temp-mail.org") + assert ("504", "suspend") not in actions, "dry-run must not call apply_action" + assert any("[DRY-RUN]" in d and "drybothbad" in d for d in dms), dms + assert ("504", "drybothbad") not in sent, "still held by the individual signal holds" + + # E. both flagged, live, but apply_action fails -> falls back to the + # normal held-welcome path instead of silently dropping the signup. + actions.clear(); sent.clear(); dms.clear() + main.SUSPICIOUS_COMBINED_DRY_RUN = False + + def boom(target_id, action, text): + raise main.httpx.HTTPError("boom") + + main.apply_action = boom + main.process_signup("505", "failsuspend", "198.51.100.40", "failsuspend@temp-mail.org") + assert ("505", "failsuspend") not in sent, "still held, falls back to the normal flagged path" + + def suspicious_watch_tests(): """Drive maybe_start_suspicious_watch: baseline capture on flagged signups only, never on clean ones, and never twice for the same account.""" @@ -553,6 +643,7 @@ if __name__ == "__main__": policy_tests() ip_scrutiny_tests() email_scrutiny_tests() + combined_signal_tests() suspicious_watch_tests() suspicious_sweep_tests() print("ALL TESTS PASSED")