diff --git a/lib/__tests__/message-list-tabs-store.test.ts b/lib/__tests__/message-list-tabs-store.test.ts index ba972d6b8..8a4e914f6 100644 --- a/lib/__tests__/message-list-tabs-store.test.ts +++ b/lib/__tests__/message-list-tabs-store.test.ts @@ -8,6 +8,7 @@ import { resolveTabFilter, } from '@/stores/message-list-tabs-store'; import { useEmailStore } from '@/stores/email-store'; +import { useAuthStore } from '@/stores/auth-store'; import { messageListTabHooks } from '@/lib/plugin-hooks'; import { jmapError } from '@/lib/jmap/jmap-error'; import type { IJMAPClient } from '@/lib/jmap/client-interface'; @@ -191,6 +192,11 @@ describe('categorizeEmails', () => { from: [{ email: `${id}@example.com` }], ...(sourceAccountId ? { accountId: 'login-1', sourceClientAccountId: 'login-1', sourceAccountId } : {}), }) as unknown as Email; + // The stamped login is connected, through this client. + const loginClient = (client: Partial) => { + useAuthStore.setState({ getClientForAccount: (id: string) => (id === 'login-1' ? client : undefined) } as never); + return client as IJMAPClient; + }; const PATCH = { 'keywords/$category-updates': null, 'keywords/$category-promotions': true }; beforeEach(() => { @@ -231,7 +237,7 @@ describe('categorizeEmails', () => { const emitted = vi.spyOn(messageListTabHooks.onEmailCategorize, 'emit'); await expect(useMessageListTabsStore.getState() - .categorizeEmails({ batchUpdateKeywords } as unknown as IJMAPClient, ['m1', 'm2'], 'promotions')).rejects.toBe(refusal); + .categorizeEmails(loginClient({ batchUpdateKeywords }), ['m1', 'm2'], 'promotions')).rejects.toBe(refusal); // Both accounts were written; only the one the server accepted moved. expect(batchUpdateKeywords).toHaveBeenCalledTimes(2); @@ -265,7 +271,7 @@ describe('categorizeEmails', () => { const batchUpdateKeywords = vi.fn().mockResolvedValue(undefined); await useMessageListTabsStore.getState() - .categorizeEmails({ batchUpdateKeywords } as unknown as IJMAPClient, ['m2'], 'promotions'); + .categorizeEmails(loginClient({ batchUpdateKeywords }), ['m2'], 'promotions'); expect(batchUpdateKeywords).toHaveBeenCalledWith(['m2'], PATCH, 'shared'); expect(batchUpdateKeywords).not.toHaveBeenCalledWith(expect.anything(), expect.anything(), 'login-1'); diff --git a/lib/filters/__tests__/quick-rule-target.test.ts b/lib/filters/__tests__/quick-rule-target.test.ts index 45ecdb863..02d271ccc 100644 --- a/lib/filters/__tests__/quick-rule-target.test.ts +++ b/lib/filters/__tests__/quick-rule-target.test.ts @@ -25,7 +25,10 @@ function email(mailboxIds: Record, stamps: Partial = {}) describe('resolveQuickRuleTarget', () => { beforeEach(() => { - useAuthStore.setState({ client, activeAccountId: 'login-a' } as never); + useAuthStore.setState({ + client, activeAccountId: 'login-a', + getClientForAccount: (id: string) => (id === 'login-a' ? client : undefined), + } as never); useEmailStore.setState({ mailboxes, selectedMailbox: 'a', viewingAccountId: null, accountMailboxes: {} } as never); }); @@ -44,6 +47,10 @@ describe('resolveQuickRuleTarget', () => { expect(resolveQuickRuleTarget(email({ 'c:a': true }, { sourceAccountId: 'c', sourceClientAccountId: 'login-a' }))?.shared).toBe(true); }); + it('offers no target for a message of a login that is gone', () => { + expect(resolveQuickRuleTarget(email({ a: true }, { sourceAccountId: 'd', sourceClientAccountId: 'login-b' }))).toBeNull(); + }); + it('finds the folder the message is in', () => { const target = resolveQuickRuleTarget(email({ h: true }))!; expect(sourceMailboxOf(email({ h: true }), target)?.id).toBe('h'); diff --git a/lib/filters/quick-rule-target.ts b/lib/filters/quick-rule-target.ts index 1c6f0962a..94660d598 100644 --- a/lib/filters/quick-rule-target.ts +++ b/lib/filters/quick-rule-target.ts @@ -46,7 +46,12 @@ function clientKey(client: IJMAPClient): number { export function resolveQuickRuleTarget(email: Email): QuickRuleTarget | null { const auth = useAuthStore.getState(); if (!auth.client) return null; - const context = resolveEmailActionContext(email, auth.client); + let context: ReturnType; + try { + context = resolveEmailActionContext(email, auth.client); + } catch { + return null; // a message of a login that is gone + } const client = context.client; const ownAccountId = client.getAccountId(); const accountId = context.accountId ?? ownAccountId; diff --git a/stores/__tests__/email-store-gone-login.test.ts b/stores/__tests__/email-store-gone-login.test.ts new file mode 100644 index 000000000..c165b56e7 --- /dev/null +++ b/stores/__tests__/email-store-gone-login.test.ts @@ -0,0 +1,95 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { useEmailStore, resolveEmailActionContext } from '../email-store'; +import { useAuthStore } from '../auth-store'; +import type { Email, Mailbox } from '@/lib/jmap/types'; +import type { IJMAPClient } from '@/lib/jmap/client-interface'; + +// A row stamped with a login that is no longer connected must not be written +// through another login: email and account ids repeat across logins and +// servers, so the active login's client would act on its own message with +// the same id (GHSA-8x23 class). + +function mailbox(id: string, role: string, accountId: string): Mailbox { + return { id, originalId: id, name: id, role, accountId, isShared: false, sortOrder: 0, totalEmails: 0, unreadEmails: 0, totalThreads: 0, unreadThreads: 0, isSubscribed: true } as Mailbox; +} + +function makeClient(accountId: string) { + return { + getAccountId: () => accountId, + markAsRead: vi.fn().mockResolvedValue(undefined), + toggleStar: vi.fn().mockResolvedValue(undefined), + moveEmail: vi.fn().mockResolvedValue(undefined), + moveToTrash: vi.fn().mockResolvedValue(undefined), + setKeyword: vi.fn().mockResolvedValue(undefined), + batchUpdateKeywords: vi.fn().mockResolvedValue(undefined), + batchMarkAsRead: vi.fn().mockResolvedValue(undefined), + } as unknown as IJMAPClient; +} + +// A message of login B (gone), with the same id and account id as one of the +// active login A's own messages. +const goneRow = { + id: 'eaaaaab', threadId: 't', subject: 'B', receivedAt: '2026-10-26T10:00:00Z', + keywords: {}, mailboxIds: { a: true }, + sourceClientAccountId: 'login-b', sourceAccountId: 'cf', +} as unknown as Email; + +describe('a row of a login that is no longer connected', () => { + let active: IJMAPClient; + + beforeEach(() => { + active = makeClient('cf'); + useAuthStore.setState({ + activeAccountId: 'login-a', + client: active as never, + getClientForAccount: (id: string) => (id === 'login-a' ? active : undefined) as never, + } as never); + useEmailStore.setState({ + isUnifiedView: true, viewingAccountId: null, selectedMailbox: '', + mailboxes: [mailbox('a', 'inbox', 'cf'), mailbox('b', 'trash', 'cf')], + accountMailboxes: {}, + emails: [goneRow], selectedEmail: null, selectedEmailKeys: new Set(), + processingReadStatus: new Set(), + threadEmailsCache: new Map(), expandedThreadIds: new Set(), + } as never); + }); + + it('resolves to no client instead of the active one', () => { + expect(() => resolveEmailActionContext(goneRow, active)).toThrow('Not connected'); + }); + + it('is not marked read through the active login', async () => { + await expect(useEmailStore.getState().markAsRead(active, 'eaaaaab', true)).rejects.toThrow('Not connected'); + expect(active.markAsRead).not.toHaveBeenCalled(); + }); + + it('is not starred through the active login', async () => { + await expect(useEmailStore.getState().toggleStar(active, 'eaaaaab')).rejects.toThrow('Not connected'); + expect(active.toggleStar).not.toHaveBeenCalled(); + }); + + it('is not moved through the active login', async () => { + await expect(useEmailStore.getState().moveToMailbox(active, 'eaaaaab', 'b')).rejects.toThrow('Not connected'); + expect(active.moveEmail).not.toHaveBeenCalled(); + expect(useEmailStore.getState().emails).toEqual([goneRow]); + }); + + it('gets no keyword through the active login', async () => { + await expect(useEmailStore.getState().markEmailKeyword(active, 'eaaaaab', '$answered')).rejects.toThrow('Not connected'); + expect(active.setKeyword).not.toHaveBeenCalled(); + }); + + it('is not moved back to the Inbox through the active login', async () => { + (active as unknown as { undoSpam: unknown }).undoSpam = vi.fn().mockResolvedValue(undefined); + await expect(useEmailStore.getState().undoSpam(active, 'eaaaaab')).rejects.toThrow('Not connected'); + expect((active as unknown as { undoSpam: ReturnType }).undoSpam).not.toHaveBeenCalled(); + }); + + it('is not marked read with its expanded thread through the active login', async () => { + useEmailStore.setState({ emails: [{ ...goneRow, threadId: 't' }] } as never); + vi.spyOn(console, 'error').mockImplementation(() => {}); + await useEmailStore.getState().markThreadAsRead(active, 'login-b/cf:t'); + expect(active.batchMarkAsRead).not.toHaveBeenCalled(); + expect(useEmailStore.getState().emails[0].keywords.$seen).toBeUndefined(); + }); +}); diff --git a/stores/email-store.ts b/stores/email-store.ts index c1d1bdcfa..8786f56b7 100644 --- a/stores/email-store.ts +++ b/stores/email-store.ts @@ -1010,6 +1010,16 @@ function resolveActionClient(passedClient: IJMAPClient): IJMAPClient { return c ?? passedClient; } +/** + * The client of the login a row is stamped with. Never another login's once + * that one is gone: its ids name other messages there. + */ +function stampedClient(clientAccountId: string): IJMAPClient { + const client = useAuthStore.getState().getClientForAccount(clientAccountId); + if (!client) throw new Error('Not connected'); + return client; +} + /** * Thrown by archive actions when no archive folder exists in the target * account. Callers map it to the translated @@ -1434,10 +1444,10 @@ export function resolveEmailActionContext( // The stamps are honored whenever present, not only while `isUnifiedView` // is set: a stamped email (e.g. the open selection) can outlive the flag, and // bare ids collide across accounts, so the active client is never a safe - // substitute for the stamped one. (#847) + // substitute for the stamped one, also not once that login is gone. (#847) if (email.sourceClientAccountId && email.sourceAccountId) { return { - client: useAuthStore.getState().getClientForAccount(email.sourceClientAccountId) ?? resolveActionClient(passedClient), + client: stampedClient(email.sourceClientAccountId), mailboxes: sourceMailboxes(state, email.sourceClientAccountId, email.sourceAccountId) ?? state.mailboxes, accountId: email.sourceAccountId, }; @@ -5883,7 +5893,7 @@ export const useEmailStore = create((set, get) => ({ targetMailboxId = cachedData.originalMailboxId; accountId = cachedData.accountId; if (cachedData.sourceClientAccountId) { - undoClient = useAuthStore.getState().getClientForAccount(cachedData.sourceClientAccountId) ?? undoClient; + undoClient = stampedClient(cachedData.sourceClientAccountId); } get().spamUndoCache.delete(emailKey ?? emailId); } else { @@ -5894,7 +5904,7 @@ export const useEmailStore = create((set, get) => ({ const listEmail = get().emails.find(isTarget); let inboxMailboxes = mailboxes; if (listEmail?.sourceClientAccountId && listEmail?.sourceAccountId) { - undoClient = useAuthStore.getState().getClientForAccount(listEmail.sourceClientAccountId) ?? undoClient; + undoClient = stampedClient(listEmail.sourceClientAccountId); accountId = listEmail.sourceAccountId; inboxMailboxes = sourceMailboxes(get(), listEmail.sourceClientAccountId, listEmail.sourceAccountId) ?? mailboxes; } else { @@ -6666,11 +6676,17 @@ export const useEmailStore = create((set, get) => ({ // or marks an unrelated message that happens to share the id. So a thread // in a shared or group folder has to name its account. const groups = new Map>(); - for (const id of unreadIds) { - const { client: actionClient, accountId } = resolveEmailActionContext(allEmailMap.get(id)!, client); - const byAccount = groups.get(actionClient) ?? new Map(); - byAccount.set(accountId, [...(byAccount.get(accountId) ?? []), id]); - groups.set(actionClient, byAccount); + try { + for (const id of unreadIds) { + const { client: actionClient, accountId } = resolveEmailActionContext(allEmailMap.get(id)!, client); + const byAccount = groups.get(actionClient) ?? new Map(); + byAccount.set(accountId, [...(byAccount.get(accountId) ?? []), id]); + groups.set(actionClient, byAccount); + } + } catch (error) { + // A message of a login that is gone: the thread stays unread. + console.error('Failed to mark thread as read:', error); + return; } // Mark all as read on the server. A message the server refused (a