fix(reader): delete from reader bypasses trash policy and permanently expunges without confirmation #481

Open
opened 2026-07-10 19:14:04 +00:00 by JMR-dev · 0 comments
JMR-dev commented 2026-07-10 19:14:04 +00:00 (Migrated from github.com)

Verified finding(s) from the 2026-07-09 whole-repo multi-agent review (independent finder, then adversarial verifier; verdict CONFIRMED).

Triage: above the cut — fix dispatched immediately; this issue tracks the fix to Done.

app/src/main/kotlin/org/libremail/ui/reader/ReaderViewModel.kt:183 — critical

The delete-disposition policy (move to Trash unless already in Trash/Spam, after confirmation) is implemented only in MailboxViewModel.requestDelete instead of in the repository, so the reader's delete() calls the raw repository.deleteMessage(), which performs an unconfirmed, permanent server-side UID EXPUNGE from any folder.

Failure scenario: User opens a message from their INBOX in the reader and taps the Delete icon in the app bar: ReaderViewModel.delete() -> MailRepositoryImpl.deleteMessage -> ImapClient.deleteMessages flags \Deleted and issues UID EXPUNGE, permanently destroying the message on the server with no confirmation dialog and no Trash copy. The identical message deleted from the mailbox list is confirmed first and moved to Trash (recoverable) via mailRepository.trash(). Because the policy sits in one ViewModel rather than the shared repository, every new surface that deletes (reader today, swipe-to-delete or notifications tomorrow) silently gets the destructive path.

Verifier justification (CONFIRMED): The full destructive chain exists exactly as claimed: ReaderScreen.kt:153 binds the Delete icon directly to viewModel::delete with no confirmation dialog anywhere in ReaderScreen (grep for Dialog/confirm finds none); ReaderViewModel.kt:183 calls repository.deleteMessage(messageId); MailRepositoryImpl.deleteMessage (line 386) invokes imapClient.deleteMessage on the message's source folder with no folder-role check or Trash routing; and ImapClient.deleteMessages (lines 410-411) flags \Deleted and issues a targeted UID EXPUNGE — its own KDoc says "Permanently deletes". Meanwhile MailboxViewModel.requestDelete/confirmPending (lines 285-305) confirms first and routes to mailRepository.trash() unless the folder is already Trash/Spam, only then expunging with the dialog marked permanent. So a single tap on Delete in the reader permanently destroys an INBOX message server-side with no confirmation and no Trash copy, while the same message deleted from the list view is confirmed and recoverable. Existing tests (ReaderScreenJvmTest.tappingDelete_deletesAndInvokesOnBack, ReaderViewModelActionsTest) merely assert the destructive call happens, confirming the behavior rather than guarding against it. Not covered by any known-intentional decision.

Defective line: ReaderViewModel.kt:183: repository.deleteMessage(messageId) — reached from ReaderScreen.kt:153: IconButton(onClick = viewModel::delete), landing in ImapClient.deleteMessages: mailbox.setFlags(messages, Flags(Flags.Flag.DELETED), true); expungeTargeted(mailbox, messages)

Fix hint: Hoist the disposition policy into the repository (e.g. MailRepository.deleteMessage resolves the message's folder role and delegates to trash(listOf(id)) unless the role is TRASH/SPAM, only then expunging), and add a pendingConfirm-style confirmation dialog to ReaderScreen/ReaderViewModel mirroring MailboxViewModel.requestDelete/confirmPending so every delete surface confirms and gets Trash-recoverability by default.

Verified finding(s) from the 2026-07-09 whole-repo multi-agent review (independent finder, then adversarial verifier; verdict **CONFIRMED**). **Triage: above the cut — fix dispatched immediately; this issue tracks the fix to Done.** ## `app/src/main/kotlin/org/libremail/ui/reader/ReaderViewModel.kt:183` — critical The delete-disposition policy (move to Trash unless already in Trash/Spam, after confirmation) is implemented only in MailboxViewModel.requestDelete instead of in the repository, so the reader's delete() calls the raw repository.deleteMessage(), which performs an unconfirmed, permanent server-side UID EXPUNGE from any folder. **Failure scenario:** User opens a message from their INBOX in the reader and taps the Delete icon in the app bar: ReaderViewModel.delete() -> MailRepositoryImpl.deleteMessage -> ImapClient.deleteMessages flags \Deleted and issues UID EXPUNGE, permanently destroying the message on the server with no confirmation dialog and no Trash copy. The identical message deleted from the mailbox list is confirmed first and moved to Trash (recoverable) via mailRepository.trash(). Because the policy sits in one ViewModel rather than the shared repository, every new surface that deletes (reader today, swipe-to-delete or notifications tomorrow) silently gets the destructive path. **Verifier justification (CONFIRMED):** The full destructive chain exists exactly as claimed: ReaderScreen.kt:153 binds the Delete icon directly to viewModel::delete with no confirmation dialog anywhere in ReaderScreen (grep for Dialog/confirm finds none); ReaderViewModel.kt:183 calls repository.deleteMessage(messageId); MailRepositoryImpl.deleteMessage (line 386) invokes imapClient.deleteMessage on the message's source folder with no folder-role check or Trash routing; and ImapClient.deleteMessages (lines 410-411) flags \Deleted and issues a targeted UID EXPUNGE — its own KDoc says "Permanently deletes". Meanwhile MailboxViewModel.requestDelete/confirmPending (lines 285-305) confirms first and routes to mailRepository.trash() unless the folder is already Trash/Spam, only then expunging with the dialog marked permanent. So a single tap on Delete in the reader permanently destroys an INBOX message server-side with no confirmation and no Trash copy, while the same message deleted from the list view is confirmed and recoverable. Existing tests (ReaderScreenJvmTest.tappingDelete_deletesAndInvokesOnBack, ReaderViewModelActionsTest) merely assert the destructive call happens, confirming the behavior rather than guarding against it. Not covered by any known-intentional decision. **Defective line:** `ReaderViewModel.kt:183: repository.deleteMessage(messageId) — reached from ReaderScreen.kt:153: IconButton(onClick = viewModel::delete), landing in ImapClient.deleteMessages: mailbox.setFlags(messages, Flags(Flags.Flag.DELETED), true); expungeTargeted(mailbox, messages)` **Fix hint:** Hoist the disposition policy into the repository (e.g. MailRepository.deleteMessage resolves the message's folder role and delegates to trash(listOf(id)) unless the role is TRASH/SPAM, only then expunging), and add a pendingConfirm-style confirmation dialog to ReaderScreen/ReaderViewModel mirroring MailboxViewModel.requestDelete/confirmPending so every delete surface confirms and gets Trash-recoverability by default.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: JMR-dev/LibreMail#481