Skip to content

Commit ac0663f

Browse files
spamtest reads the server's verdict, never the sender's claim of it
The spamtest test (RFC 3685) with RFC 5235's :percent, advertised as "spamtest spamtestplus" over ManageSieve. A user can finally write "if spamtest :value ge 10, file into Junk" instead of matching on diagnostic headers. The design decision that matters: the verdict comes from the message's spam FLAG alone, handed into the evaluator by the delivery path (Message::GetFlagSpam). The X-hMailServer verdict headers are deliberately NOT consulted - a sender can write them into their own message, inbound mail is not stripped of them, and a test that read them would let senders steer recipients' filters in both directions, including downgrading a real verdict with a forged low score. The first draft of this change consulted them as a fallback; writing the spoofing control is what killed it. That control then earned its keep twice in one hour: it also caught a stale binary still running the header-trusting draft, by filing a message whose only "verdict" was sender-written. Grounding on the flag fixes the granularity honestly. The pipeline persists no score for mail it does not classify (AddSpamScoreHeaders runs only on classification), so a graded 2..9 midrange would be derived from nothing real. The values are 0 - the RFC's "no information", which also covers tested-but-clean, recorded nowhere - and 10, or 100 under :percent. virustest is deliberately absent and still refused at upload: infected mail is blocked or stripped before delivery, a script never sees a message with a virus verdict, and a test that always answered "not scanned" would be advertised and inert - the exact trap the capability-line rule exists to prevent. The fixture's verdicts come from the real antispam pipeline (the SURBL permanent test point, as AntiSpam.Basics uses) with every verdict-header option off. Suite: 1533/1533.
1 parent 3d8a852 commit ac0663f

13 files changed

Lines changed: 966 additions & 643 deletions

File tree

