chore: placement model follow-ups (reap on repoint, claim lifetime, sync naming) #413

2026-08-14T22:41:34Z

KofTwentyTwo commented

2026-08-14 22:41:34 +00:00

(Migrated from github.com)

Three placement-model follow-ups: one confidentiality fix, one documented decision, one round of housekeeping. One commit per issue.

#406 — a repointed placement stranded its old message (confidentiality)

save_email_at's placement upsert ends in DO UPDATE SET message_key = excluded.message_key, .... If the same (account, folder, uidvalidity, uid) slot was re-saved under a different message_key, the previous message silently lost that placement without passing through delete_placements. If it was its last one, nothing would ever delete the messages row — and since messages_fts holds the decrypted plaintext body and is only cleared by the messages_ad trigger on DELETE FROM messages, the body stayed readable to anyone with filesystem access to the database, indefinitely. This was the one write path that bypassed the reap the placement model exists to guarantee.

The fix reads the slot's current occupant key instead of just its existence (the COUNT(*) that fed placement_is_new became a SELECT message_key, so placement_is_new is unchanged), and, in the same transaction as the upsert, reaps a displaced key on exactly the terms delete_placements uses:

DELETE FROM messages WHERE message_key = ?
 AND NOT EXISTS (SELECT 1 FROM placements WHERE message_key = ?)

So the message goes only when nothing points at it any more; the trigger clears FTS as a consequence. No new reap semantics were invented.

Two tests, both mutation-verified:

repointing_a_slot_reaps_the_displaced_message_and_its_plaintext_index — repoint strands the old message; messages row and messages_fts row must both be gone. Disabling the reap fails exactly this test.
repointing_a_slot_keeps_a_displaced_message_that_still_lives_elsewhere — the old message still holds an Archive placement, so it must survive with its FTS row intact. Dropping the NOT EXISTS guard (reaping unconditionally) fails exactly this test.

#409 — filter fire claims are permanent, deliberately (no behaviour change)

filter_fired (rule_id, message_key, fired_at) is never reaped, so a claim outlives the message it was made against. This PR changes no behaviour — it documents the decision on claim_filter_fire, whose doc comment explained the claim but not its lifetime.

The decision is to leave it as-is. Reaping alongside the message would let a remote party re-trigger third-party side effects at will: a mailing-list message deleted and re-delivered would FORWARD again. Under content-addressed identity a byte-identical redelivery genuinely is the same message, so declining to re-fire is the honest reading rather than a missed event. And the failure direction this leaves open — "a rule that should have fired again did not" — is the safe one for actions that reach outside the mailbox, where the priority is that side-effecting actions never double-fire.

The cost is documented too: a table that only grows, bounded by rules × messages ever seen at roughly 150 bytes a row, cleared only when an identity schema bump resets user_version. No TTL, no reaping.

#410 — housekeeping (three parts)

1. Rename. sync_folder_messages{,_with_session,_batched} return FolderChanges (upserts, removals, presence, next checkpoint), not messages. Renamed to sync_folder_changes*; all call sites are within crates/nuncio-mail/src/imap.rs and the return type is explicit everywhere.

2. export_mbox's dropped X-Nuncio-Folder header — chose to leave it out and sharpen the comment to a permanent omission. Not because threading Placement in would ripple (it would: export_messages_to_file hands the same &[Email] to all four formats), but because the header cannot be made correct. A message may hold several placements at once — the same message filed in INBOX and Archive is one message, not two — so a single X-Nuncio-Folder could only ever name one of them, and MBOX has no way to spell "these several". That would be a wrong answer dressed as an authoritative one. Callers needing occupancy read it from the store, where it is complete. X-Nuncio-Account stays, because an account is single-valued on the message.

3. UID-less guard in the two body-fetch loops. The enumeration pass filters uid >= 1 and counts what it skips; the body and headers-only fetch loops did not, so a FETCH item arriving without a UID would write a junk placement at remote_id: "0". Both loops now mirror the enumeration skip and feed the same skipped counter in the completion log. RFC 3501 section 6.4.8 forbids a server omitting UID in a UID FETCH response, so this needs a non-conforming server to trigger — hardened anyway.

Gates

cargo fmt --all -- --check, cargo check-all, and cargo test-all are green at each commit. The tracing-race flake fix also holds: RUST_TEST_THREADS=2 cargo test-all run three times, zero FAILED lines each.

Closes #406
Closes #409
Closes #410

