From 6cbd53c8272d0197c11b6e6e77fbac4e62e78071 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sun, 1 Nov 2026 08:06:49 +0100 Subject: [PATCH] fix: run plugin send checks for quick replies --- components/mail/mail-app.tsx | 22 ++++++----- lib/outgoing/__tests__/quick-reply.test.ts | 44 +++++++++++++++++++++- lib/outgoing/plugin-send-check.ts | 23 +++++++++++ lib/outgoing/quick-reply.ts | 33 ++++++++++++++-- lib/plugin-types.ts | 7 ++++ 5 files changed, 116 insertions(+), 13 deletions(-) create mode 100644 lib/outgoing/plugin-send-check.ts diff --git a/components/mail/mail-app.tsx b/components/mail/mail-app.tsx index 310ef7dcb..e04c7186d 100644 --- a/components/mail/mail-app.tsx +++ b/components/mail/mail-app.tsx @@ -4099,6 +4099,12 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { }); } }; + // RFC 5322 §3.6.4 threading - keep the conversation stitched together (#234). + const threading = computeReplyThreadingHeaders({ + messageId: selectedEmail.messageId, + references: selectedEmail.references, + }); + const replySubject = buildReplySubject(selectedEmail.subject || t('email_viewer.no_subject'), t('email_composer.prefix.reply')); const prepared = await prepareQuickReply({ body, signatureIdentity, @@ -4107,6 +4113,12 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { to: recipients, fromEmail: headerFromEmail, identityId: sendingIdentity?.id, + subject: replySubject, + inReplyTo: threading?.inReplyTo?.[0], + // A plugin that said no told the user; one that crashed didn't. + onSendCheckStopped: (outcome) => { + if (outcome === 'failed') toast.error(t('email_composer.send_guard_failed')); + }, questions: { // The fill-in words of the UI language and of English, as the composer. placeholderWords: settings.placeholderWarningEnabled @@ -4137,16 +4149,8 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { } } - // RFC 5322 §3.6.4 threading - keep the conversation stitched together (#234). - const threading = computeReplyThreadingHeaders({ - messageId: selectedEmail.messageId, - references: selectedEmail.references, - }); - // The label goes on last, as in the composer. - const labelled = classifyOutgoing(quickLabel.policy, quickLabel.label, { - subject: buildReplySubject(selectedEmail.subject || t('email_viewer.no_subject'), t('email_composer.prefix.reply')), - }); + const labelled = classifyOutgoing(quickLabel.policy, quickLabel.label, { subject: replySubject }); // Send reply with just the body text const result = await sendEmail( diff --git a/lib/outgoing/__tests__/quick-reply.test.ts b/lib/outgoing/__tests__/quick-reply.test.ts index 8ee19a28e..723e96556 100644 --- a/lib/outgoing/__tests__/quick-reply.test.ts +++ b/lib/outgoing/__tests__/quick-reply.test.ts @@ -2,6 +2,8 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; import { prepareQuickReply, type QuickReplyQuestion, type QuickReplyQuestions } from '@/lib/outgoing/quick-reply'; import { registerOutgoingFinalizer, registerPreSendCheck, resetSendPipeline } from '@/lib/outgoing/send-pipeline'; import type { Identity } from '@/lib/jmap/types'; +import { emailHooks, pluginErrorTracker } from '@/lib/plugin-hooks'; +import type { OutgoingEmail } from '@/lib/plugin-types'; vi.mock('@/lib/debug', () => ({ debug: { log: () => {}, warn: () => {}, error: () => {} } })); @@ -14,7 +16,11 @@ const input = (signatureIdentity: Identity) => ({ body: 'Thanks <3', signatureIdentity, separator: true, to: ['bob@example.com'], fromEmail: 'me@example.com', identityId: 'i', }); -afterEach(() => resetSendPipeline()); +afterEach(() => { + resetSendPipeline(); + emailHooks.onBeforeEmailSend.clear(); + pluginErrorTracker.resetAll(); +}); describe('prepareQuickReply', () => { it('builds the text and, for an HTML signature, an escaped HTML body', async () => { @@ -107,3 +113,39 @@ describe('prepareQuickReply warnings', () => { expect(asked).toEqual([]); }); }); + +// Plugins' send checks (onBeforeEmailSend) ran for the composer and a +// plugin's forward, never for a quick reply: a way around a send guard. +describe('prepareQuickReply and plugin send checks', () => { + it('asks the plugins with the reply and stops when one says no', async () => { + const seen: OutgoingEmail[] = []; + emailHooks.onBeforeEmailSend.register('guard', (email: OutgoingEmail) => { seen.push(email); return false; }); + const stopped: string[] = []; + const out = await prepareQuickReply({ + ...input(text), body: 'IBAN DE89370400440532013000', subject: 'Re: Invoice', inReplyTo: '', + onSendCheckStopped: (outcome) => stopped.push(outcome), + }); + expect(out).toBeNull(); + expect(stopped).toEqual(['vetoed']); + expect(seen).toEqual([expect.objectContaining({ + surface: 'quick-reply', mode: 'reply', to: ['bob@example.com'], subject: 'Re: Invoice', + inReplyTo: '', identityId: 'i', fromEmail: 'me@example.com', + })]); + expect(seen[0].textBody).toContain('IBAN DE89370400440532013000'); + }); + + it('fails closed when a check crashes', async () => { + emailHooks.onBeforeEmailSend.register('guard', () => { throw new Error('boom'); }); + const stopped: string[] = []; + expect(await prepareQuickReply({ ...input(text), onSendCheckStopped: (o) => stopped.push(o) })).toBeNull(); + expect(stopped).toEqual(['failed']); + }); + + it('sends when the plugins allow it, with the disclaimer stage after them', async () => { + const order: string[] = []; + emailHooks.onBeforeEmailSend.register('guard', () => { order.push('plugin'); return undefined; }); + registerOutgoingFinalizer('disclaimer', 10, (bodies) => { order.push('last'); return bodies; }, 'last'); + expect(await prepareQuickReply(input(text))).not.toBeNull(); + expect(order).toEqual(['plugin', 'last']); + }); +}); diff --git a/lib/outgoing/plugin-send-check.ts b/lib/outgoing/plugin-send-check.ts new file mode 100644 index 000000000..919657290 --- /dev/null +++ b/lib/outgoing/plugin-send-check.ts @@ -0,0 +1,23 @@ +/** + * Plugins' say before a user's mail goes out (onBeforeEmailSend), the same + * for every surface that sends it: the composer (inline, pop-out, Pro tab), + * quick reply and a plugin's forward (api.email.forward). The bus fails + * closed: a handler that throws or runs out of time stops the send. + * + * Not run for mail Bulwark sends on its own (calendar invitations and RSVP + * replies, read receipts, List-Unsubscribe mails, phishing reports) nor for + * a crypto plugin's raw send, which happens after the composer's check. + */ + +import { emailHooks } from '@/lib/plugin-hooks'; +import type { OutgoingEmail } from '@/lib/plugin-types'; + +/** + * allowed: send. vetoed: a plugin said no and told the user. failed: a + * handler threw or ran out of time (nobody told the user yet). + */ +export type PluginSendCheckOutcome = 'allowed' | 'vetoed' | 'failed'; + +export async function runPluginSendCheck(email: OutgoingEmail): Promise { + return emailHooks.onBeforeEmailSend.interceptOutcome(email); +} diff --git a/lib/outgoing/quick-reply.ts b/lib/outgoing/quick-reply.ts index e4fc39d19..41bf5cf84 100644 --- a/lib/outgoing/quick-reply.ts +++ b/lib/outgoing/quick-reply.ts @@ -3,6 +3,7 @@ import { appendHtmlSignature, appendPlainTextSignature } from '@/lib/signature-u import { currentSignatureIdentity } from '@/lib/org/signature-state'; import { findUnfilledPlaceholders, type PlaceholderWords } from '@/lib/placeholder-check'; import { finalizeOutgoing, runPreSendChecks, type OutgoingBodies } from './send-pipeline'; +import { runPluginSendCheck, type PluginSendCheckOutcome } from './plugin-send-check'; // Registers the organisation signature check and the disclaimer. import '@/lib/org/outgoing'; @@ -29,7 +30,10 @@ export interface QuickReplyQuestions { * checks and finalisers as the composer's send. Null when one stopped the * send. The organisation signature applies as in the composer * (`signatureIdentity` is what the reply would sign with on its own, - * `sendingIdentity` the identity it goes out from). + * `sendingIdentity` the identity it goes out from). Plugins' send checks + * (onBeforeEmailSend) run on it too, after the organisation signature and + * before the disclaimer, as in the composer; `onSendCheckStopped` hears + * why one stopped it. */ export async function prepareQuickReply(input: { body: string; @@ -39,7 +43,11 @@ export async function prepareQuickReply(input: { to: string[]; fromEmail?: string; identityId?: string; + /** The reply's subject (without a classification prefix) and the original's Message-ID, for plugins. */ + subject?: string; + inReplyTo?: string; questions?: QuickReplyQuestions; + onSendCheckStopped?: (outcome: Exclude) => void; }): Promise { const { body, separator, questions } = input; @@ -88,6 +96,25 @@ export async function prepareQuickReply(input: { if (!(await runPreSendChecks(context))) return null; // Asked last, as in the composer, so an answered warning doesn't ask twice. if (questions?.confirmSend && !(await questions.ask({ kind: 'send' }))) return null; - // Quick reply has no plugin step in between: both stages at once. - return finalizeOutgoing(finalizeOutgoing({ html, text }, context), context, 'last'); + const bodies = finalizeOutgoing({ html, text }, context); + // A quick reply used to skip plugins' send checks: a way around them. + const outcome = await runPluginSendCheck({ + to: input.to, + cc: [], + bcc: [], + subject: input.subject ?? '', + htmlBody: bodies.html ?? '', + textBody: bodies.text, + identityId: input.identityId ?? '', + fromEmail: input.fromEmail, + attachments: [], + inReplyTo: input.inReplyTo, + surface: 'quick-reply', + mode: 'reply', + }); + if (outcome !== 'allowed') { + input.onSendCheckStopped?.(outcome); + return null; + } + return finalizeOutgoing(bodies, context, 'last'); } diff --git a/lib/plugin-types.ts b/lib/plugin-types.ts index 7e404f567..ee62206d8 100644 --- a/lib/plugin-types.ts +++ b/lib/plugin-types.ts @@ -1091,6 +1091,13 @@ export interface OutgoingEmail { inReplyTo?: string; /** Free-form custom headers added by the composer or earlier handlers */ headers?: Record; + /** + * Where the send comes from (onBeforeEmailSend, Bulwark 1.20 and later): + * the composer, a quick reply under a message, or a plugin's forward. + */ + surface?: 'composer' | 'quick-reply' | 'plugin-forward'; + /** compose, reply, replyAll or forward (onBeforeEmailSend, 1.20 and later). */ + mode?: 'compose' | 'reply' | 'replyAll' | 'forward'; } /** * Passed to onBeforeDraftAutoSave handlers as a transform value.