Roadmap.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ strong and where it is thin far more honestly than any prose summary.
5757
| [Transport security and deliverability](#transport-security-and-deliverability) | 43 || 4 | 1 |
5858
| [IMAP](#imap) | 58 || 17 | 3 |
5959
| [POP3](#pop3) | 21 || 6 ||
60-
| [Sieve, ManageSieve and rules](#sieve-managesieve-and-rules) | 54 | 0 | 10 ||
60+
| [Sieve, ManageSieve and rules](#sieve-managesieve-and-rules) | 55 | 0 | 9 ||
6161
| [Authentication and cryptography](#authentication-and-cryptography) | 57 | 0 | 18 ||
6262
| [Anti-spam, anti-virus and content control](#anti-spam-anti-virus-and-content-control) | 54 || 13 ||
6363
| [Storage, accounts and data model](#storage-accounts-and-data-model) | 86 || 10 ||
@@ -72,7 +72,7 @@ strong and where it is thin far more honestly than any prose summary.
7272
| [Future-proofing: standards and protocols](#future-proofing-standards-and-protocols) | 2 || 4 | 2 |
7373
| [Future-proofing: platform and supply chain](#future-proofing-platform-and-supply-chain) | 5 | 1 | 2 | 2 |
7474
| [Future-proofing: deployment and operations](#future-proofing-deployment-and-operations) | 4 | 1 | 4 ||
75-
| **Total** | **629** | **12** | **139** | **15** |
75+
| **Total** | **630** | **12** | **138** | **15** |
7676

7777
Three things stand out and are worth naming rather than leaving to be inferred.
7878
**Storage and the administration surface are the best-covered areas**, and the
@@ -448,7 +448,7 @@ the source, not from documentation.
448448

449449
### Sieve, ManageSieve and rules
450450

451-
54 shipped · 0 underway · 10 not started · 0 deferred
451+
55 shipped · 0 underway · 9 not started · 0 deferred
452452

453453
| | Capability | Detail |
454454
|:-:|---|---|
@@ -510,7 +510,7 @@ the source, not from documentation.
510510
|| reject / ereject (RFC 5429) | Not implemented. **The hazard this row used to describe is gone and the correction matters more than the gap:** neither is in the known-command allowlist and `require "reject"` is refused, so a script that tries to reject mail is rejected AT UPLOAD with a message naming the unsupported extension. The author is told; mail is not silently kept while they believe it is being refused. `SieveSyntax.cs` asserts exactly that script is refused. Implementing them means an SMTP-time refusal for `ereject` and an RFC 3834 bounce for `reject`. |
511511
|| relational (RFC 5231) | Not implemented. No :count or :value match types, and no i;ascii-numeric comparator to make them meaningful — SplitArguments recognises only is/contains/matches **Shipped in dc9301a.** |
512512
| ✅ | RENAMESCRIPT and UNAUTHENTICATE | **Both shipped 16 August 2026.** RENAMESCRIPT was owed rather than optional: the capability response has always claimed VERSION "1.0", which promises the core RFC 5804 commands, and this one was answered with NO "Unknown command". The case that justifies it existing is renaming the ACTIVE script - the manual GETSCRIPT/PUTSCRIPT/DELETESCRIPT route cannot do that at all, because the active script refuses deletion - and the active status follows the new name (`SieveStorage::RenameScript` reads the active name before the move and rewrites it after). Renaming onto a taken name is refused rather than overwriting. UNAUTHENTICATE (RFC 5804 2.14.1) drops the session to the unauthenticated state keeping the connection and is now advertised; the auth-failure counter deliberately carries over so the command is not a way to reset auto-ban bookkeeping mid-connection. Both proven in the round-trip test, including a fresh AUTHENTICATE after UNAUTHENTICATE finding the renamed script still active. |
513-
|| spamtest / spamtestplus / virustest (RFC 5235) | Not implemented, and again the underlying data exists: messages already carry a spam flag and SpamAssassin/AV scores from the antispam pipeline, but no Sieve test can read them |
513+
| ✅ | spamtest / spamtestplus / virustest (RFC 3685 / RFC 5235) | **spamtest and spamtestplus shipped 16 August 2026; virustest deliberately not.** The verdict comes from the message's spam FLAG alone, handed into the evaluator by the delivery path (`Message::GetFlagSpam`) - NOT from the X-hMailServer verdict headers, which a sender can write into their own message and inbound mail is not stripped of; a test reading them would let senders steer recipients' filters in both directions, including downgrading a real verdict with a forged low score. The spoofing control in `SieveSpamtestDelivery.cs` delivers exactly that forgery and asserts it files nothing - the same test that caught a stale binary still trusting headers during development. Grounding on the flag also fixes the granularity honestly: the pipeline persists no score for unclassified mail (`AddSpamScoreHeaders` runs only on classification), so the values are 0 (no verdict) and 10 (classified; 100 under `:percent`), nothing between - a graded midrange would be derived from nothing real. End-to-end verdicts in the fixture come from the REAL antispam pipeline via the SURBL permanent test point, with every verdict-header option off. virustest stays absent and refused at upload: infected mail is blocked or stripped before delivery, so a script never sees a message with a virus verdict, and a test that always answered "not scanned" would be advertised and inert - the exact trap the capability rule exists to prevent. |
514514
| ⬜ | Structured response codes | Partial. `(QUOTA/MAXSIZE)` is emitted by HAVESPACE, PUTSCRIPT and CHECKSCRIPT when a script exceeds the 1 MB limit, and `(ENCRYPT-NEEDED)` by AUTHENTICATE when the connecting range requires TLS on a cleartext connection. Still missing: (WARNINGS), (QUOTA/MAXSCRIPTS), (NONEXISTENT), (ALREADYEXISTS), (TAG), (REFERRAL) and the BYE response codes, so most refusals are still a bare NO with a quoted human string that a client can only show verbatim.
515515
|| subaddress (RFC 5233) | Not implemented. Address parts stop at :all/:localpart/:domain; :user and :detail are not recognised, so plus-addressing cannot be filtered on **Shipped in dc9301a.** |
516516
| ✅ | TLS | STARTTLS (RFC 5804 2.2) is offered whenever a TLS certificate is available on an IMAP, POP3 or SMTP port, and the SSL context comes from `SslContextInitializer` - the same function, cipher list, TLS version floor and key-exchange groups (including the hybrid post-quantum KEMs) as SMTP/POP3/IMAP, so this listener cannot drift from them. It can now also be *required* before a password: the per-IP-range `RequireTLSForAuth` that POP3, IMAP and SMTP already honour is read in `IsConnectionAllowed_`, and on a cleartext connection from such a range the capability line advertises `"SASL" ""` and `AUTHENTICATE` is answered `NO (ENCRYPT-NEEDED)` before the mechanism is validated, before the inline response is parsed and before a literal is read - so the password never crosses the wire, and the connection is dropped rather than desynchronized if a literal was announced. With no certificate the requirement fails closed (no STARTTLS, no mechanism) and is explained once per service start in the application log. Declared gap: no implicit-TLS variant - RFC 5804 defines no such port and no client asks for one, so that is deliberate rather than pending. Covered end to end by `Sieve/ManageSieveTls.cs`.

hmailserver/source/Server/Common/Sieve/ManageSieveServer.cpp

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,17 @@ namespace HM
263263
// it), and :index/:last select among repeated header fields for the header,
264264
// address and date tests. End-to-end tests in SieveDateDelivery.cs include
265265
// zone conversion proven by filing on an hour the raw header does not state.
266-
const char *AdvertisedSieveExtensions = "fileinto copy relational subaddress vacation vacation-seconds imap4flags body mailbox regex ihave environment date index";
266+
// spamtest and spamtestplus moved in on 16 August 2026, under the same rule.
267+
// The delivery-side step is twofold: LocalDelivery passes the message's own
268+
// spam FLAG into the evaluator (so the test works with every header option
269+
// off), and the graded value is derived from the verdict headers when the
270+
// administrator writes them, anchored on the mark threshold - at the score
271+
// that classifies a message, the answer is 10 (or 100 under :percent).
272+
// virustest is deliberately NOT here: infected mail is blocked or stripped
273+
// before delivery, so a script never sees a message with a virus verdict,
274+
// and advertising a test that always answers "not scanned" is the inert-
275+
// capability trap this list exists to prevent.
276+
const char *AdvertisedSieveExtensions = "fileinto copy relational subaddress vacation vacation-seconds imap4flags body mailbox regex ihave environment date index spamtest spamtestplus";
267277

268278
AnsiString EscapeQuoted(const String &value)
269279
{

hmailserver/source/Server/Common/Sieve/SieveEvaluator.cpp

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "../Application/Application.h"
1111
#include "../Application/Configuration.h"
1212
#include "../Util/Time.h"
13+
#include "../AntiSpam/AntiSpamConfiguration.h"
1314

1415
#ifdef _DEBUG
1516
#define DEBUG_NEW new(_NORMAL_BLOCK, __FILE__, __LINE__)
@@ -96,6 +97,12 @@ namespace HM
9697
mailbox_exists_ = callback;
9798
}
9899

100+
void
101+
SieveEvaluator::SetClassifiedAsSpam(bool classified)
102+
{
103+
classified_as_spam_ = classified;
104+
}
105+
99106
bool
100107
SieveEvaluator::GetEnvironmentItem_(const String &name, String &value)
101108
{
@@ -907,6 +914,9 @@ namespace HM
907914
if (name == _T("environment"))
908915
return EvaluateComparisonTest_(test, message, ValueSource::Environment);
909916

917+
if (name == _T("spamtest"))
918+
return EvaluateSpamTest_(test, message);
919+
910920
if (name == _T("date"))
911921
return EvaluateDateTest_(test, message, false);
912922

@@ -1202,6 +1212,45 @@ namespace HM
12021212
values.push_back(selected);
12031213
}
12041214

1215+
bool
1216+
SieveEvaluator::EvaluateSpamTest_(const std::shared_ptr<SieveTest> &test, const SieveMessage &message)
1217+
{
1218+
SieveArgumentSet set;
1219+
String ignored;
1220+
if (!SieveParser::SplitArguments(test->arguments, set, ignored))
1221+
return false;
1222+
1223+
if (set.stringLists.size() != 1 || set.stringLists[0].empty())
1224+
return false;
1225+
1226+
// The verdict comes from the message's spam FLAG alone, handed in by the
1227+
// delivery path. The verdict HEADERS are deliberately not consulted: a
1228+
// sender can write X-hMailServer-Spam and X-hMailServer-Reason-Score into
1229+
// their own message, and inbound mail is not stripped of them - so a test
1230+
// reading them would let senders steer recipients' filters in both
1231+
// directions, including downgrading a real verdict with a forged low score.
1232+
// The flag cannot be forged from outside; it exists only in this process.
1233+
//
1234+
// This also decides the granularity honestly: the pipeline persists no
1235+
// score for mail it did NOT classify (AddSpamScoreHeaders runs only on
1236+
// classification), so a graded 2..9 midrange has nothing real to be
1237+
// derived from. The answer is 10 (100 under :percent) for classified mail
1238+
// and "0" - the RFC's "no information" - for everything else, including
1239+
// tested-but-clean, which the pipeline records nowhere.
1240+
String value;
1241+
if (!classified_as_spam_)
1242+
value = _T("0");
1243+
else if (set.percentGiven)
1244+
value = _T("100");
1245+
else
1246+
value = _T("10");
1247+
1248+
std::vector<String> values;
1249+
values.push_back(value);
1250+
1251+
return MatchValuesAgainstKeys_(set, values, set.stringLists[0]);
1252+
}
1253+
12051254
bool
12061255
SieveEvaluator::EvaluateDateTest_(const std::shared_ptr<SieveTest> &test, const SieveMessage &message, bool currentDate)
12071256
{

hmailserver/source/Server/Common/Sieve/SieveEvaluator.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,12 @@ namespace HM
115115
// through to their non-conditional branches rather than acting on a guess.
116116
void SetMailboxExists(std::function<bool(const String &)> callback);
117117

118+
// The server's own spam classification for the message being delivered, from
119+
// the delivery path (Message::GetFlagSpam). The spamtest value can also be
120+
// derived from the verdict headers when the administrator has them on; this
121+
// flag is the channel that works when they are off.
122+
void SetClassifiedAsSpam(bool classified);
123+
118124
private:
119125
// Where the values a test compares against come from.
120126
enum class ValueSource { Header, Address, Envelope, Flags, Body, Environment };
@@ -131,6 +137,9 @@ namespace HM
131137
// match against that item false, which is the RFC's unknown-item behaviour.
132138
static bool GetEnvironmentItem_(const String &name, String &value);
133139

140+
// spamtest (RFC 3685 / RFC 5235).
141+
bool EvaluateSpamTest_(const std::shared_ptr<SieveTest> &test, const SieveMessage &message);
142+
134143
// date / currentdate (RFC 5260).
135144
bool EvaluateDateTest_(const std::shared_ptr<SieveTest> &test, const SieveMessage &message, bool currentDate);
136145

@@ -212,5 +221,6 @@ namespace HM
212221
SieveResult *result_;
213222

214223
std::function<bool(const String &)> mailbox_exists_;
224+
bool classified_as_spam_ = false;
215225
};
216226
}

hmailserver/source/Server/Common/Sieve/SieveParser.cpp

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,13 @@ namespace HM
164164
// registration, though they ship together.
165165
L"date",
166166
L"index",
167+
// RFC 3685 / RFC 5235: the server's own spam verdict as a graded value.
168+
// virustest is deliberately absent - this server's antivirus pipeline
169+
// blocks or strips infected mail before delivery, so a message reaching a
170+
// script has no virus verdict to report, and a test that always answered
171+
// "not scanned" would be advertised and inert.
172+
L"spamtest",
173+
L"spamtestplus",
167174
L"comparator-i;ascii-casemap",
168175
L"comparator-i;octet",
169176
L"comparator-i;ascii-numeric"
@@ -342,6 +349,11 @@ namespace HM
342349
// :index :last (RFC 5260 6): count from the end.
343350
result.lastGiven = true;
344351
}
352+
else if (tag == _T("percent"))
353+
{
354+
// spamtest :percent (RFC 5235).
355+
result.percentGiven = true;
356+
}
345357

346358
continue;
347359
}
@@ -482,7 +494,8 @@ namespace HM
482494
{
483495
L"address", L"allof", L"anyof", L"exists", L"false", L"header",
484496
L"not", L"size", L"true", L"envelope", L"hasflag", L"body",
485-
L"mailboxexists", L"ihave", L"environment", L"date", L"currentdate"
497+
L"mailboxexists", L"ihave", L"environment", L"date", L"currentdate",
498+
L"spamtest"
486499
};
487500

488501
for (const wchar_t *candidate : known)
@@ -1539,6 +1552,25 @@ namespace HM
15391552
return true;
15401553
}
15411554

1555+
if (name == _T("spamtest"))
1556+
{
1557+
if (!NeedExtension_(_T("spamtest"), _T("the 'spamtest' test"), test->line, errorMessage))
1558+
return false;
1559+
1560+
if (!CheckTags_(set, _T("comparator is contains matches value count regex percent"),
1561+
_T("'spamtest'"), errorMessage))
1562+
return false;
1563+
1564+
if (set.percentGiven &&
1565+
!NeedExtension_(_T("spamtestplus"), _T("':percent'"), test->line, errorMessage))
1566+
return false;
1567+
1568+
if (!ValidateMatchArguments_(set, _T("'spamtest'"), test->line, errorMessage, 1))
1569+
return false;
1570+
1571+
return true;
1572+
}
1573+
15421574
if (name == _T("ihave"))
15431575
{
15441576
if (!NeedExtension_(_T("ihave"), _T("the 'ihave' test"), test->line, errorMessage))

hmailserver/source/Server/Common/Sieve/SieveParser.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,9 @@ namespace HM
126126
bool indexGiven = false;
127127
bool lastGiven = false;
128128

129+
// spamtest :percent (RFC 5235).
130+
bool percentGiven = false;
131+
129132
// Positional string-list arguments, in order.
130133
std::vector<std::vector<String>> stringLists;
131134
};

hmailserver/source/Server/Common/Sieve/SieveScript.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,8 @@ namespace HM
7070
const String &rawMessage,
7171
const SieveEnvelope &envelope,
7272
SieveResult &result,
73-
std::function<bool(const String &)> mailboxExists)
73+
std::function<bool(const String &)> mailboxExists,
74+
bool classifiedAsSpam)
7475
{
7576
String errorMessage;
7677

@@ -88,6 +89,7 @@ namespace HM
8889
SieveMessage message(rawMessage);
8990
SieveEvaluator evaluator;
9091
evaluator.SetMailboxExists(mailboxExists);
92+
evaluator.SetClassifiedAsSpam(classifiedAsSpam);
9193
return evaluator.Evaluate(sieveScript.GetCommands(), message, envelope, result);
9294
}
9395
}

hmailserver/source/Server/Common/Sieve/SieveScript.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,8 @@ namespace HM
5252
const String &rawMessage,
5353
const SieveEnvelope &envelope,
5454
SieveResult &result,
55-
std::function<bool(const String &)> mailboxExists = nullptr);
55+
std::function<bool(const String &)> mailboxExists = nullptr,
56+
bool classifiedAsSpam = false);
5657

5758
private:
5859
std::vector<std::shared_ptr<SieveCommand>> commands_;

hmailserver/source/Server/SMTP/LocalDelivery.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -456,7 +456,7 @@ namespace HM
456456
};
457457

458458
SieveResult sieveResult;
459-
String actions = SieveScript::Evaluate(script, rawMessage, envelope, sieveResult, mailboxExists);
459+
String actions = SieveScript::Evaluate(script, rawMessage, envelope, sieveResult, mailboxExists, message->GetFlagSpam());
460460

461461
// A script that fails to parse must never break delivery; fall through to a
462462
// normal keep. (The structured result is already at its defaults in that case,

0 commit comments

Comments
 (0)