Three placement-model follow-ups: one confidentiality fix, one documented decision, one round of housekeeping. One commit per issue. ## #406 — a repointed placement stranded its old message (confidentiality) `save_email_at`'s placement upsert ends in `DO UPDATE SET message_key = excluded.message_key, ...`. If the same `(account, folder, uidvalidity, uid)` slot was re-saved under a *different* `message_key`, the previous message silently lost that placement without passing through `delete_placements`. If it was its last one, nothing would ever delete the `messages` row — and since `messages_fts` holds the **decrypted plaintext** body and is only cleared by the `messages_ad` trigger on `DELETE FROM messages`, the body stayed readable to anyone with filesystem access to the database, indefinitely. This was the one write path that bypassed the reap the placement model exists to guarantee. The fix reads the slot's current occupant key instead of just its existence (the `COUNT(*)` that fed `placement_is_new` became a `SELECT message_key`, so `placement_is_new` is unchanged), and, in the same transaction as the upsert, reaps a displaced key on exactly the terms `delete_placements` uses: ```sql DELETE FROM messages WHERE message_key = ? AND NOT EXISTS (SELECT 1 FROM placements WHERE message_key = ?) ``` So the message goes only when nothing points at it any more; the trigger clears FTS as a consequence. No new reap semantics were invented. Two tests, both mutation-verified: - `repointing_a_slot_reaps_the_displaced_message_and_its_plaintext_index` — repoint strands the old message; `messages` row and `messages_fts` row must both be gone. Disabling the reap fails exactly this test. - `repointing_a_slot_keeps_a_displaced_message_that_still_lives_elsewhere` — the old message still holds an Archive placement, so it must survive with its FTS row intact. Dropping the `NOT EXISTS` guard (reaping unconditionally) fails exactly this test. ## #409 — filter fire claims are permanent, deliberately (no behaviour change) `filter_fired (rule_id, message_key, fired_at)` is never reaped, so a claim outlives the message it was made against. This PR **changes no behaviour** — it documents the decision on `claim_filter_fire`, whose doc comment explained the claim but not its lifetime. The decision is to leave it as-is. Reaping alongside the message would let a remote party re-trigger third-party side effects at will: a mailing-list message deleted and re-delivered would `FORWARD` again. Under content-addressed identity a byte-identical redelivery genuinely *is* the same message, so declining to re-fire is the honest reading rather than a missed event. And the failure direction this leaves open — "a rule that should have fired again did not" — is the safe one for actions that reach outside the mailbox, where the priority is that side-effecting actions never double-fire. The cost is documented too: a table that only grows, bounded by rules × messages ever seen at roughly 150 bytes a row, cleared only when an identity schema bump resets `user_version`. No TTL, no reaping. ## #410 — housekeeping (three parts) **1. Rename.** `sync_folder_messages{,_with_session,_batched}` return `FolderChanges` (upserts, removals, presence, next checkpoint), not messages. Renamed to `sync_folder_changes*`; all call sites are within `crates/nuncio-mail/src/imap.rs` and the return type is explicit everywhere. **2. `export_mbox`'s dropped `X-Nuncio-Folder` header — chose to leave it out and sharpen the comment to a permanent omission.** Not because threading `Placement` in would ripple (it would: `export_messages_to_file` hands the same `&[Email]` to all four formats), but because the header cannot be made correct. A message may hold several placements at once — the same message filed in INBOX and Archive is one message, not two — so a single `X-Nuncio-Folder` could only ever name one of them, and MBOX has no way to spell "these several". That would be a wrong answer dressed as an authoritative one. Callers needing occupancy read it from the store, where it is complete. `X-Nuncio-Account` stays, because an account *is* single-valued on the message. **3. UID-less guard in the two body-fetch loops.** The enumeration pass filters `uid >= 1` and counts what it skips; the body and headers-only fetch loops did not, so a FETCH item arriving without a UID would write a junk placement at `remote_id: "0"`. Both loops now mirror the enumeration skip and feed the same `skipped` counter in the completion log. RFC 3501 section 6.4.8 forbids a server omitting UID in a `UID FETCH` response, so this needs a non-conforming server to trigger — hardened anyway. ## Gates `cargo fmt --all -- --check`, `cargo check-all`, and `cargo test-all` are green at each commit. The tracing-race flake fix also holds: `RUST_TEST_THREADS=2 cargo test-all` run three times, zero `FAILED` lines each. Closes #406 Closes #409 Closes #410

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: KofTwentyTwo/nuncio#413