feat(email-ingress): exactly-once dedup via Message-ID hash (LC-77-MID-DEDUP) #207

Merged

longjacksonle merged 2 commits from feat/lc-77-mid-dedup into main 2026-05-26 02:14:50 +02:00

Conversation 0 Commits 2 Files changed 28 +548 -2

longjacksonle commented 2026-05-26 02:09:50 +02:00

Owner

Copy link

Summary

Closes #202. Adds an exactly-once dedup defense to the LC-77 IMAP poll loop. The v1 posture is at-least-once with \Seen-after-attempt: a crash between (process) and (STORE +Seen) on the same UID re-fetches and re-posts on the next tick. This PR adds a second defense that drops the replay before the actor runs.

Mechanism

• Every successfully-posted message's RFC 5322 Message-ID: header is HMAC-SHA256-hashed under LETS_CHAT_SECRET_KEY and recorded in chat.db::processed_message_ids (only the hash; the plaintext never persists).
• Before each post, process_polled_message checks the table. If the Message-ID is already there, drop with DropReason::Duplicate and post nothing.
• The check runs BEFORE the resolver, so a wire-byte-identical replay drops as duplicate regardless of intervening state changes (revoke, token expiry).
• Sweep at 30 days piggybacks on the hourly orphan sweeper. A sender re-issuing the same Message-ID after that window is treated as fresh.
• A message with no Message-ID: header falls back to v1 at-least-once. RFC 5322 says senders SHOULD include one; almost all real mail does. Documented as a known gap.

Scoping decision (deviates from ticket)

The ticket proposed a (inbox_id, message_id_hash) UNIQUE composite key. After LC-77-REPLY stage 2 shipped, the polled mailbox now serves both the per-room inbox path AND the reply-by-email path, and the reply path has no inbox_id. This PR uses a single global key over message_id_hash alone, which covers both without an awkward NULL/sentinel column. A Message-ID is sender-globally-unique in practice so the global keying adds no false-positive risk.

Tests

• db_email_ingress_dedup.rs (5 round-trip tests: hash stability + keying, unknown-hash lookup, mark-then-is round trip, idempotent mark under racing inserts, 30-day cutoff sweep).
• email_ingress_dedup_path.rs (4 integration tests: same Message-ID twice -> Posted+Duplicate, two distinct Message-IDs both post, no-Message-ID falls back to at-least-once, dedup check runs before resolve so a replay against a revoked inbox still drops as duplicate).
• just test and just test-saas pass (modulo the documented routes_uploads concurrent-load flake; passes in isolation).
• just check clean (fmt + clippy --deny warnings).

Docs

• docs/email-ingress.md removes the deferred entry for exactly-once dedup and adds a new Duplicate suppression section covering the mechanism, coverage gaps, and an operator escape hatch (a DELETE ... WHERE message_id_hash = ? workaround for forcing a re-process).
• CLAUDE.md gets a one-paragraph note.

Test plan

• just test
• just test-saas
• just check
• Manual: send an email to the inbox address, confirm it posts; manually edit the IMAP \Seen flag back to unseen on the source mailbox to force a re-fetch; confirm the second tick drops with reason=duplicate and posts nothing additional.

## Summary Closes #202. Adds an exactly-once dedup defense to the LC-77 IMAP poll loop. The v1 posture is at-least-once with `\Seen`-after-attempt: a crash between (process) and (STORE +Seen) on the same UID re-fetches and re-posts on the next tick. This PR adds a second defense that drops the replay before the actor runs. ## Mechanism - Every successfully-posted message's RFC 5322 `Message-ID:` header is HMAC-SHA256-hashed under `LETS_CHAT_SECRET_KEY` and recorded in `chat.db::processed_message_ids` (only the hash; the plaintext never persists). - Before each post, `process_polled_message` checks the table. If the Message-ID is already there, drop with `DropReason::Duplicate` and post nothing. - The check runs BEFORE the resolver, so a wire-byte-identical replay drops as `duplicate` regardless of intervening state changes (revoke, token expiry). - Sweep at 30 days piggybacks on the hourly orphan sweeper. A sender re-issuing the same Message-ID after that window is treated as fresh. - A message with no `Message-ID:` header falls back to v1 at-least-once. RFC 5322 says senders SHOULD include one; almost all real mail does. Documented as a known gap. ## Scoping decision (deviates from ticket) The ticket proposed a `(inbox_id, message_id_hash)` UNIQUE composite key. After LC-77-REPLY stage 2 shipped, the polled mailbox now serves both the per-room inbox path AND the reply-by-email path, and the reply path has no `inbox_id`. This PR uses a single global key over `message_id_hash` alone, which covers both without an awkward NULL/sentinel column. A Message-ID is sender-globally-unique in practice so the global keying adds no false-positive risk. ## Tests - `db_email_ingress_dedup.rs` (5 round-trip tests: hash stability + keying, unknown-hash lookup, mark-then-is round trip, idempotent mark under racing inserts, 30-day cutoff sweep). - `email_ingress_dedup_path.rs` (4 integration tests: same Message-ID twice -> Posted+Duplicate, two distinct Message-IDs both post, no-Message-ID falls back to at-least-once, dedup check runs before resolve so a replay against a revoked inbox still drops as `duplicate`). - `just test` and `just test-saas` pass (modulo the documented `routes_uploads` concurrent-load flake; passes in isolation). - `just check` clean (fmt + clippy --deny warnings). ## Docs - `docs/email-ingress.md` removes the deferred entry for exactly-once dedup and adds a new Duplicate suppression section covering the mechanism, coverage gaps, and an operator escape hatch (a `DELETE ... WHERE message_id_hash = ?` workaround for forcing a re-process). - `CLAUDE.md` gets a one-paragraph note. ## Test plan - [x] `just test` - [x] `just test-saas` - [x] `just check` - [ ] Manual: send an email to the inbox address, confirm it posts; manually edit the IMAP \Seen flag back to unseen on the source mailbox to force a re-fetch; confirm the second tick drops with `reason=duplicate` and posts nothing additional.

