M6.5 [P2]: Graceful bulk-path handling when one stored body is corrupted #265

New Issue

2026-07-31T16:07:15Z

KofTwentyTwo commented

2026-07-31 16:07:15 +00:00

(Migrated from github.com)

Follow-up from #237 (decrypt fail-closed). Now that decrypt fails closed, a SINGLE corrupted/tampered body ciphertext makes bulk operations abort entirely: notably backfill_message_fts runs during migration, so one bit-rotted row fails the whole migration and the daemon won't boot; list_messages/get_message_chunk/list_messages_for_export likewise error the whole operation on one bad row.

Single-item get_message SHOULD stay fail-closed (Err) — that's correct. But bulk paths shouldn't be bricked by one bad row. Goal: for the bulk paths, SURFACE the corruption honestly (WARN-log the specific message id) but do NOT abort the whole operation / block boot — skip or mark the affected row (e.g. include it with an explicit 'body unavailable: decryption failed' marker rather than a silently-empty body, or exclude it with a logged warning). Export must not silently drop a message — represent the corrupted one explicitly. Never fabricate an empty body. This makes a security-first engine robust to isolated bit-rot without hiding tampering. Source: audit follow-up.

Follow-up from #237 (decrypt fail-closed). Now that decrypt fails closed, a SINGLE corrupted/tampered body ciphertext makes bulk operations abort entirely: notably backfill_message_fts runs during migration, so one bit-rotted row fails the whole migration and the daemon won't boot; list_messages/get_message_chunk/list_messages_for_export likewise error the whole operation on one bad row. Single-item get_message SHOULD stay fail-closed (Err) — that's correct. But bulk paths shouldn't be bricked by one bad row. Goal: for the bulk paths, SURFACE the corruption honestly (WARN-log the specific message id) but do NOT abort the whole operation / block boot — skip or mark the affected row (e.g. include it with an explicit 'body unavailable: decryption failed' marker rather than a silently-empty body, or exclude it with a logged warning). Export must not silently drop a message — represent the corrupted one explicitly. Never fabricate an empty body. This makes a security-first engine robust to isolated bit-rot without hiding tampering. Source: audit follow-up.

KofTwentyTwo commented

2026-08-15 02:14:51 +00:00

(Migrated from github.com)

Fixed in #429 (c484fb0), merged to dev.

Bulk paths now return a per-item MessageRead = Result<Email, UnreadableBody>: the row keeps its slot, never carries a fabricated body, and is named to the caller. get_message is unchanged and still fails closed — its test kept and re-asserted alongside the new bulk tests.

Per-item was chosen over skipping for a concrete reason: filtering unreadable rows out would shorten pages below the query window, corrupting keyset cursors and chunk boundaries — real data loss, not just a cosmetic gap.

FTS backfill skips the row rather than indexing an empty body, so boot proceeds and the row self-heals on re-sync. Export omits and names the ids, since an archive outlives the store.

Proto change is strictly additive (28 insertions, zero deletions); the descriptor golden was regenerated by the test's own instructions. Note MessageWithPlacement changed shape to (MessageRead, Placement) — a breaking store-crate API change, with all in-repo callers updated.

Fixed in #429 (`c484fb0`), merged to `dev`. Bulk paths now return a per-item `MessageRead = Result<Email, UnreadableBody>`: the row keeps its slot, never carries a fabricated body, and is named to the caller. `get_message` is unchanged and still fails closed — its test kept and re-asserted alongside the new bulk tests. **Per-item was chosen over skipping for a concrete reason:** filtering unreadable rows out would shorten pages below the query window, corrupting keyset cursors and chunk boundaries — real data loss, not just a cosmetic gap. FTS backfill skips the row rather than indexing an empty body, so boot proceeds and the row self-heals on re-sync. Export omits and names the ids, since an archive outlives the store. Proto change is strictly additive (28 insertions, zero deletions); the descriptor golden was regenerated by the test's own instructions. Note `MessageWithPlacement` changed shape to `(MessageRead, Placement)` — a breaking store-crate API change, with all in-repo callers updated.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: KofTwentyTwo/nuncio#265