diff --git a/backend/tests/plugins/test_antispam_corpus.py b/backend/tests/plugins/test_antispam_corpus.py new file mode 100644 index 0000000..2b05657 --- /dev/null +++ b/backend/tests/plugins/test_antispam_corpus.py @@ -0,0 +1,167 @@ +"""SPEC §C.8 acceptance 4 — the learning loop actually learns. + +"After 20 spam + 20 ham teachings on a fixture corpus, a held-out obvious-spam +fixture scores above move_threshold for that user and a ham fixture below +banner_threshold (per-user isolation: a second user's scores are unaffected)." + +This is the only test that proves the loop *works* rather than merely runs, so it +talks to a real Rspamd. It skips when there is no engine — on a machine without +one that is a missing check, not a failure. + +**Deviation from the literal wording, recorded in docs/open-questions.md OQ-8.** +The held-out spam does not cross move_threshold (12.0) and cannot: a synthetic +fixture has no delivery history, and this deployment deliberately zeroes the +SMTP-time symbols that would otherwise pile score onto exactly such a message +(DATE_IN_PAST, HFILTER_HOSTNAME_UNKNOWN, ARC_NA, DMARC_NA...). Those were the +false-positive engine on real mail. Asserting 12.0 here would mean either +un-tuning the filter or writing fixtures that fake delivery headers. + +What is asserted instead is the substance the criterion exists to check: after +training, Bayes recognises held-out spam, held-out ham stays under the banner +threshold, and the same message scores materially higher for the user who taught +than for one who did not. +""" + +from __future__ import annotations + +import uuid + +import pytest +from webmail_plugin_antispam import policy, rspamd + +pytestmark = pytest.mark.asyncio + +SETTINGS = policy.Settings( + enabled_accounts=[], + move_threshold=12.0, + banner_threshold=6.0, + network_checks=False, + learn_from_provider_junk=True, +) + +_SPAM_PHRASES = [ + "CONGRATULATIONS! You have WON the international lottery jackpot", + "Urgent business proposal: transfer of $45,000,000 USD to your account", + "Cheap pills online, no prescription needed, discreet worldwide shipping", + "Your account will be SUSPENDED unless you verify your password immediately", + "Work from home and earn $5000 per week with this one weird trick", + "Hot singles in your area are waiting to meet you tonight", + "Claim your free iPhone now, limited offer, click here immediately", + "I am the widow of a late oil magnate seeking a trustworthy partner", + "Bitcoin investment doubling scheme, guaranteed returns, act now", + "FINAL NOTICE: your payment is overdue, wire funds within 24 hours", +] + +_HAM_PHRASES = [ + "Attached are the notes from this morning's planning meeting", + "Could you review the deployment checklist before Thursday please", + "The invoice for last month's hosting is ready for your approval", + "Thanks for sending the render over, the lighting looks much better", + "Reminder: the project retrospective is at 14:00 in the small room", + "I have pushed the fix to the branch, it needs a second pair of eyes", + "Here is the schedule for next week's shoot, let me know about Tuesday", + "The client approved the storyboard, we can start animation on Monday", + "Quick question about the colour grade on shot 42, do you have a minute", + "Please find the signed contract attached, scanned as a PDF", +] + + +def _message(subject: str, body: str, n: int) -> bytes: + return ( + f"From: sender{n}@example.test\r\n" + f"To: recipient@example.test\r\n" + f"Subject: {subject}\r\n" + f"Message-ID: \r\n" + "\r\n" + f"{body}\r\n" + ).encode() + + +def _corpus(phrases: list[str], count: int) -> list[bytes]: + """`count` messages built from the phrase pool, each worded differently.""" + out = [] + for i in range(count): + a, b = phrases[i % len(phrases)], phrases[(i + 3) % len(phrases)] + out.append(_message(a, f"{a}. {b}. Reference {i}.", i)) + return out + + +async def _engine_available() -> bool: + return await rspamd.check(_message("ping", "hello", 0), user_key="probe") is not None + + +@pytest.fixture +async def taught_user() -> str: + """A user whose Bayes has seen 20 spam and 20 ham, and nobody else's.""" + if not await _engine_available(): + pytest.skip("no Rspamd engine reachable (RSPAMD_URL)") + user = f"corpus-{uuid.uuid4().hex[:12]}" + for raw in _corpus(_SPAM_PHRASES, 20): + assert await rspamd.learn(raw, user_key=user, spam=True) + for raw in _corpus(_HAM_PHRASES, 20): + assert await rspamd.learn(raw, user_key=user, spam=False) + return user + + +async def _score(raw: bytes, user: str) -> float: + verdict = await rspamd.check(raw, user_key=user, network_checks=False) + assert verdict is not None, "engine went away mid-test" + return verdict.score + + +# Held out: never taught, but plainly of each kind. +HELD_OUT_SPAM = _message( + "WINNER! Claim your lottery prize money now", + "Congratulations, you have won the jackpot. Urgent: wire your bank details " + "immediately to claim $10,000,000 USD. Free pills included, act now.", + 999, +) +HELD_OUT_HAM = _message( + "Notes from the planning meeting", + "Attached are the notes from this morning. Could you review the checklist " + "before Thursday? The invoice is ready for approval when you have a minute.", + 998, +) + + +async def _symbols(raw: bytes, user: str) -> list[str]: + verdict = await rspamd.check(raw, user_key=user, network_checks=False) + assert verdict is not None, "engine went away mid-test" + return verdict.symbols + + +async def test_training_makes_bayes_recognise_held_out_spam(taught_user: str) -> None: + """The classifier generalises: it has never seen this message.""" + assert "BAYES_SPAM" in await _symbols(HELD_OUT_SPAM, taught_user) + + +async def test_training_does_not_make_bayes_cry_spam_at_ham(taught_user: str) -> None: + assert "BAYES_SPAM" not in await _symbols(HELD_OUT_HAM, taught_user) + + +async def test_held_out_ham_stays_below_the_banner_threshold(taught_user: str) -> None: + score = await _score(HELD_OUT_HAM, taught_user) + assert score < SETTINGS.banner_threshold, ( + f"taught ham corpus, held-out ham scored {score} " + f"(needs < {SETTINGS.banner_threshold} to pass without a warning)" + ) + + +async def test_one_users_training_does_not_move_anothers_scores(taught_user: str) -> None: + """Per-user isolation — the reason Deliver-To is sent on every scan.""" + stranger = f"untaught-{uuid.uuid4().hex[:12]}" + taught_score = await _score(HELD_OUT_SPAM, taught_user) + stranger_score = await _score(HELD_OUT_SPAM, stranger) + assert "BAYES_SPAM" not in await _symbols(HELD_OUT_SPAM, stranger), ( + "an untaught user sees BAYES_SPAM: training is leaking between users" + ) + assert stranger_score < taught_score, ( + f"untaught user scored {stranger_score} vs the taught user's {taught_score}" + ) + + +async def test_spam_and_ham_are_separated_by_training(taught_user: str) -> None: + """The gap is the point: a threshold anywhere between them files correctly.""" + spam_score = await _score(HELD_OUT_SPAM, taught_user) + ham_score = await _score(HELD_OUT_HAM, taught_user) + assert spam_score > ham_score, f"spam {spam_score} did not outscore ham {ham_score}" diff --git a/backend/tests/plugins/test_antispam_learning.py b/backend/tests/plugins/test_antispam_learning.py index c239b67..bfb7015 100644 --- a/backend/tests/plugins/test_antispam_learning.py +++ b/backend/tests/plugins/test_antispam_learning.py @@ -188,3 +188,95 @@ async def test_bootstrap_respects_the_setting(monkeypatch: pytest.MonkeyPatch) - monkeypatch.setattr(jobs, "_SDK", _Fake()) await jobs.bootstrap_account({}, user_id=str(uuid.uuid4()), account_id=str(uuid.uuid4())) assert sampled == [] + + +async def test_bootstrap_refuses_a_lopsided_corpus(monkeypatch: pytest.MonkeyPatch) -> None: + """Three spam beside a thousand ham would teach "everything is ham". + + Accounts do not sync the provider's spam folder by default, so this is the + common case, not an edge case. + """ + taught: list[bool] = [] + + async def _settings(_u: object) -> policy.Settings: + return SETTINGS + + class _Kv: + async def get(self, _k: str, _d: object = None) -> list[str]: + return [] + + async def set(self, _k: str, _v: object) -> None: + raise AssertionError("must not mark the account done") + + class _M: + async def sample(self, *_a: object, **kw: object) -> list[uuid.UUID]: + n = 3 if kw.get("special_use") == "junk" else 1000 + return [uuid.uuid4() for _ in range(n)] + + async def raw(self, *_a: object, **_k: object) -> bytes: + return b"x" + + class _Fake: + messages = _M() + + def kv(self, _u: object) -> _Kv: + return _Kv() + + async def _learn(_raw: bytes, **kw: object) -> bool: + taught.append(bool(kw.get("spam"))) + return True + + monkeypatch.setattr(policy, "load_settings", _settings) + monkeypatch.setattr(jobs, "_SDK", _Fake()) + monkeypatch.setattr(jobs.rspamd, "learn", _learn) + + await jobs.bootstrap_account({}, user_id=str(uuid.uuid4()), account_id=str(uuid.uuid4())) + assert taught == [], "taught from a corpus too lopsided to learn from" + + +async def test_bootstrap_caps_ham_at_a_multiple_of_spam(monkeypatch: pytest.MonkeyPatch) -> None: + marked: list[object] = [] + + async def _settings(_u: object) -> policy.Settings: + return SETTINGS + + class _Kv: + async def get(self, _k: str, _d: object = None) -> list[str]: + return [] + + async def set(self, _k: str, v: object) -> None: + marked.append(v) + + class _M: + async def sample(self, *_a: object, **kw: object) -> list[uuid.UUID]: + n = 20 if kw.get("special_use") == "junk" else 1000 + return [uuid.uuid4() for _ in range(n)] + + async def raw(self, *_a: object, **_k: object) -> bytes: + return b"x" + + class _Fake: + messages = _M() + published: list[object] = [] + + def kv(self, _u: object) -> _Kv: + return _Kv() + + async def publish(self, _e: str, _p: object) -> None: + return None + + taught: list[bool] = [] + + async def _learn(_raw: bytes, **kw: object) -> bool: + taught.append(bool(kw.get("spam"))) + return True + + monkeypatch.setattr(policy, "load_settings", _settings) + monkeypatch.setattr(jobs, "_SDK", _Fake()) + monkeypatch.setattr(jobs.rspamd, "learn", _learn) + monkeypatch.setattr(jobs, "PAUSE_SECONDS", 0) + + await jobs.bootstrap_account({}, user_id=str(uuid.uuid4()), account_id=str(uuid.uuid4())) + assert taught.count(True) == 20 + assert taught.count(False) == 60, "ham should be capped at 3x the spam count" + assert marked, "a completed bootstrap must mark the account done" diff --git a/docker/rspamd/local.d/classifier-bayes.conf b/docker/rspamd/local.d/classifier-bayes.conf index b69410a..a0ed210 100644 --- a/docker/rspamd/local.d/classifier-bayes.conf +++ b/docker/rspamd/local.d/classifier-bayes.conf @@ -4,3 +4,10 @@ per_user = true; autolearn = false; # only explicit/passive teaching from the app (C2) backend = "redis"; + +# Rspamd ignores Bayes until it has seen this many messages (stock default 200). +# For a personal mailbox that is a long silence: the filter would learn from every +# correction the user made and appear to ignore all of them. 20 is enough to be +# useful without being erratic, and the provider-Junk bootstrap (§C.5) clears it +# on the first account that has a Junk folder worth reading. +min_learns = 20; diff --git a/docs/open-questions.md b/docs/open-questions.md index 4fdc8fe..5ea5740 100644 --- a/docs/open-questions.md +++ b/docs/open-questions.md @@ -3,6 +3,38 @@ Per the setup rules, deviations and unresolved decisions are recorded here rather than guessed silently. +## OQ-8 — Appendix C acceptance 4's threshold is not reachable by a fixture + +**Status:** resolved by testing the property rather than the number; recorded +because the test deviates from the spec's literal wording. + +**The criterion (§C.8.4):** after 20 spam + 20 ham teachings, "a held-out +obvious-spam fixture scores above `move_threshold` for that user and a ham +fixture below `banner_threshold`". + +**What happens:** the ham half holds easily. The spam half does not — a held-out +spam fixture scores around 0.9 against a 12.0 threshold, even though `BAYES_SPAM` +fires correctly and the engine reports 160+ messages learned. + +**Why, and why it is not a bug:** real spam crosses 12.0 by accumulating many +symbols, most of which describe its *delivery* — no DKIM, unknown sending host, +dates that do not line up. This deployment deliberately zeroes those +(`docker/rspamd/local.d/groups.conf`), because scoring already-delivered mail on +SMTP-time facts was firing on 11.3% of ordinary mail. A synthetic fixture has no +delivery history at all, so once those symbols contribute nothing, almost the +only thing left to score is Bayes itself — and one classifier symbol cannot carry +a message to 12.0 on its own, nor should it. + +Asserting the literal number would therefore require either un-tuning the filter +or writing fixtures that fake plausible delivery headers — testing the fixture +generator rather than the learning loop. + +**What the test asserts instead** (`tests/plugins/test_antispam_corpus.py`): +after training, Bayes recognises held-out spam it has never seen; it does not cry +spam at held-out ham; held-out ham stays below `banner_threshold`; spam outscores +ham; and an untaught user sees neither the symbol nor the score — the per-user +isolation the criterion also asks for. + ## OQ-6 — Appendix C needs SDK surfaces the frozen §11 API does not have **Status:** resolved by additive SDK extension; recorded here because C.8.1 says diff --git a/docs/plugins.md b/docs/plugins.md index 622e5fb..c48e5fb 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -98,18 +98,55 @@ s = sdk("myplugin") | `s.blob` | blob store (`store`/`load`/`exists`/`remove`), keyed by user | | `s.outbox` | draft/send helpers (`upsert_draft`, `create_outbox`) | | `await s.http_get(url)` | HTTPS GET through the SSRF guard (blocks private IPs) | -| `s.messages` | read a message: `envelope(user_id, id)`, `header(user_id, id, name)` | -| `s.actions` | act on a message: `set_flags(...)`, `trash(user_id, id)` | +| `s.messages` | read a message: `envelope`, `header`, `raw`, `sample` | +| `s.actions` | act on a message: `set_flags`, `trash`, `move_to_special` | Plus, for routers: `CurrentUserId` (auth dependency → the user's uuid) and `require_csrf` (guard mutating routes). -`s.messages.envelope(user_id, message_id)` returns -`{message_id, subject, from_email, from_name, to: [{email,name}], flags}` or `None`. -`s.messages.header(user_id, message_id, "List-Unsubscribe")` returns a raw header. -`s.actions.set_flags(user_id, id, add=["\\Seen"], remove=[])` — user-settable flags -only (`\Seen`, `\Flagged` [= "Pin"], `\Answered`). `s.actions.trash(user_id, id)` -moves the message's thread to Trash (same as the UI Delete). +### Reading a message + +`s.messages.envelope(user_id, message_id)` → `None`, or: + +```python +{message_id, subject, from_email, from_name, to: [{email, name}], flags, + account_id, # which account it arrived on + mailbox_special_use, # "inbox" | "junk" | "sent" | … | None + account_state} # "backfilling" | "live" | … +``` + +The last three matter more than they look. `message.new` fires for imported +history as well as new arrivals, and for every folder — so a plugin that should +only react to genuinely new inbox mail has to check `account_state == "live"` +and `mailbox_special_use == "inbox"` itself. Acting on a backfill means acting on +years of someone's archive, inline, in the sync loop. + +`s.messages.header(user_id, id, "List-Unsubscribe")` returns one raw header. + +`s.messages.raw(user_id, id)` returns the whole stored RFC822 as bytes, or `None` +when the message isn't the user's or its blob is unreadable. For engines that +need headers *and* body — spam scoring, signature checks. + +`s.messages.sample(user_id, account_id=…, special_use="junk", limit=500, +older_than_days=30)` returns message **ids** from one folder, newest first. For +building a corpus out of mail the user already has. Ids only, so the caller pulls +bodies one at a time rather than materialising 500 messages at once. + +### Acting on a message + +`s.actions.set_flags(user_id, id, add=["\\Seen"], remove=[])` — user-settable +flags only (`\Seen`, `\Flagged` [= "Pin"], `\Answered`). + +`s.actions.trash(user_id, id)` moves the message's whole **thread** to Trash +(same as the UI Delete). + +`s.actions.move_to_special(user_id, id, "junk")` moves **one message** into the +account's folder with that special-use, on the same path as the UI's move: +optimistic DB update, then the IMAP move, then `message.changed`. Per-message on +purpose — filing one spam reply must not drag a legitimate conversation with it. +Returns `False` (never raises) when the message isn't the user's, the account has +no such folder, or it is already there; treat that as "could not act", because +mail must still arrive for an account whose server has no Junk folder. --- @@ -133,6 +170,16 @@ Keyed by `(plugin, user_id, key)` — users can't see each other's values. Subscribe to §8.3 events (`message.new`, `flags.changed`, `thread.updated`, …). Handlers are `async def h(payload: dict) -> None`; every payload carries `user_id`. + +`message.changed` describes a **move**, not just a destination: + +```python +{user_id, message_id, thread_id, mailbox_id, + previous_mailbox_id, special_use, previous_special_use} +``` + +Read the pair, not `special_use` alone — "now in Junk" and "moved out of Junk" +are different facts, and only the transition distinguishes them. They fire wherever the event is published — `message.new` originates in the **worker**. ```python @@ -276,8 +323,19 @@ async def bundle(filename: str) -> FileResponse: ``` **Slots** (inject into the mail UI): `thread.toolbar`, `message.actions`, -`message.banner`, `sidebar.section`, `settings.page`. `message.*` slots receive -`props = { messageId }`. +`message.actions.menu`, `message.banner`, `sidebar.section`, `settings.page`. +`message.*` slots receive `props = { messageId }`. + +- `message.actions` sits on the message header — a persistent control. +- `message.actions.menu` sits *inside* the Actions dropdown, beside Reply and + Delete — for a verb the user reaches for occasionally. The host closes the menu + after any click inside, so the bundle only has to act. +- `settings.page` becomes its own section in Settings. Give the slot a `title` + and that is its nav label; without one it falls back to the plugin name: + + ```python + Slot(name="settings.page", bundle=f"{_UI}/settings.js", title="Antispam filtering") + ``` **Pages** (`app.page`): a full content-area view at `/p/` with a nav entry. @@ -299,7 +357,11 @@ ui_manifest = UiManifest( Style bundles with these theme-aware classes (they adapt to light/dark) instead of inline styles: `wm-root`, `wm-h2`, `wm-muted`, `wm-faint`, `wm-card`, `wm-row`, `wm-input`, `wm-select`, `wm-btn` / `wm-btn-primary` / `wm-btn-danger` / `wm-btn-ghost`, -`wm-chip`, `wm-table`, `wm-list`. +`wm-chip`, `wm-table`, `wm-list`, `wm-menu-item`. + +`wm-menu-item` is for the `message.actions.menu` slot: it applies the exact +utilities the core Actions dropdown uses, so a plugin verb is indistinguishable +from Reply or Delete rather than approximately similar. --- diff --git a/plugins/webmail_plugin_antispam/webmail_plugin_antispam/jobs.py b/plugins/webmail_plugin_antispam/webmail_plugin_antispam/jobs.py index 52b1455..289ba47 100644 --- a/plugins/webmail_plugin_antispam/webmail_plugin_antispam/jobs.py +++ b/plugins/webmail_plugin_antispam/webmail_plugin_antispam/jobs.py @@ -36,6 +36,18 @@ HAM_MIN_AGE_DAYS = 30 # live mail must never queue behind a training backlog. PAUSE_SECONDS = 0.05 +# A Bayes classifier learns the ratio it is shown. Accounts do not sync their +# provider's Junk folder by default (accounts.sync_spam is off), so the spam side +# of this corpus is routinely tiny while the inbox side is thousands — teaching +# both as found would train "everything is ham" and make the filter worse than +# leaving it untrained. Two guards: +# +# below MIN_SPAM_CORPUS teach nothing at all, and leave the account unmarked +# so a later run can do it properly +# above it teach at most MAX_HAM_RATIO times as much ham as spam +MIN_SPAM_CORPUS = 10 +MAX_HAM_RATIO = 3 + _DONE_KEY = "_bootstrapped" @@ -78,9 +90,21 @@ async def bootstrap_account( limit=HAM_SAMPLE, older_than_days=HAM_MIN_AGE_DAYS, ) - if not spam_ids and not ham_ids: + if len(spam_ids) < MIN_SPAM_CORPUS: + # Nothing to be learned from an empty Junk folder, and plenty of harm in + # learning only the ham beside it. Usually means the account is not + # syncing its provider's spam folder. + logger.info( + "antispam bootstrap for %s skipped: only %d spam samples (need %d); " + "enable spam-folder sync on the account to build a corpus", + account_id, + len(spam_ids), + MIN_SPAM_CORPUS, + ) return + ham_ids = ham_ids[: len(spam_ids) * MAX_HAM_RATIO] + spam_taught = await _teach(uid, spam_ids, spam=True) if spam_taught < 0: logger.warning(