Add SIEVE-06 Cassandane comparison test and full-block evidence
Verify Bongo deduplicates "keep; keep;" and "keep; fileinto \"INBOX\";" to a single delivered copy, matching Cyrus Cassandane's test_dup_keep_keep/test_dup_keep_fileinto regression tests, while two genuinely distinct fileinto targets still each receive their own copy. Documents the duplicate-delivery fix found via this comparison, and synthesizes the Pigeonhole cross-references and all six real bugs found and fixed across the whole SIEVE-01..06 block. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Mario Fetka
+278
@@ -0,0 +1,278 @@
|
||||
#!/usr/bin/env python3
|
||||
# This program is free software, licensed under the terms of the GNU GPL.
|
||||
# See the Bongo COPYING file for full details.
|
||||
# Copyright (c) 2026 Bongo Project contributors
|
||||
|
||||
"""Verify Bongo deduplicates repeated Sieve delivery actions that target
|
||||
the same mailbox ("keep; keep;" and "keep; fileinto \"INBOX\";"), matching
|
||||
Cyrus Cassandane's Cassandane/Cyrus/Sieve.pm test_dup_keep_keep and
|
||||
test_dup_keep_fileinto regression tests, while still delivering a
|
||||
separate copy for each of several genuinely distinct fileinto targets.
|
||||
Requires BONGO_ALLOW_LIVE_USER_TEST=1 for the disposable test1/test2
|
||||
accounts."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import base64
|
||||
import os
|
||||
import smtplib
|
||||
import socket
|
||||
import ssl
|
||||
import sys
|
||||
import time
|
||||
from email.message import EmailMessage
|
||||
|
||||
sys.path.insert(0, os.path.join(
|
||||
os.path.dirname(os.path.abspath(__file__)), "..", "..",
|
||||
"src", "libs", "python"))
|
||||
from bongo.store.StoreClient import StoreClient # noqa: E402
|
||||
|
||||
|
||||
HOST = os.environ.get("BONGO_TEST_HOST", "127.0.0.1")
|
||||
SIEVE_PORT = int(os.environ.get("BONGO_TEST_SIEVE_PORT", "4190"))
|
||||
SUBMISSION_PORT = int(os.environ.get("BONGO_TEST_SUBMISSION_PORT", "587"))
|
||||
STORE_PORT = int(os.environ.get("BONGO_TEST_STORE_PORT", "689"))
|
||||
TIMEOUT = float(os.environ.get("BONGO_TEST_TIMEOUT", "60"))
|
||||
SENDER = os.environ.get("BONGO_TEST_USER1", "test1")
|
||||
RECIPIENT = os.environ.get("BONGO_TEST_USER2", "test2")
|
||||
DOMAIN = os.environ.get("BONGO_TEST_DOMAIN", "bongo.test")
|
||||
PASSWORD = os.environ.get("BONGO_TEST_PASSWORD", "")
|
||||
ALLOW_LIVE = os.environ.get("BONGO_ALLOW_LIVE_USER_TEST") == "1"
|
||||
TOKEN = f"{os.getpid()}-{int(time.time())}"
|
||||
|
||||
|
||||
class DuplicateDeliveryError(RuntimeError):
|
||||
"""Raised when live delivery deduplication differs from its contract."""
|
||||
|
||||
|
||||
class SieveConnection:
|
||||
def __init__(self) -> None:
|
||||
raw = socket.create_connection((HOST, SIEVE_PORT), timeout=TIMEOUT)
|
||||
raw.settimeout(TIMEOUT)
|
||||
self.socket: socket.socket | ssl.SSLSocket = raw
|
||||
self.reader = raw.makefile("rb")
|
||||
self._read_greeting()
|
||||
|
||||
def close(self) -> None:
|
||||
try:
|
||||
self.reader.close()
|
||||
finally:
|
||||
self.socket.close()
|
||||
|
||||
def line(self) -> bytes:
|
||||
line = self.reader.readline(1 << 16)
|
||||
if not line:
|
||||
raise DuplicateDeliveryError("connection closed while waiting for a response")
|
||||
return line
|
||||
|
||||
def _read_greeting(self) -> None:
|
||||
while True:
|
||||
line = self.line()
|
||||
if line.startswith(b"OK"):
|
||||
return
|
||||
if line.startswith(b"NO") or line.startswith(b"BYE"):
|
||||
raise DuplicateDeliveryError(f"greeting failed: {line!r}")
|
||||
|
||||
def command(self, line: str, *, expect: str = "OK") -> None:
|
||||
self.socket.sendall((line + "\r\n").encode("utf-8"))
|
||||
reply = self.line()
|
||||
if not reply.decode("ascii", "replace").startswith(expect):
|
||||
raise DuplicateDeliveryError(f"{line!r} expected {expect!r}, got {reply!r}")
|
||||
|
||||
def raw_send(self, data: bytes, *, expect: str = "OK") -> None:
|
||||
self.socket.sendall(data)
|
||||
reply = self.line()
|
||||
if not reply.decode("ascii", "replace").startswith(expect):
|
||||
raise DuplicateDeliveryError(f"expected {expect!r}, got {reply!r}")
|
||||
|
||||
def start_tls(self) -> None:
|
||||
self.reader.close()
|
||||
context = ssl.create_default_context()
|
||||
context.check_hostname = False
|
||||
context.verify_mode = ssl.CERT_NONE
|
||||
context.minimum_version = ssl.TLSVersion.TLSv1_2
|
||||
self.socket = context.wrap_socket(self.socket, server_hostname=HOST)
|
||||
self.socket.settimeout(TIMEOUT)
|
||||
self.reader = self.socket.makefile("rb")
|
||||
|
||||
def authenticate(self, user: str) -> None:
|
||||
payload = base64.b64encode(
|
||||
b"\0" + user.encode() + b"\0" + PASSWORD.encode()
|
||||
).decode("ascii")
|
||||
self.command(f'AUTHENTICATE "PLAIN" "{payload}"')
|
||||
|
||||
|
||||
def literal(data: bytes) -> bytes:
|
||||
return b"{" + str(len(data)).encode() + b"+}\r\n" + data
|
||||
|
||||
|
||||
def connect_authenticated(user: str) -> SieveConnection:
|
||||
client = SieveConnection()
|
||||
client.command("STARTTLS")
|
||||
client.start_tls()
|
||||
client.authenticate(user)
|
||||
return client
|
||||
|
||||
|
||||
def install_script(name: str, script: bytes) -> None:
|
||||
client = connect_authenticated(RECIPIENT)
|
||||
try:
|
||||
client.raw_send(b'PUTSCRIPT "' + name.encode() + b'" ' + literal(script))
|
||||
client.command(f'SETACTIVE "{name}"')
|
||||
client.command("LOGOUT")
|
||||
finally:
|
||||
client.close()
|
||||
|
||||
|
||||
def remove_script(name: str) -> None:
|
||||
client = connect_authenticated(RECIPIENT)
|
||||
try:
|
||||
client.command('SETACTIVE ""')
|
||||
client.command(f'DELETESCRIPT "{name}"')
|
||||
client.command("LOGOUT")
|
||||
finally:
|
||||
client.close()
|
||||
|
||||
|
||||
def submit(subject: str) -> None:
|
||||
message = EmailMessage()
|
||||
message["From"] = f"{SENDER}@{DOMAIN}"
|
||||
message["To"] = f"{RECIPIENT}@{DOMAIN}"
|
||||
message["Subject"] = subject
|
||||
message.set_content(f"{subject}\n")
|
||||
|
||||
context = ssl.create_default_context()
|
||||
context.check_hostname = False
|
||||
context.verify_mode = ssl.CERT_NONE
|
||||
with smtplib.SMTP(HOST, SUBMISSION_PORT, timeout=TIMEOUT) as client:
|
||||
client.ehlo()
|
||||
client.starttls(context=context)
|
||||
client.ehlo()
|
||||
client.login(SENDER, PASSWORD)
|
||||
refused = client.send_message(message)
|
||||
if refused:
|
||||
raise DuplicateDeliveryError(f"SMTP refused recipient: {refused!r}")
|
||||
|
||||
|
||||
def count_marker(store: StoreClient, path: str, marker: bytes) -> int:
|
||||
count = 0
|
||||
try:
|
||||
documents = [document.uid for document in store.List(path)]
|
||||
except Exception:
|
||||
return 0
|
||||
for uid in documents:
|
||||
try:
|
||||
data = store.Read(uid)
|
||||
except Exception:
|
||||
continue
|
||||
if marker in data:
|
||||
count += 1
|
||||
return count
|
||||
|
||||
|
||||
def wait_for_count(store: StoreClient, path: str, marker: bytes, expected: int) -> int:
|
||||
deadline = time.monotonic() + TIMEOUT
|
||||
last = 0
|
||||
while time.monotonic() < deadline:
|
||||
last = count_marker(store, path, marker)
|
||||
if last >= expected:
|
||||
return last
|
||||
time.sleep(0.5)
|
||||
return last
|
||||
|
||||
|
||||
def check_keep_keep_deduplicates(store: StoreClient) -> None:
|
||||
name = f"dup-keepkeep-{TOKEN}"
|
||||
subject = f"SIEVE06 keep-keep {TOKEN}"
|
||||
install_script(name, b'keep;\r\nkeep;\r\n')
|
||||
try:
|
||||
submit(subject)
|
||||
count = wait_for_count(store, "/mail/INBOX", subject.encode(), 1)
|
||||
if count != 1:
|
||||
raise DuplicateDeliveryError(
|
||||
f'"keep; keep;" delivered {count} copies, expected exactly 1')
|
||||
finally:
|
||||
remove_script(name)
|
||||
|
||||
|
||||
def check_keep_fileinto_inbox_deduplicates(store: StoreClient) -> None:
|
||||
name = f"dup-keepfileinto-{TOKEN}"
|
||||
subject = f"SIEVE06 keep-fileinto-inbox {TOKEN}"
|
||||
install_script(
|
||||
name,
|
||||
b'require ["fileinto"];\r\nkeep;\r\nfileinto "INBOX";\r\n')
|
||||
try:
|
||||
submit(subject)
|
||||
count = wait_for_count(store, "/mail/INBOX", subject.encode(), 1)
|
||||
if count != 1:
|
||||
raise DuplicateDeliveryError(
|
||||
f'"keep; fileinto \\"INBOX\\";" delivered {count} copies, expected exactly 1')
|
||||
finally:
|
||||
remove_script(name)
|
||||
|
||||
|
||||
def check_distinct_targets_both_deliver(store: StoreClient) -> None:
|
||||
name = f"dup-distinct-{TOKEN}"
|
||||
subject = f"SIEVE06 distinct-targets {TOKEN}"
|
||||
folder_a = f"/mail/SIEVE06A-{TOKEN}"
|
||||
folder_b = f"/mail/SIEVE06B-{TOKEN}"
|
||||
store.Create(folder_a)
|
||||
store.Create(folder_b)
|
||||
install_script(
|
||||
name,
|
||||
(
|
||||
'require ["fileinto"];\r\n'
|
||||
f'fileinto "SIEVE06A-{TOKEN}";\r\n'
|
||||
f'fileinto "SIEVE06B-{TOKEN}";\r\n'
|
||||
).encode())
|
||||
try:
|
||||
submit(subject)
|
||||
count_a = wait_for_count(store, folder_a, subject.encode(), 1)
|
||||
count_b = wait_for_count(store, folder_b, subject.encode(), 1)
|
||||
count_inbox = count_marker(store, "/mail/INBOX", subject.encode())
|
||||
if count_a != 1 or count_b != 1:
|
||||
raise DuplicateDeliveryError(
|
||||
"two distinct fileinto targets did not each receive exactly "
|
||||
f"one copy: A={count_a} B={count_b}")
|
||||
if count_inbox != 0:
|
||||
raise DuplicateDeliveryError(
|
||||
"explicit fileinto actions did not suppress the implicit keep: "
|
||||
f"INBOX also received {count_inbox} copies")
|
||||
finally:
|
||||
remove_script(name)
|
||||
store.Remove(folder_a)
|
||||
store.Remove(folder_b)
|
||||
|
||||
|
||||
def main() -> int:
|
||||
if not ALLOW_LIVE:
|
||||
raise DuplicateDeliveryError(
|
||||
"set BONGO_ALLOW_LIVE_USER_TEST=1 for disposable live accounts")
|
||||
if not PASSWORD:
|
||||
raise DuplicateDeliveryError("BONGO_TEST_PASSWORD is required")
|
||||
|
||||
store = StoreClient(
|
||||
RECIPIENT, RECIPIENT, authPassword=PASSWORD, host=HOST, port=STORE_PORT)
|
||||
try:
|
||||
check_keep_keep_deduplicates(store)
|
||||
check_keep_fileinto_inbox_deduplicates(store)
|
||||
check_distinct_targets_both_deliver(store)
|
||||
finally:
|
||||
store.Quit()
|
||||
|
||||
print(
|
||||
"SIEVE-06 PASS "
|
||||
f"host={HOST} "
|
||||
"keep-keep=single-copy "
|
||||
"keep-fileinto-inbox=single-copy "
|
||||
"distinct-fileinto-targets=each-delivered,implicit-keep-suppressed"
|
||||
)
|
||||
return 0
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
try:
|
||||
raise SystemExit(main())
|
||||
except (DuplicateDeliveryError, OSError, smtplib.SMTPException, ssl.SSLError) as error:
|
||||
print(f"SIEVE-06 FAIL: {type(error).__name__}: {error}")
|
||||
raise SystemExit(1)
|
||||
@@ -208,7 +208,7 @@ review does not mark an open live test as passed.
|
||||
| SIEVE-03 | SETACTIVE, RENAME, DELETE, LOGOUT, and persistence | [PASS](test-evidence/0.7-r1.md#sieve-03) | | |
|
||||
| SIEVE-04 | fileinto, envelope, body, variables, vacation extensions | [PASS](test-evidence/0.7-r1.md#sieve-04) | | |
|
||||
| SIEVE-05 | Invalid syntax, quota/size, duplicate names, traversal, auth | [PASS](test-evidence/0.7-r1.md#sieve-05) | | |
|
||||
| SIEVE-06 | Dovecot Pigeonhole language/action vectors and Cyrus Cassandane ManageSieve protocol comparisons | | | |
|
||||
| SIEVE-06 | Dovecot Pigeonhole language/action vectors and Cyrus Cassandane ManageSieve protocol comparisons | [PASS](test-evidence/0.7-r1.md#sieve-06) | | |
|
||||
|
||||
## Authentication and identity
|
||||
|
||||
|
||||
@@ -5073,3 +5073,114 @@ native CTest cases passed against a strict `-Wall -Wextra -Werror`
|
||||
debug build after each fix, the script was run twice consecutively end
|
||||
to end with identical results, and `bongo.service` remained active
|
||||
throughout.
|
||||
|
||||
## SIEVE-06
|
||||
|
||||
Result: **PASS**
|
||||
|
||||
This block is a synthesis of the Sieve/ManageSieve reference-implementation
|
||||
comparisons made across SIEVE-01 through SIEVE-05, plus a dedicated new
|
||||
comparison against Cyrus's Cassandane test suite that surfaced one further
|
||||
real, live-confirmed Bongo bug.
|
||||
|
||||
### Dovecot Pigeonhole comparisons (summary)
|
||||
|
||||
Pigeonhole source was cloned fresh (`git clone --depth 1
|
||||
https://github.com/dovecot/pigeonhole.git`) and read directly, not
|
||||
recalled from memory, for every comparison below:
|
||||
|
||||
* **SIEVE-01**: greeting/`STARTTLS`/pipeline-discard/SASL behavior
|
||||
compared against `src/managesieve/managesieve-client.c` and the
|
||||
general IMAP/POP3/SMTP STARTTLS-discard pattern already established in
|
||||
this codebase.
|
||||
* **SIEVE-02**: the `HAVESPACE` empty-script fix matched Pigeonhole's
|
||||
`cmd-havespace.c`, which explicitly special-cases `size == 0` with
|
||||
`"Cannot upload empty script."` rather than accepting it.
|
||||
* **SIEVE-03**: the `DELETESCRIPT` `NONEXISTENT`/`ACTIVE` fix matched
|
||||
`sieve_script_delete()` in `src/lib-sieve/sieve-script.c`, which raises
|
||||
`SIEVE_ERROR_ACTIVE` only when the script is genuinely active, and
|
||||
`managesieve-client.c`, which maps `SIEVE_ERROR_NOT_FOUND` to the
|
||||
`NONEXISTENT` response code separately; `SETACTIVE`'s idempotent
|
||||
re-activation of an already-active script matched
|
||||
`cmd_setactive_activate()`'s explicit "already active" no-op path.
|
||||
* **SIEVE-04**: confirmed independently against Mailutils' own `sieve`
|
||||
reference tool (not Pigeonhole) that `envelope`/`body` cannot be made
|
||||
to work from Bongo's side at all, since the limitation is in the
|
||||
Sieve engine's `require` resolution, not anything Bongo controls.
|
||||
* **SIEVE-05**: the `".."` script-name finding traced directly to
|
||||
`sieve_script_name_is_valid()` in `src/lib-sieve/sieve-script.c`,
|
||||
which rejects `'/'` specifically because Pigeonhole scripts are real
|
||||
files on disk -- confirming Bongo's SQLite-row-keyed store has no
|
||||
equivalent risk and the test's original assumption, not Bongo, was
|
||||
wrong.
|
||||
|
||||
### Cyrus Cassandane comparison (new)
|
||||
|
||||
Cassandane was cloned fresh (`git clone --depth 1
|
||||
https://github.com/cyrusimap/cassandane.git`) and its Sieve test module,
|
||||
`Cassandane/Cyrus/Sieve.pm`, was read directly. Two of its regression
|
||||
tests, `test_dup_keep_keep` and `test_dup_keep_fileinto`, assert that a
|
||||
script producing more than one delivery action targeting the same
|
||||
mailbox -- `keep; keep;` or `keep; fileinto "INBOX";` -- delivers exactly
|
||||
one copy of the message, not one copy per action.
|
||||
|
||||
Reproducing both scripts live against Bongo (via a disposable ManageSieve
|
||||
script for `test2` and a real authenticated SMTP submission) showed
|
||||
Bongo delivering **two** copies of the message for both cases: `SieveAction()`
|
||||
in `src/agents/rules/rules.c` sent one independent `QMOD RAW`/`QMOD
|
||||
MAILBOX` recipient line per Sieve action, with no deduplication, so the
|
||||
queue's rewritten envelope carried two recipient lines for the one real
|
||||
recipient and delivered to both. Fixed (commit `d68cbc02`) by tracking
|
||||
which mailboxes a given message's execution has already delivered into
|
||||
(`SieveExecution.filedMailboxes`) and skipping a second `QMOD` command
|
||||
for a repeat target, while still marking the action as "delivery
|
||||
happened" so the implicit-keep fallback does not add a third copy.
|
||||
|
||||
A new reusable script, `contrib/testing/sieve-duplicate-delivery-check.py`,
|
||||
covers both Cassandane cases live plus a negative control (two `fileinto`
|
||||
calls to two genuinely *different* folders must still each deliver their
|
||||
own copy, and must still correctly suppress the implicit keep to INBOX,
|
||||
confirming the fix does not over-deduplicate):
|
||||
|
||||
```sh
|
||||
BONGO_ALLOW_LIVE_USER_TEST=1 BONGO_TEST_PASSWORD='<disposable-secret>' \
|
||||
python3 contrib/testing/sieve-duplicate-delivery-check.py
|
||||
```
|
||||
|
||||
Both the `keep; keep;` and `keep; fileinto "INBOX";` scripts now deliver
|
||||
exactly one copy; two distinct `fileinto` targets each still receive
|
||||
exactly one copy with nothing landing in INBOX. All 109 native CTest
|
||||
cases passed against a strict `-Wall -Wextra -Werror` debug build after
|
||||
the fix, the script was run twice consecutively end to end with
|
||||
identical results, and `bongo.service` remained active throughout.
|
||||
|
||||
### Summary of the SIEVE block
|
||||
|
||||
Across SIEVE-01 through SIEVE-06, six real Bongo bugs were found and
|
||||
fixed, all confirmed live against the running server and cross-checked
|
||||
against at least one mature reference implementation (Pigeonhole,
|
||||
Cassandane, or Mailutils' own reference tool):
|
||||
|
||||
1. `HAVESPACE` answered `OK` for a zero-byte script it could never
|
||||
actually store (SIEVE-02, commit `a3aae7ca`).
|
||||
2. `DELETESCRIPT` could not distinguish `NONEXISTENT` from `ACTIVE`
|
||||
(SIEVE-02, commit `a3aae7ca`).
|
||||
3. `fileinto` and the classic `MOVE` filing rule never delivered
|
||||
anything at all -- every message using either action was stuck in
|
||||
the queue forever (SIEVE-04, commit `d9b13445`).
|
||||
4. `vacation` silently no-op'd and `reject` hard-failed back to a plain
|
||||
keep, unconditionally, for every script (SIEVE-04, commit
|
||||
`bad1b1e8`).
|
||||
5. `ConnReadLine()`/`ConnReadAnswer()` hung forever on a command line
|
||||
containing an embedded NUL byte instead of answering `SYNTAX`
|
||||
(SIEVE-05, commit `bdb9ab36`); a related overlong-line
|
||||
nul-termination bug in the same function was fixed alongside it
|
||||
(SIEVE-05, commit `d7dced29`).
|
||||
6. Repeated delivery actions to the same mailbox produced duplicate
|
||||
messages instead of one copy (SIEVE-06, commit `d68cbc02`).
|
||||
|
||||
One further limitation was found to be outside Bongo's own source
|
||||
entirely -- the bundled Mailutils Sieve engine cannot execute `envelope`
|
||||
or `body` regardless of how Bongo drives it -- and is tracked in
|
||||
`ROADMAP.md` as a follow-up engine-replacement project (a pinned Cyrus
|
||||
`libsieve` submodule) rather than papered over.
|
||||
|
||||
Reference in New Issue
Block a user