Unqualified EXPUNGE in MoveMessages can permanently delete unrelated messages

[bscott]

Found while investigating #28. This is a data-loss path, not a style issue.

The interaction

DeleteMessages with permanent=false marks messages \Deleted and deliberately does not expunge (internal/imap/client.go:548):

// Expunge if permanent
if permanent {
    if err := c.client.Expunge().Close(); err != nil {

MoveMessages then issues an unqualified EXPUNGE (internal/imap/client.go:651):

if err := c.client.Expunge().Close(); err != nil {

Per RFC 3501, EXPUNGE permanently removes every message in the selected mailbox carrying \Deleted — not only the ones this move just marked.

Consequence

pm-cli mail delete 123          # soft: marks \Deleted, no expunge
pm-cli mail move 456 -d Archive # expunges 123 as a side effect, permanently

Message 123 is destroyed by a command that never named it. The same applies to any message left \Deleted by another client sharing the mailbox.

mail archive and mail batch's move/archive operations all route through MoveMessages, so they carry the same behavior.

Fix

Adopting native MOVE (#28) removes the explicit EXPUNGE entirely and resolves this. If the COPY fallback is kept for any reason, it must use UIDExpunge(uidSet) when UIDPLUS is available — which is exactly what imapclient's own fallback does, and what our hand-rolled version omits.

Bridge advertises UIDPLUS, so UIDEXPUNGE is available in practice.

Verification status

Identified by code inspection and RFC semantics; not reproduced against a live server. The Bridge account used for 0.2.6 validation was removed afterward, so I could not confirm how Bridge maps \Deleted — it may translate soft deletes to a Trash move rather than leaving the flag set, which would narrow real-world exposure. That should be checked before deciding severity, but the unqualified EXPUNGE is wrong regardless of how Bridge behaves, and pm-cli should not depend on a server-specific mapping to be safe.

Reproducible against imapmemserver without a live account: mark one message \Deleted, move a different one, assert the first still exists.

[bscott]

Reproduced. Upgrading this from "identified by inspection" to confirmed.

Ran against imapmemserver (IMAP4rev2, so UIDPLUS available) using the existing fixture from client_affected_test.go:

after soft delete of A: 2 message(s) still in INBOX
after moving B:         0 message(s) in INBOX; A survived = false
CONFIRMED: moving message B permanently expunged unrelated message A (uid 1),
which the move never named

Message A was soft-deleted (flag set, still present — the listing confirms 2 messages remained). Moving an entirely different message then destroyed it.

The reproduction, ready to drop into internal/imap/ as a regression test once fixed:

func TestUnqualifiedExpungeDestroysUnrelatedMessage(t *testing.T) {
	fx, uidA := newIMAPFixture(t) // seeds message A
	uidB := appendMessage(t, fx.raw, "INBOX", "second@example.com")

	// Soft delete A: marks \\Deleted, does NOT expunge.
	if err := fx.client.DeleteMessages("INBOX", []string{presentSelector(uidA)}, false); err != nil {
		t.Fatalf("soft delete of A: %v", err)
	}

	// Move B somewhere else. This names only B.
	if err := fx.client.MoveMessages("INBOX", []string{presentSelector(uidB)}, "Archive"); err != nil {
		t.Fatalf("move of B: %v", err)
	}

	after, err := fx.client.ListMessages(ListOptions{Mailbox: "INBOX", Limit: 50})
	if err != nil {
		t.Fatal(err)
	}
	for _, m := range after {
		if imap.UID(m.UID) == uidA {
			return // A survived, as it must
		}
	}
	t.Errorf("moving B expunged unrelated message A (uid %d)", uidA)
}

One caveat stands: this proves the behavior in pm-cli's own code path. It does not prove Proton Bridge leaves \\Deleted set on a soft delete rather than translating it to a Trash move — that determines real-world exposure for Bridge users specifically, and I could not check it because the test account was removed. The defect in MoveMessages is server-independent either way.