Suspend immediately when both IP and email signals flag a signup
docker-build-push / build-push (push) Successful in 21s
docker-build-push / build-push (push) Successful in 21s
The hourly OR-based sweep (SUSPICIOUS_GRACE_HOURS) still handles a single-signal flag as before. This adds an additional check in process_signup(): a signup flagged by BOTH IP-scrutiny and email-domain scrutiny at once is a stronger signal, so it's suspended right away instead of waiting out the grace period, skipping the hold/welcome path and the suspicious_watch entry entirely. Gated behind new SUSPICIOUS_COMBINED_* env vars, dry-run first per the usual rollout convention (unlike SUSPICIOUS_DRY_RUN, which shipped live by design). Falls back to the normal held-welcome path if the suspend API call fails, and respects ABUSE_ALLOWLIST.
This commit is contained in:
@@ -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
|
||||
|
||||
+52
@@ -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
|
||||
|
||||
@@ -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", "[email protected]")
|
||||
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", "[email protected]")
|
||||
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", "[email protected]")
|
||||
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", "[email protected]")
|
||||
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", "[email protected]")
|
||||
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")
|
||||
|
||||
Reference in New Issue
Block a user