From e120ac63617a3beedca95d081d2fa2950ef4af9f Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Mon, 26 Oct 2026 14:22:27 +0100 Subject: [PATCH] fix: let plugins read the message of the account they name, as ids repeat across accounts --- lib/__tests__/plugin-host-mail-bytes.test.ts | 24 +++++- lib/__tests__/plugin-host-mail-read.test.ts | 37 +++++++- lib/__tests__/plugin-projection.test.ts | 7 ++ lib/plugin-projection.ts | 10 ++- lib/plugin-sandbox/host-api-mail.ts | 90 +++++++++++++------- lib/plugin-sandbox/host-api.ts | 6 +- lib/plugin-sandbox/runtime.tsx | 14 +-- lib/plugin-types.ts | 10 +++ 8 files changed, 156 insertions(+), 42 deletions(-) diff --git a/lib/__tests__/plugin-host-mail-bytes.test.ts b/lib/__tests__/plugin-host-mail-bytes.test.ts index dee905e22..c1a314007 100644 --- a/lib/__tests__/plugin-host-mail-bytes.test.ts +++ b/lib/__tests__/plugin-host-mail-bytes.test.ts @@ -40,9 +40,10 @@ beforeEach(() => { if (blobId === 'blob-raw') return new TextEncoder().encode('From').buffer; return new Uint8Array([1, 2, 3]).buffer; }), + getAccountId: vi.fn(() => 'acc1'), }; useAuthStore.setState({ client: client as unknown as IJMAPClient }); - useEmailStore.setState({ selectedEmail: null }); + useEmailStore.setState({ selectedEmail: null, emails: [], selectedMailbox: '' }); }); afterEach(() => { @@ -96,6 +97,27 @@ describe('api.email.getRaw', () => { expect(client.fetchBlobArrayBuffer).not.toHaveBeenCalled(); }); + it('reads the source of the account a plugin names', async () => { + // A group folder is open and its message has the same id as an own one. + const group = { ...message, blobId: 'blob-group' } as Email; + client.getEmail.mockImplementation(async (id: string, accountId?: string) => ( + id === 'm1' ? (accountId === 'grp' ? group : message) : null)); + useEmailStore.setState({ + mailboxes: [{ id: 'grp:inbox', originalId: 'inbox', name: 'Inbox', isShared: true, accountId: 'grp' } as never], + selectedMailbox: 'grp:inbox', + emails: [{ ...group, mailboxIds: { 'grp:inbox': true } } as Email], + selectedEmail: { ...group, mailboxIds: { 'grp:inbox': true } } as Email, + }); + + await call(ARCHIVER, 'email.getRaw', 'm1', 'grp'); + expect(client.getEmail).toHaveBeenLastCalledWith('m1', 'grp'); + expect(client.fetchBlobArrayBuffer).toHaveBeenLastCalledWith('blob-group', 'message.eml', 'message/rfc822', 'grp'); + + await call(ARCHIVER, 'email.getRaw', 'm1', 'acc1'); + expect(client.getEmail).toHaveBeenLastCalledWith('m1'); + expect(client.fetchBlobArrayBuffer).toHaveBeenLastCalledWith('blob-raw', 'message.eml', 'message/rfc822'); + }); + it('leaves arbitrary blob reads to the privileged tier', async () => { await expect(call(ARCHIVER, 'jmap.fetchBlob', 'blob-raw')).rejects.toThrow(/requires the privileged plugin tier/); }); diff --git a/lib/__tests__/plugin-host-mail-read.test.ts b/lib/__tests__/plugin-host-mail-read.test.ts index 8e9723359..f85d71457 100644 --- a/lib/__tests__/plugin-host-mail-read.test.ts +++ b/lib/__tests__/plugin-host-mail-read.test.ts @@ -48,13 +48,14 @@ beforeEach(() => { : null)), getEmails: vi.fn(async () => ({ emails: [mail('a'), mail('b')], hasMore: false, total: 2 })), getMailboxes: vi.fn(async () => [inbox]), + getAccountId: vi.fn(() => 'acc1'), }; useAuthStore.setState({ client: client as unknown as IJMAPClient }); useEmailStore.setState({ mailboxes: [inbox, shared], selectedEmail: null }); }); afterEach(() => { - useAuthStore.setState({ client: null, getClientForAccount: realGetClientForAccount }); + useAuthStore.setState({ client: null, activeAccountId: null, getClientForAccount: realGetClientForAccount }); useEmailStore.setState({ mailboxes: [], selectedEmail: null, selectedMailbox: '', emails: [] }); }); @@ -89,6 +90,33 @@ describe('api.email.get', () => { expect(client.getEmail).not.toHaveBeenCalled(); }); + // Ids repeat across accounts: a unified view lists the own message and a + // group's under the same id, and a bare id named whichever came first. + it('reads the message of the account a plugin names, and says which account it is', async () => { + client.getEmail.mockImplementation(async (id: string, accountId?: string) => ( + id === 'm1' ? mail('m1', { subject: accountId === 'grp' ? 'Group' : 'Own' }) : null)); + useAuthStore.setState({ + activeAccountId: 'login1', + getClientForAccount: ((id: string) => (id === 'login1' ? client : undefined)) as never, + }); + useEmailStore.setState({ + emails: [ + mail('m1', { sourceClientAccountId: 'login1', sourceAccountId: 'grp' }), + mail('m1', { sourceClientAccountId: 'login1', sourceAccountId: 'acc1' }), + ], + }); + + expect(await call(READER, 'email.get', 'm1', 'acc1')).toEqual(expect.objectContaining({ subject: 'Own', accountId: 'acc1' })); + expect(await call(READER, 'email.get', 'm1', 'grp')).toEqual(expect.objectContaining({ subject: 'Group', accountId: 'grp' })); + // Without one, as before: the first listed row. + expect(await call(READER, 'email.get', 'm1')).toEqual(expect.objectContaining({ subject: 'Group', accountId: 'grp' })); + // An account this login doesn't show is not asked. + client.getEmail.mockClear(); + expect(await call(READER, 'email.get', 'm1', 'elsewhere')).toBeNull(); + expect(client.getEmail).not.toHaveBeenCalled(); + await expect(call(READER, 'email.get', 'm1', 7)).rejects.toThrow(/accountId must be a string/); + }); + it('uses the open message when its body is loaded', async () => { useEmailStore.setState({ selectedEmail: mail('open', { bodyValues: { 1: { value: 'x' } }, textBody: [{ partId: '1' }] } as unknown as Partial) }); expect(await call(READER, 'email.get', 'open')).toEqual(expect.objectContaining({ id: 'open', text: 'x' })); @@ -139,6 +167,13 @@ describe('api.email.list', () => { expect(client.getEmails).toHaveBeenCalledWith('box', 'acc2', 50, 0); }); + it('names the account of each message', async () => { + const own = await call(READER, 'email.list', { mailboxId: 'inbox' }) as { accountId?: string }[]; + const team = await call(READER, 'email.list', { mailboxId: 'acc2:box' }) as { accountId?: string }[]; + expect(own.map((e) => e.accountId)).toEqual(['acc1', 'acc1']); + expect(team.map((e) => e.accountId)).toEqual(['acc2', 'acc2']); + }); + it('caps the page at 100 and refuses unknown folders', async () => { await expect(call(READER, 'email.list', { limit: 101 })).rejects.toThrow(/limit must be a whole number from 1 to 100/); await expect(call(READER, 'email.list', { mailboxId: 'nope' })).rejects.toThrow(/no mailbox with id "nope"/); diff --git a/lib/__tests__/plugin-projection.test.ts b/lib/__tests__/plugin-projection.test.ts index 29f6f7a2f..750025aa4 100644 --- a/lib/__tests__/plugin-projection.test.ts +++ b/lib/__tests__/plugin-projection.test.ts @@ -31,6 +31,13 @@ describe('plugin projections', () => { expect(JSON.stringify(summary)).not.toContain('secret body'); }); + it("names the account of a merged view's row, which shares its id with others", () => { + const row = { id: 'm1', threadId: 't', mailboxIds: {}, keywords: {}, sourceClientAccountId: 'login1', sourceAccountId: 'grp' } as unknown as Email; + expect(emailSummary(row).accountId).toBe('grp'); + expect(emailToReadView(row).accountId).toBe('grp'); + expect(emailSummary({ ...row, sourceAccountId: undefined })).not.toHaveProperty('accountId'); + }); + it('marks recurring events and completed tasks', () => { expect(eventToView({ id: 'e', uid: 'u', calendarIds: { c: true }, recurrenceRules: [{ frequency: 'yearly' }] } as unknown as CalendarEvent).isRecurring).toBe(true); expect(taskToView({ id: 't', uid: 'u', calendarIds: {}, progress: 'completed' } as unknown as CalendarTask).isComplete).toBe(true); diff --git a/lib/plugin-projection.ts b/lib/plugin-projection.ts index d02e89e3e..3aa5a58c6 100644 --- a/lib/plugin-projection.ts +++ b/lib/plugin-projection.ts @@ -47,9 +47,14 @@ function plainTextFromEmail(email: Email): string { return (email.preview || '').trim(); } -export function emailToReadView(email: Email): EmailReadView { +/** + * `accountId` names the JMAP account the message is in; rows of merged views + * carry it, others only the caller knows (see EmailReadView.accountId). + */ +export function emailToReadView(email: Email, accountId = email.sourceAccountId): EmailReadView { return { id: email.id, + ...(accountId ? { accountId } : {}), threadId: email.threadId, mailboxIds: Object.keys(email.mailboxIds || {}).filter(k => email.mailboxIds[k]), from: (email.from || []).map(a => ({ name: a.name || '', email: a.email })), @@ -83,9 +88,10 @@ const trueKeys = (rec?: Record | null): string[] => Object.keys(rec || {}).filter((k) => rec?.[k]); /** Headers and flags of a message, without the body (see EmailSummary). */ -export function emailSummary(email: Email): EmailSummary { +export function emailSummary(email: Email, accountId = email.sourceAccountId): EmailSummary { return { id: email.id, + ...(accountId ? { accountId } : {}), threadId: email.threadId, mailboxIds: trueKeys(email.mailboxIds), from: (email.from || []).map((a) => ({ name: a.name || '', email: a.email })), diff --git a/lib/plugin-sandbox/host-api-mail.ts b/lib/plugin-sandbox/host-api-mail.ts index 33a757ae1..5b8466409 100644 --- a/lib/plugin-sandbox/host-api-mail.ts +++ b/lib/plugin-sandbox/host-api-mail.ts @@ -3,10 +3,10 @@ // (api.email.getAttachment / getRaw). Reads go to the server through the // JMAP client and never change what the user has open. // -// api.email.get also reads a message of a shared account when it is in the -// loaded list, from the account that owns it, always through the signed-in -// login. The bytes of attachments and sources are read from the signed-in -// account only. +// Message ids repeat across accounts. A plugin may name the account +// (EmailSummary.accountId); without one, api.email.get reads a message listed +// in the open view from the account it belongs to, the bytes come from the +// open message or the signed-in account. Always through the signed-in login. import type { EmailReadView, EmailSummary, MailboxView } from '../plugin-types'; import type { Email, Mailbox } from '../jmap/types'; @@ -19,32 +19,61 @@ import { MAX_PLUGIN_FETCH_BYTES, readBoolean, readId, readInteger, readObject, r /** Most messages one api.email.list call returns. */ export const MAX_PLUGIN_EMAIL_PAGE = 100; -/** The open message when it is this one and its body is loaded, else the server's copy. */ -async function loadEmail(client: IJMAPClient, emailId: string): Promise { +/** + * The message from `accountId` (a shared or group account), else the + * signed-in account's: the open message when it is this one and its body is + * loaded, else the server's copy. `named`: the plugin named the signed-in + * account, so the open message counts only when it is that account's. + */ +async function loadEmail(client: IJMAPClient, emailId: string, accountId?: string, named = false): Promise { + if (accountId) return client.getEmail(emailId, accountId); const { selectedEmail } = useEmailStore.getState(); if (selectedEmail?.id === emailId && selectedEmail.bodyValues && !selectedEmail.accountId) { - return selectedEmail; + // A message of an open shared folder carries no stamp. + const open = named ? resolveEmailBlobContext(selectedEmail, client) : null; + if (!open || (open.client === client && !open.accountId)) return selectedEmail; } return client.getEmail(emailId); } -export async function doEmailGet(id: unknown): Promise { +/** + * The account to read `emailId` from: undefined for the signed-in account, + * null when it is not this login's to read. + */ +async function sourceAccount(client: IJMAPClient, emailId: string, method: string, accountArg: unknown): Promise { + const { emails } = useEmailStore.getState(); + if (accountArg === undefined) { + // A listed message of a shared account is asked of the account that owns + // it; the signed-in account doesn't know its id. + const listed = emails.find((e) => e.id === emailId); + const owner = listed ? resolveEmailBlobContext(listed, client) : null; + // Only through the signed-in login. A row of another connected login is + // not read here, nor asked of this login under an id that isn't its own. + // The rows can come from a plugin (onEmailsFetched), so their stamps must + // not pick which login is asked. + if (owner && owner.client !== client) return null; + return owner?.accountId; + } + const accountId = readId(accountArg, method, 'accountId'); + if (accountId === client.getAccountId()) return undefined; + // An account this login shows: one of its folders, or a listed row of it. + if ((await knownMailboxes(client)).some((m) => m.isShared && m.accountId === accountId)) return accountId; + const listed = emails.some((e) => { + if (e.id !== emailId) return false; + const owner = resolveEmailBlobContext(e, client); + return owner.client === client && owner.accountId === accountId; + }); + return listed ? accountId : null; +} + +export async function doEmailGet(id: unknown, accountArg?: unknown): Promise { const method = 'email.get'; const emailId = readId(id, method, 'emailId'); const client = sessionClient(method); - // A listed message of a shared account is asked of the account that owns - // it; the signed-in account doesn't know its id. - const listed = useEmailStore.getState().emails.find((e) => e.id === emailId); - const owner = listed ? resolveEmailBlobContext(listed, client) : null; - // Only through the signed-in login. A row of another connected login is - // not read here, nor asked of this login under an id that isn't its own. - // The rows can come from a plugin (onEmailsFetched), so their stamps must - // not pick which login is asked. - if (owner && owner.client !== client) return null; - const email = owner?.accountId - ? await client.getEmail(emailId, owner.accountId) - : await loadEmail(client, emailId); - return email ? emailToReadView(email) : null; + const accountId = await sourceAccount(client, emailId, method, accountArg); + if (accountId === null) return null; + const email = await loadEmail(client, emailId, accountId, accountArg !== undefined); + return email ? emailToReadView(email, accountId ?? client.getAccountId()) : null; } function mailboxToView(mailbox: Mailbox): MailboxView { @@ -88,7 +117,7 @@ export async function doEmailList(options: unknown): Promise { accountId = mailbox.isShared ? mailbox.accountId : undefined; } const { emails } = await client.getEmails(jmapMailboxId, accountId, limit, position); - return emails.map(emailSummary); + return emails.map((email) => emailSummary(email, accountId ?? client.getAccountId())); } /** Most messages one search returns. */ @@ -132,7 +161,7 @@ export async function doEmailSearch(options: unknown): Promise { accountId = mailbox.isShared ? mailbox.accountId : undefined; } const { emails } = await client.getEmails(jmapMailboxId, accountId, limit, 0, undefined, false, filter); - return emails.map(emailSummary); + return emails.map((email) => emailSummary(email, accountId ?? client.getAccountId())); } // ─── Bytes ──────────────────────────────────────────────────── @@ -158,17 +187,19 @@ export interface PluginAttachmentData { * attachment of that message: a plugin can't use this to read arbitrary * blobs (that stays jmap.fetchBlob, privileged tier only). */ -export async function doEmailGetAttachment(emailIdArg: unknown, blobIdArg: unknown): Promise { +export async function doEmailGetAttachment(emailIdArg: unknown, blobIdArg: unknown, accountArg?: unknown): Promise { const method = 'email.getAttachment'; const emailId = readId(emailIdArg, method, 'emailId'); const blobId = readId(blobIdArg, method, 'blobId'); const client = sessionClient(method); - const email = await loadEmail(client, emailId); + // Without an account, as before: the open message or the signed-in account's. + const accountId = accountArg === undefined ? undefined : await sourceAccount(client, emailId, method, accountArg); + const email = accountId === null ? null : await loadEmail(client, emailId, accountId, accountArg !== undefined); if (!email) throw new Error(`${method}: no message with id "${emailId}"`); const part = attachmentParts(email).find((a) => a.blobId === blobId); if (!part) throw new Error(`${method}: that blob is not an attachment of this message`); if (part.size > MAX_PLUGIN_FETCH_BYTES) throw tooBig(method); - const data = await client.fetchBlobArrayBuffer(blobId, part.name || undefined, part.type || undefined); + const data = await client.fetchBlobArrayBuffer(blobId, part.name || undefined, part.type || undefined, ...(accountId ? [accountId] : [])); if (data.byteLength > MAX_PLUGIN_FETCH_BYTES) throw tooBig(method); return { name: part.name || 'attachment', @@ -179,15 +210,16 @@ export async function doEmailGetAttachment(emailIdArg: unknown, blobIdArg: unkno } /** The message as the server stores it (RFC 5322), e.g. to save it as a .eml file. */ -export async function doEmailGetRaw(emailIdArg: unknown): Promise { +export async function doEmailGetRaw(emailIdArg: unknown, accountArg?: unknown): Promise { const method = 'email.getRaw'; const emailId = readId(emailIdArg, method, 'emailId'); const client = sessionClient(method); - const email = await loadEmail(client, emailId); + const accountId = accountArg === undefined ? undefined : await sourceAccount(client, emailId, method, accountArg); + const email = accountId === null ? null : await loadEmail(client, emailId, accountId, accountArg !== undefined); if (!email) throw new Error(`${method}: no message with id "${emailId}"`); if (!email.blobId) throw new Error(`${method}: the server did not say where the message source is`); if (email.size > MAX_PLUGIN_FETCH_BYTES) throw tooBig(method); - const data = await client.fetchBlobArrayBuffer(email.blobId, 'message.eml', 'message/rfc822'); + const data = await client.fetchBlobArrayBuffer(email.blobId, 'message.eml', 'message/rfc822', ...(accountId ? [accountId] : [])); if (data.byteLength > MAX_PLUGIN_FETCH_BYTES) throw tooBig(method); return data; } diff --git a/lib/plugin-sandbox/host-api.ts b/lib/plugin-sandbox/host-api.ts index 960ae1f18..0790e8a09 100644 --- a/lib/plugin-sandbox/host-api.ts +++ b/lib/plugin-sandbox/host-api.ts @@ -2172,12 +2172,12 @@ export async function dispatchApiCall( case 'vacation.get': return doVacationGet(); case 'vacation.set': return doVacationSet(args[0]); - case 'email.get': return doEmailGet(args[0]); + case 'email.get': return doEmailGet(args[0], args[1]); case 'email.list': return doEmailList(args[0]); case 'email.search': return doEmailSearch(args[0]); case 'mailboxes.list': return doMailboxesList(); - case 'email.getAttachment': return doEmailGetAttachment(args[0], args[1]); - case 'email.getRaw': return doEmailGetRaw(args[0]); + case 'email.getAttachment': return doEmailGetAttachment(args[0], args[1], args[2]); + case 'email.getRaw': return doEmailGetRaw(args[0], args[1]); case 'email.move': return doEmailMove(plugin.id, args[0], args[1]); case 'email.archive': return doEmailArchive(plugin.id, args[0]); diff --git a/lib/plugin-sandbox/runtime.tsx b/lib/plugin-sandbox/runtime.tsx index 336996c39..3f8f46405 100644 --- a/lib/plugin-sandbox/runtime.tsx +++ b/lib/plugin-sandbox/runtime.tsx @@ -643,9 +643,11 @@ function buildPluginApi(manifest: PluginManifest) { /** * One message with its plain text, headers and source, or null when * there is none with that id. Any message of the account, not only the - * open one. + * open one. Ids repeat across accounts: pass the view's `accountId` to + * name the message of that account. */ - get: (emailId: string) => callApi('email.get', [emailId]) as Promise, + get: (emailId: string, accountId?: string) => + callApi('email.get', [emailId, accountId]) as Promise, /** * Move messages (at most 100) into a folder of the account, like the * user would; the move hooks run. 20 moves a minute. Needs email:write. @@ -673,13 +675,13 @@ function buildPluginApi(manifest: PluginManifest) { * 25 MB). `blobId` must be one of that message's attachments, e.g. * from an `attachment-actions` slot or onAttachmentDownload. */ - getAttachment: (emailId: string, blobId: string) => - callApi('email.getAttachment', [emailId, blobId], NETWORK_API_TIMEOUT_MS) as Promise<{ + getAttachment: (emailId: string, blobId: string, accountId?: string) => + callApi('email.getAttachment', [emailId, blobId, accountId], NETWORK_API_TIMEOUT_MS) as Promise<{ name: string; type: string; size: number; data: ArrayBuffer; }>, /** The full RFC 5322 source of a message, to save as .eml (email:blob-read, at most 25 MB). */ - getRaw: (emailId: string) => - callApi('email.getRaw', [emailId], NETWORK_API_TIMEOUT_MS) as Promise, + getRaw: (emailId: string, accountId?: string) => + callApi('email.getRaw', [emailId, accountId], NETWORK_API_TIMEOUT_MS) as Promise, setKeyword: (emailId: string, keyword: string, accountId?: string) => callApi('email.setKeyword', [emailId, keyword, accountId]) as Promise, removeKeyword: (emailId: string, keyword: string, accountId?: string) => diff --git a/lib/plugin-types.ts b/lib/plugin-types.ts index 4df1b48c9..a5829fa67 100644 --- a/lib/plugin-types.ts +++ b/lib/plugin-types.ts @@ -397,6 +397,14 @@ export interface KeyboardShortcut { export interface EmailReadView { id: string; + /** + * The JMAP account the message is in. Message ids repeat across accounts + * (own, shared, group), so pass it to email.get / getRaw / getAttachment / + * setKeyword to name this very message. Always set on what email.get, + * list and search return; elsewhere set at least when messages of several + * accounts are listed together. + */ + accountId?: string; threadId: string; mailboxIds: string[]; from: { name: string; email: string }[]; @@ -462,6 +470,8 @@ export interface EmailSummary { keywords: string[]; hasAttachment: boolean; size: number; + /** See EmailReadView.accountId. */ + accountId?: string; } export interface DraftView {