longjacksonle added 2 commits 2026-05-26 02:09:50 +02:00

feat(email-ingress): processed_message_ids dedup schema + db primitives (LC-77-MID-DEDUP commit 1) ... 2d0a21fd88

Adds the `chat.db::processed_message_ids` table that backs the LC-77 poll loop's exactly-once dedup defense. Migration 0051 plus a new `db::email_ingress_dedup` module with four primitives:

- `hash_message_id(secret_key, msg_id)`: HMAC-SHA256 of the RFC 5322 Message-ID under `LETS_CHAT_SECRET_KEY`. Reuses `auth::hash_api_token` so the at-rest hash format is consistent with per-room inbox secrets and webhook secrets. Privacy: only the hash is stored, never the plaintext, so an operator with DB read access cannot enumerate correspondents.
- `is_processed(pool, hash)`: read-only check used before the actor dispatch.
- `mark_processed(pool, hash)`: `INSERT OR IGNORE` writer used after a successful post. Returns `true` when a new row landed, `false` when the hash was already present (two-concurrent-polls race; the caller can log a duplicate-detected warning without aborting).
- `sweep_old(pool, days)`: drops rows older than `days` days. Wired into the hourly orphan sweeper in commit 2 with a 30-day cutoff.

Scoping decision: the dedup key is GLOBAL across the polled mailbox, not per-inbox. The original ticket proposed `(inbox_id, message_id_hash)` UNIQUE, but the polled mailbox now serves both the per-room inbox path AND the reply-by-email path (LC-77-REPLY stage 2 shipped after the ticket was filed), and the reply path has no `inbox_id`. A single global key covers both without an awkward NULL/sentinel column; in practice a Message-ID is sender-globally-unique enough that scoping it per-inbox adds no real recovery power.

Adds `DropReason::Duplicate` + as_str `"duplicate"` for the operator log when commit 2 wires the dedup check into `process_polled_message`.

Tests: `server/tests/db_email_ingress_dedup.rs` (5 round-trip tests covering hash stability + keying, unknown-hash lookup, mark-then-is round trip, idempotent mark under racing inserts, and 30-day cutoff sweep).

Test migration list updated for the 17 array-form files plus `db_private_rooms.rs` (verbose form) per `CLAUDE.md` Test maintenance category 2; the 40+ `sqlx::migrate!`-backed files pick the migration up automatically.

Commit 2 wires the dedup check into the poll loop, adds the sweeper hook, and updates docs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

feat(email-ingress): wire processed-Message-ID dedup into poll loop (LC-77-MID-DEDUP commit 2) ... 5bd6f8734d

Wires `db::email_ingress_dedup` into `process_polled_message`: before resolving the address, extract the RFC 5322 `Message-ID:` header, HMAC-hash it under `LETS_CHAT_SECRET_KEY`, and check the `processed_message_ids` table. If present, drop with `DropReason::Duplicate` and post nothing. After a successful post on either path (per-room inbox actor OR reply-by-email actor), record the hash. A message with no `Message-ID:` header falls back to v1 at-least-once.

The dedup check runs BEFORE the resolver intentionally: a wire-byte-identical replay should drop as `duplicate` regardless of intervening state changes (revoke, token expiry). The operator log shows the dedup doing its job rather than chasing the new state's failure reason.

Mark_processed and is_processed lookup failures are logged WARN and do NOT abort the process flow: a DB hiccup in the dedup layer must not lose mail. The fallback is v1 at-least-once, which is the v1 posture anyway.

Sweep: `spawn_orphan_sweeper` in `main.rs` now also calls `db::email_ingress_dedup::sweep_old(30)` each hourly tick. Idle deployments (no polled mail) touch zero rows.

Tests: `server/tests/email_ingress_dedup_path.rs` adds four integration tests that drive `process_polled_message` directly:
- Same Message-ID processed twice: first posts, second drops with `duplicate` and inserts no row (the core crash-race scenario).
- Two distinct Message-IDs both post (regression that dedup is keyed correctly).
- Message without a `Message-ID:` header re-posts on second call (the documented at-least-once fallback).
- Ordering check: a replay against a revoked inbox still drops as `duplicate`, not `revoked_inbox`, confirming the dedup check runs before resolve.

Docs: `docs/email-ingress.md` removes the deferred entry for exactly-once dedup and adds a Duplicate suppression (LC-77-MID-DEDUP) section covering the table, the check ordering, coverage gaps (no Message-ID, sender-controlled hash, 30-day TTL), and the operator surface (a `DELETE ... WHERE message_id_hash = ?` workaround for forcing a re-process). `CLAUDE.md` gets a one-paragraph note.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

longjacksonle merged commit 653cd97397 into main 2026-05-26 02:14:50 +02:00

longjacksonle deleted branch feat/lc-77-mid-dedup 2026-05-26 02:14:51 +02:00

longjacksonle referenced this pull request from a commit 2026-05-26 02:14:52 +02:00

Merge pull request 'feat(email-ingress): exactly-once dedup via Message-ID hash (LC-77-MID-DEDUP)' (#207) from feat/lc-77-mid-dedup into main

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

psa-systems/lets-chat!207

No description provided.