Antispam: lessons refused by the engine no longer pass for learned
Found on the NAS, where nothing had been learned in weeks while the UI said "Marked as spam": the source tree predated v1.3.2, so api/worker-* had no controller password and every lesson got 401 — and the trail of state that left behind kept it broken after the password was fixed. - /learn records a lesson only when rspamd accepted it. Recording refused ones made the menu claim "Marked as spam" and its dedup blocked every retry. - Bootstrap: done-markers move to a new key. The old one may have been written by the "-0" bug (a total outage read as a clean run of zero), which left an account marked done with nothing taught and never retried; every older install re-runs the bootstrap once, harmlessly (rspamd skips what it knows). - "Learn from my Spam folder now" in Settings > Antispam filtering queues the bootstrap for the user's accounts at once, instead of the hourly sweep. - webmail.sh update now recreates rspamd: it reads the controller password only when created, so a changed .env reached the app but never the engine. - doctor's advice for the 401 names both real causes (a pre-v1.3.2 compose file, or an rspamd started with an old password) and the fix for each; the old "update v1.3.2" advice could not help. - docs/nas-install.md: how to refresh the source tree on a tarball install. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -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}]
|
||||
|
||||
@@ -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
|
||||
|
||||
+16
-2
@@ -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
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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,6 +143,7 @@ async def bootstrap_account(
|
||||
)
|
||||
return
|
||||
|
||||
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",
|
||||
|
||||
@@ -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,6 +234,7 @@ 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.
|
||||
if taught:
|
||||
await _remember_learned(
|
||||
user_id,
|
||||
body.message_id,
|
||||
@@ -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:
|
||||
|
||||
@@ -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) {
|
||||
<label class="wm-row"><input type="checkbox" data-f="network_checks"${settings.network_checks ? " checked" : ""}> Allow online reputation checks</label>
|
||||
<p class="wm-faint">Off by default. When on, the filter asks <strong>Spamhaus</strong> whether the sending server or a linked domain is known for spam, and <strong>DNSWL</strong> 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.</p>
|
||||
<label class="wm-row"><input type="checkbox" data-f="learn_from_provider_junk"${settings.learn_from_provider_junk ? " checked" : ""}> Learn from mail your provider already marked as spam</label>
|
||||
<div class="wm-row"><button class="wm-btn" data-bootstrap${settings.learn_from_provider_junk ? "" : " disabled"}>Learn from my Spam folder now</button><span class="wm-faint" data-bootstrap-note>${esc(bootstrapNote)}</span></div>
|
||||
<p class="wm-faint">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.</p>
|
||||
</div>
|
||||
|
||||
<div class="wm-card"><strong>Activity</strong>${statsBlock()}</div>`;
|
||||
@@ -133,6 +136,20 @@ export default function mount(el, ctx) {
|
||||
patch({ enabled_accounts: ids });
|
||||
}),
|
||||
);
|
||||
const boot = el.querySelector("[data-bootstrap]");
|
||||
if (boot)
|
||||
boot.addEventListener("click", async () => {
|
||||
boot.disabled = true;
|
||||
try {
|
||||
const res = await ctx.api.pluginFetch("/bootstrap", { method: "POST" });
|
||||
bootstrapNote = res.queued
|
||||
? "Started — it learns in the background over the next few minutes."
|
||||
: "No account has finished its first import yet.";
|
||||
} catch {
|
||||
bootstrapNote = "Could not start it — try again in a moment.";
|
||||
}
|
||||
render();
|
||||
});
|
||||
el.querySelectorAll("[data-add]").forEach((b) =>
|
||||
b.addEventListener("click", async () => {
|
||||
const kind = b.getAttribute("data-add");
|
||||
|
||||
Reference in New Issue
Block a user