diff --git a/backend/tests/plugins/test_antispam_api.py b/backend/tests/plugins/test_antispam_api.py index b0e0a18..274b612 100644 --- a/backend/tests/plugins/test_antispam_api.py +++ b/backend/tests/plugins/test_antispam_api.py @@ -7,6 +7,8 @@ tables. Requires the plugin installed and `install antispam` run. from __future__ import annotations +import importlib +import types import uuid from collections.abc import AsyncIterator @@ -32,6 +34,29 @@ from app.models import User BASE = "/api/plugins/antispam" +class _SdkProxy: + """The router's SDK handle with a few calls swapped out. The SDK object is a + frozen dataclass, and `.messages` / `.accounts` hand out a fresh object on + every access, so patching attributes on them does not stick.""" + + def __init__(self, real: object, **overrides: object) -> None: + self._real = real + self._overrides = overrides + + def __getattr__(self, name: str) -> object: + if name in self._overrides: + return self._overrides[name] + return getattr(self._real, name) + + +def _router_module() -> object: + return importlib.import_module("webmail_plugin_antispam.router") + + +async def _raw(*_a: object, **_k: object) -> bytes: + return b"Subject: x\r\n\r\nbody" + + async def _mk_user(sm: async_sessionmaker) -> uuid.UUID: # type: ignore[type-arg] async with sm() as s: user = User( @@ -188,18 +213,82 @@ async def test_ui_bundles_are_served_and_gated(ctx: _Ctx) -> None: assert (await ctx.client_a.get(f"{BASE}/ui/../hook.py")).status_code == 404 -async def test_learn_records_even_without_a_prior_verdict(ctx: _Ctx) -> None: +async def test_learn_records_even_without_a_prior_verdict( + ctx: _Ctx, monkeypatch: pytest.MonkeyPatch +) -> None: """Mail the provider filed as spam has no verdict row of ours to update. The write must therefore upsert — and it must happen before the move, or the passive-learning hook re-teaches the lesson the move announces. """ + router_mod = _router_module() + + async def _accepted(_raw: bytes, **_kw: object) -> bool: + return True + + monkeypatch.setattr( + router_mod, + "_SDK", + _SdkProxy(router_mod._SDK, messages=_SdkProxy(router_mod._SDK.messages, raw=_raw)), + ) + monkeypatch.setattr(rspamd_mod, "learn", _accepted) message_id = uuid.uuid4() r = await ctx.client_a.post( f"{BASE}/learn", json={"message_id": str(message_id), "verdict": "spam"} ) assert r.status_code == 200 + assert r.json()["taught"] is True assert r.json()["moved"] is False # not this user's message: nothing to move body = (await ctx.client_a.get(f"{BASE}/context/{message_id}")).json() assert body["verdict"]["learned"] == "spam" + + +async def test_a_refused_lesson_is_not_recorded(ctx: _Ctx, monkeypatch: pytest.MonkeyPatch) -> None: + """The engine said no (a 401 from a mismatched controller password, say). + + Recording it anyway made the menu say "Marked as spam" for a lesson never + taught, and its dedup then blocked every retry. + """ + router_mod = _router_module() + + async def _refused(_raw: bytes, **_kw: object) -> bool: + return False + + monkeypatch.setattr( + router_mod, + "_SDK", + _SdkProxy(router_mod._SDK, messages=_SdkProxy(router_mod._SDK.messages, raw=_raw)), + ) + monkeypatch.setattr(rspamd_mod, "learn", _refused) + message_id = uuid.uuid4() + r = await ctx.client_a.post( + f"{BASE}/learn", json={"message_id": str(message_id), "verdict": "spam"} + ) + assert r.status_code == 200 + assert r.json()["taught"] is False + body = (await ctx.client_a.get(f"{BASE}/context/{message_id}")).json() + assert body["verdict"] is None + + +async def test_bootstrap_now_queues_only_this_users_accounts( + ctx: _Ctx, monkeypatch: pytest.MonkeyPatch +) -> None: + router_mod = _router_module() + mine, theirs = uuid.uuid4(), uuid.uuid4() + queued: list[dict[str, object]] = [] + + async def _live() -> list[tuple[uuid.UUID, uuid.UUID]]: + return [(ctx.user_a, mine), (ctx.user_b, theirs)] + + async def _enqueue(_func: object, **kwargs: object) -> None: + queued.append(kwargs) + + monkeypatch.setattr( + router_mod, + "_SDK", + _SdkProxy(router_mod._SDK, accounts=types.SimpleNamespace(live=_live), enqueue=_enqueue), + ) + r = await ctx.client_a.post(f"{BASE}/bootstrap") + assert r.json() == {"queued": 1} + assert queued == [{"user_id": str(ctx.user_a), "account_id": str(mine), "force": True}] diff --git a/backend/tests/plugins/test_antispam_learning.py b/backend/tests/plugins/test_antispam_learning.py index 2df9e3c..9646c92 100644 --- a/backend/tests/plugins/test_antispam_learning.py +++ b/backend/tests/plugins/test_antispam_learning.py @@ -459,3 +459,85 @@ def test_compose_gives_the_app_the_controller_password() -> None: f"{rel}: the app containers do not receive RSPAMD_CONTROLLER_PASSWORD — " "antispam will classify but never learn" ) + + +class _BootKv: + """A kv store holding a given value per key; records what gets written.""" + + def __init__(self, values: dict[str, object]) -> None: + self.values = values + self.written: list[tuple[str, object]] = [] + + async def get(self, key: str, default: object = None) -> object: + return self.values.get(key, default) + + async def set(self, key: str, value: object) -> None: + self.written.append((key, value)) + + +def _boot_fake(kv: _BootKv, taught: list[bool]) -> object: + class _M: + async def sample(self, *_a: object, **_kw: object) -> list[uuid.UUID]: + return [uuid.uuid4() for _ in range(12)] + + async def raw(self, *_a: object, **_k: object) -> bytes: + return b"x" + + class _Fake: + messages = _M() + + def kv(self, _u: object) -> _BootKv: + return kv + + async def publish(self, *_a: object, **_k: object) -> None: + taught.append(True) + + return _Fake() + + +@pytest.mark.asyncio +async def test_an_old_style_marker_does_not_block_the_bootstrap( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Markers under the old key may come from the `-0` bug: done, nothing taught.""" + account = str(uuid.uuid4()) + kv = _BootKv({"_bootstrapped": [account]}) + ran: list[bool] = [] + + async def _settings(_u: object) -> policy.Settings: + return SETTINGS + + async def _ok(_raw: bytes, **_kw: object) -> bool: + return True + + monkeypatch.setattr(policy, "load_settings", _settings) + monkeypatch.setattr(jobs, "_SDK", _boot_fake(kv, ran)) + monkeypatch.setattr(jobs.rspamd, "learn", _ok) + monkeypatch.setattr(jobs, "PAUSE_SECONDS", 0) + await jobs.bootstrap_account({}, user_id=str(uuid.uuid4()), account_id=account) + assert ran == [True] + assert kv.written == [("bootstrap_done", [account])] + + +@pytest.mark.asyncio +async def test_done_accounts_are_skipped_unless_forced(monkeypatch: pytest.MonkeyPatch) -> None: + account = str(uuid.uuid4()) + + async def _settings(_u: object) -> policy.Settings: + return SETTINGS + + async def _ok(_raw: bytes, **_kw: object) -> bool: + return True + + monkeypatch.setattr(policy, "load_settings", _settings) + monkeypatch.setattr(jobs.rspamd, "learn", _ok) + monkeypatch.setattr(jobs, "PAUSE_SECONDS", 0) + + kv, ran = _BootKv({"bootstrap_done": [account]}), [] + monkeypatch.setattr(jobs, "_SDK", _boot_fake(kv, ran)) + await jobs.bootstrap_account({}, user_id=str(uuid.uuid4()), account_id=account) + assert ran == [] # the hourly sweep leaves it alone + + await jobs.bootstrap_account({}, user_id=str(uuid.uuid4()), account_id=account, force=True) + assert ran == [True] # the button re-runs it + assert kv.written == [] # already listed: not appended twice diff --git a/deploy/webmail.sh b/deploy/webmail.sh index 04f8326..9369f5c 100755 --- a/deploy/webmail.sh +++ b/deploy/webmail.sh @@ -397,8 +397,15 @@ print('AUTHORISED') *401*|*Unauthorized*) red " the api container cannot authenticate to the antispam controller." red " Mail will still be classified, but NOTHING will be learned." - echo " RSPAMD_CONTROLLER_PASSWORD reaches rspamd but not api/worker-*." - echo " fix: webmail.sh update v1.3.2 (or later), which passes it to both" + echo " The api and rspamd containers hold different controller passwords. Either:" + if [ "$(grep -c RSPAMD_CONTROLLER_PASSWORD "$COMPOSE_FILE")" -lt 3 ]; then + echo " - ${COMPOSE_FILE} is older than v1.3.2 and does not give the password" + echo " to api/worker-*. fix: refresh the source tree, keeping .env" + echo " (docs/nas-install.md §4, 'Refreshing the source tree')" + fi + echo " - rspamd was started with an older value from .env. fix: webmail.sh update" + echo " (it recreates rspamd), or just that container:" + echo " docker compose --env-file .env -f docker-compose.yml up -d --no-deps --force-recreate rspamd" ok=1 ;; *"not running"*|*"No such service"*|*"no such service"*|*"is not running"*) ylw " antispam not checked — the stack is not running. Start it, then re-run doctor." ;; @@ -567,6 +574,13 @@ cmd_update() { # stack would look updated while missing a container. --no-recreate so this # only creates what is absent and leaves the rolled services alone. IMAGE_TAG="$target" compose up -d --no-recreate + # rspamd holds no state of its own (its Bayes data lives in redis-rspamd), and + # it reads RSPAMD_CONTROLLER_PASSWORD only when its container is created. Left + # out of the roll it kept whatever password it first started with, so a later + # change in .env reached the app but not the engine, and every lesson was + # refused with 401 while mail still got classified. Recreating it is a few + # seconds of "deliver unclassified", which the filter is built to tolerate. + IMAGE_TAG="$target" compose up -d --no-deps --force-recreate rspamd echo "== 5/5 health gate (${HEALTH_TIMEOUT}s)" if health_gate; then diff --git a/docs/nas-install.md b/docs/nas-install.md index 99fcee2..f370963 100644 --- a/docs/nas-install.md +++ b/docs/nas-install.md @@ -404,6 +404,32 @@ migrations stay expand-then-contract (see `docs/migrations.md` discipline in `deployment-setup-instructions.md` §9). For the rare breaking migration, restore the dump that `update` took in step 1. +### Refreshing the source tree (installs without git) + +`update` replaces the **images**. The files around them — `docker-compose.yml`, +`deploy/webmail.sh`, `docker/rspamd/local.d` (read by the antispam engine) — come +from the source tree on the NAS, and change only when that tree does. On an +install that was unpacked from a tarball rather than cloned, refresh it now and +then, keeping `.env` and the data directory: + +```bash +cd /share/Container/webmail # the folder that holds src/ +curl -fsSL -o main.tar.gz http://192.168.1.185:8418/lukas/webmail-client/archive/main.tar.gz +tar -xzf main.tar.gz # unpacks to ./webmail-client +cp -p src/.env webmail-client/.env +[ -f src/docker/.env.prod ] && cp -p src/docker/.env.prod webmail-client/docker/.env.prod +./src/deploy/webmail.sh down # stop with the OLD files +mv src src-old-$(date +%Y%m%d) # kept as a backup +mv webmail-client src +chmod +x src/deploy/*.sh src/docker/*.sh +cd src && ./deploy/webmail.sh up && ./deploy/webmail.sh doctor +``` + +`down`/`up` rather than `update`: every container is recreated from the new +compose file, including the ones `update` leaves alone, and bind mounts point at +the new folder instead of the renamed one. Data lives in `DATA_ROOT`, outside +`src/`, and `.env` keeps `IMAGE_TAG`, so the same version comes back up. + ### Building images elsewhere Building on the NAS is simple but slow and RAM-hungry. Two alternatives: diff --git a/plugins/webmail_plugin_antispam/webmail_plugin_antispam/jobs.py b/plugins/webmail_plugin_antispam/webmail_plugin_antispam/jobs.py index efd1643..9ec00c8 100644 --- a/plugins/webmail_plugin_antispam/webmail_plugin_antispam/jobs.py +++ b/plugins/webmail_plugin_antispam/webmail_plugin_antispam/jobs.py @@ -48,7 +48,12 @@ PAUSE_SECONDS = 0.05 MIN_SPAM_CORPUS = 10 MAX_HAM_RATIO = 3 -_DONE_KEY = "_bootstrapped" +# Accounts whose warm start is done. A new key on purpose: markers under the old +# one ("_bootstrapped") may have been written by the `-0` bug described in +# _teach — an account marked done after a total engine outage, with nothing +# taught, and never retried. Ignoring them costs every older install one repeat +# bootstrap, which is harmless: rspamd skips messages it has already learned. +_DONE_KEY = "bootstrap_done" async def _teach(user_id: uuid.UUID, ids: list[uuid.UUID], *, spam: bool) -> int | None: @@ -75,9 +80,14 @@ async def _teach(user_id: uuid.UUID, ids: list[uuid.UUID], *, spam: bool) -> int async def bootstrap_account( - ctx: dict[str, Any], *, user_id: str, account_id: str + ctx: dict[str, Any], *, user_id: str, account_id: str, force: bool = False ) -> None: - """Teach Bayes from one account's existing mail. Safe to call more than once.""" + """Teach Bayes from one account's existing mail. Safe to call more than once. + + ``force`` runs it even for an account already marked done — the "learn from + my Spam folder now" button. The provider-Junk setting still applies: the + user turned that off for a reason. + """ uid, acc = uuid.UUID(user_id), uuid.UUID(account_id) settings = await policy.load_settings(uid) @@ -85,7 +95,7 @@ async def bootstrap_account( return done = await _SDK.kv(uid).get(_DONE_KEY, []) - if account_id in done: + if account_id in done and not force: return spam_ids = await _SDK.messages.sample( @@ -133,7 +143,8 @@ async def bootstrap_account( ) return - await _SDK.kv(uid).set(_DONE_KEY, [*done, account_id]) + if account_id not in done: + await _SDK.kv(uid).set(_DONE_KEY, [*done, account_id]) logger.info( "antispam bootstrap for %s: learned %d spam, %d ham", account_id, diff --git a/plugins/webmail_plugin_antispam/webmail_plugin_antispam/router.py b/plugins/webmail_plugin_antispam/webmail_plugin_antispam/router.py index f2038d7..7758985 100644 --- a/plugins/webmail_plugin_antispam/webmail_plugin_antispam/router.py +++ b/plugins/webmail_plugin_antispam/webmail_plugin_antispam/router.py @@ -218,7 +218,10 @@ async def learn(body: LearnIn, user_id: CurrentUserId) -> dict[str, object]: """Teach this user's Bayes, then file the message accordingly (§C.5). The move is what the user actually asked for, so it happens even when the - engine is unreachable — teaching is best-effort, filing is not. + engine is unreachable — teaching is best-effort, filing is not. But only a + lesson the engine accepted is recorded: recording a refused one made the menu + say "Marked as spam" for nothing, and its dedup then blocked every retry. + Left unrecorded, the move below lets the passive hook try again. """ spam = body.verdict == "spam" raw = await _SDK.messages.raw(user_id, body.message_id) @@ -231,12 +234,13 @@ async def learn(body: LearnIn, user_id: CurrentUserId) -> dict[str, object]: # the very lesson we just taught. Writing `learned` first makes that hook's # dedup see it and stand down. An upsert, because a message the provider # filed as spam has no verdict row of ours to update. - await _remember_learned( - user_id, - body.message_id, - verdict=body.verdict, - action="moved" if spam else "none", - ) + if taught: + await _remember_learned( + user_id, + body.message_id, + verdict=body.verdict, + action="moved" if spam else "none", + ) moved = await _SDK.actions.move_to_special( user_id, body.message_id, "junk" if spam else "inbox" @@ -244,6 +248,24 @@ async def learn(body: LearnIn, user_id: CurrentUserId) -> dict[str, object]: return {"status": "ok", "taught": taught, "moved": moved} +@router.post("/bootstrap", dependencies=[Depends(require_csrf)]) +async def bootstrap_now(user_id: CurrentUserId) -> dict[str, object]: + """ "Learn from my Spam folder now": queue the warm start for each of this + user's accounts, even ones already done, instead of waiting for the hourly + sweep. The job applies the same rules (setting on, enough spam samples).""" + from webmail_plugin_antispam.jobs import bootstrap_account + + accounts = [acc for uid, acc in await _SDK.accounts.live() if uid == user_id] + for account_id in accounts: + await _SDK.enqueue( + bootstrap_account, + user_id=str(user_id), + account_id=str(account_id), + force=True, + ) + return {"queued": len(accounts)} + + @router.get("/stats", response_model=StatsOut) async def stats(user_id: CurrentUserId) -> StatsOut: async with _SDK.session(user_id) as s: diff --git a/plugins/webmail_plugin_antispam/webmail_plugin_antispam/ui/settings.js b/plugins/webmail_plugin_antispam/webmail_plugin_antispam/ui/settings.js index 29a89f5..db22749 100644 --- a/plugins/webmail_plugin_antispam/webmail_plugin_antispam/ui/settings.js +++ b/plugins/webmail_plugin_antispam/webmail_plugin_antispam/ui/settings.js @@ -10,6 +10,7 @@ export default function mount(el, ctx) { let rules = []; let accounts = []; let stats = null; + let bootstrapNote = ""; // outcome of the last "learn from my Spam folder now" click let disposed = false; async function load() { @@ -115,6 +116,8 @@ export default function mount(el, ctx) {
Off by default. When on, the filter asks Spamhaus whether the sending server or a linked domain is known for spam, and DNSWL whether the sender is known-good. That reveals those addresses and domains to them — and to whichever DNS resolver you use. The message itself is never sent, and no fingerprint of it leaves your server.
+Otherwise this runs on its own once an hour, for accounts with at least 10 messages in Spam. Running it again is harmless: messages the filter already learned are skipped.