From f8805b8d526cfa16f281d8c5b635b62ac772c1cb Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sat, 31 Oct 2026 17:32:24 +0100 Subject: [PATCH 1/5] fix: keep remote content blocked for trusted senders when the organisation locks it to block --- .../remote-content-locked-block.test.tsx | 110 ++++++++++++++++++ components/email/email-composer.tsx | 4 +- components/email/email-viewer.tsx | 8 +- components/email/thread-conversation-view.tsx | 9 +- hooks/use-third-party-mode.ts | 31 +++++ 5 files changed, 153 insertions(+), 9 deletions(-) create mode 100644 components/email/__tests__/remote-content-locked-block.test.tsx diff --git a/components/email/__tests__/remote-content-locked-block.test.tsx b/components/email/__tests__/remote-content-locked-block.test.tsx new file mode 100644 index 000000000..30234fa74 --- /dev/null +++ b/components/email/__tests__/remote-content-locked-block.test.tsx @@ -0,0 +1,110 @@ +import { act, render, screen } from '@testing-library/react'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import type { Email, Mailbox } from '@/lib/jmap/types'; +import { useSettingsStore } from '@/stores/settings-store'; +import { usePolicyStore } from '@/stores/policy-store'; +import { remoteContentBlockedByOrg } from '@/hooks/use-third-party-mode'; + +// An organisation that locks the external content setting to "block" means +// it: a sender the user trusted before (or trusts now) doesn't load remote +// content, and nothing offers to trust or load. + +vi.mock('@/hooks/use-media-query', () => ({ + useDeviceDetection: () => ({ isMobile: false, isTablet: false, isDesktop: true }), + useMediaQuery: () => false, +})); +vi.mock('@/components/tour/tour-provider', () => ({ + useTour: () => ({ activeTour: null, startTour: vi.fn(), endTour: vi.fn(), registerStep: vi.fn() }), +})); +vi.mock('@/components/plugins/plugin-slot', () => ({ PluginSlot: () => null })); +vi.mock('@/hooks/use-plugin-slot-offers', () => ({ usePluginSlotOffers: () => [] })); +vi.mock('@/hooks/use-wopi-status', () => ({ useWopiStatus: () => ({ enabled: false }), canWopiOpen: () => false })); +vi.mock('@/hooks/use-own-domain-address', () => ({ useOwnDomainAddress: () => () => false })); +vi.stubGlobal('ResizeObserver', class { observe() {} unobserve() {} disconnect() {} }); + +const { EmailViewer } = await import('../email-viewer'); +const { ThreadConversationView } = await import('../thread-conversation-view'); + +const mailboxes = [{ id: 'inbox', name: 'Inbox', role: 'inbox', parentId: null, myRights: { mayAddItems: true } }] as unknown as Mailbox[]; +const email = { + id: 'e1', blobId: 'b-e1', threadId: 't1', mailboxIds: { inbox: true }, keywords: { $seen: true }, + from: [{ name: 'News', email: 'news@shop.example' }], to: [{ email: 'me@example.org' }], + subject: 'Offer', receivedAt: '2026-10-01T10:00:00Z', preview: 'Hello', hasAttachment: false, size: 100, + textBody: [], htmlBody: [{ partId: '1', type: 'text/html' }], attachments: [], + bodyValues: { '1': { value: '

Hello

' } }, +} as unknown as Email; + +const initialPolicy = usePolicyStore.getState().policy; + +function lockExternalContent(restriction: Record, defaults: Record = {}) { + usePolicyStore.setState({ + policy: { + ...initialPolicy, + restrictions: { externalContentPolicy: restriction }, + defaults: { ...initialPolicy.defaults, ...defaults }, + }, + }); +} + +afterEach(() => { + usePolicyStore.setState({ policy: initialPolicy }); + useSettingsStore.setState({ externalContentPolicy: 'ask', trustedSenders: [] }); +}); + +async function renderViewer() { + render(); + await act(async () => { await new Promise((r) => setTimeout(r, 30)); }); +} + +async function renderConversation() { + const view = render( + {}} />, + ); + await act(async () => { await new Promise((r) => setTimeout(r, 30)); }); + return view; +} + +describe('external content locked to block', () => { + it('a trusted sender still loads remote content when the user chose block', async () => { + useSettingsStore.setState({ externalContentPolicy: 'block', trustedSenders: ['news@shop.example'] }); + await renderViewer(); + expect(screen.queryByTestId('external-content-notice')).toBeNull(); + }); + + it('keeps a trusted sender blocked in the reading pane, with no trust button', async () => { + lockExternalContent({ locked: true, value: 'block' }); + useSettingsStore.setState({ externalContentPolicy: 'block', trustedSenders: ['news@shop.example'] }); + await renderViewer(); + const notice = await screen.findByTestId('external-content-notice'); + expect(notice.textContent).toContain('remote_content_blocked'); + expect(screen.queryByText('trust_sender')).toBeNull(); + expect(screen.queryByText('load_external_content')).toBeNull(); + }); + + it('keeps a trusted sender blocked in the conversation view, with nothing offered', async () => { + lockExternalContent({ locked: true, value: 'block' }); + useSettingsStore.setState({ externalContentPolicy: 'block', trustedSenders: ['news@shop.example'] }); + const { container } = await renderConversation(); + expect(screen.getByTestId('external-content-notice').textContent).toContain('third_party.remote_content_blocked'); + expect(screen.queryByText('email_viewer.load_external_content')).toBeNull(); + expect(screen.queryByText('email_viewer.trust_sender')).toBeNull(); + const srcdoc = container.querySelector('iframe')?.getAttribute('srcdoc') ?? ''; + expect(srcdoc).not.toMatch(/\ssrc="https:\/\/track\.shop\.example/); + }); +}); + +describe('remoteContentBlockedByOrg', () => { + it('is true for a lock to block, also when the block comes from the defaults', () => { + lockExternalContent({ locked: true, value: 'block' }); + expect(remoteContentBlockedByOrg()).toBe(true); + lockExternalContent({ locked: true }, { externalContentPolicy: 'block' }); + expect(remoteContentBlockedByOrg()).toBe(true); + }); + + it('is false for a lock to ask, and for block without a lock', () => { + lockExternalContent({ locked: true, value: 'ask' }); + expect(remoteContentBlockedByOrg()).toBe(false); + lockExternalContent({ value: 'block' }); + expect(remoteContentBlockedByOrg()).toBe(false); + }); +}); diff --git a/components/email/email-composer.tsx b/components/email/email-composer.tsx index 8b30992c8..85fb6d1c0 100644 --- a/components/email/email-composer.tsx +++ b/components/email/email-composer.tsx @@ -96,6 +96,7 @@ import { personalTemplatesFor } from "@/lib/templates/personal-templates"; import { notifyIdentitySelected } from "@/stores/identity-store"; import { usePluginSignature } from "@/hooks/use-plugin-signature"; import { useOrgSignature, useOrgSignatureIdentity } from "@/hooks/use-org-signature"; +import { remoteContentBlockedByOrg } from "@/hooks/use-third-party-mode"; import { OrgDisclaimerPreview } from "@/components/email/org-disclaimer-preview"; // Registers the organisation signature check and the disclaimer on the send path. import "@/lib/org/outgoing"; @@ -745,7 +746,8 @@ export function EmailComposer({ // Whether the quoted original may show its remote images in the editor: // the viewer's rule, minus the per-message "load" click it doesn't know. const { externalContentPolicy, isSenderTrusted } = useSettingsStore.getState(); - const quoteRemoteAllowed = externalContentPolicy === 'allow' || (!!from?.email && isSenderTrusted(from.email)); + const quoteRemoteAllowed = !remoteContentBlockedByOrg() + && (externalContentPolicy === 'allow' || (!!from?.email && isSenderTrusted(from.email))); const replyHeader = (html: string) => mode === 'reply' || mode === 'replyAll' ? markReplyQuoteHeader(html) diff --git a/components/email/email-viewer.tsx b/components/email/email-viewer.tsx index eab3e454c..1f61e39a9 100644 --- a/components/email/email-viewer.tsx +++ b/components/email/email-viewer.tsx @@ -172,7 +172,7 @@ import type { AttachmentInfo, AttachmentPreview } from "@/lib/plugin-types"; import { useAttachmentDrag, isDragOutSupported, type AttachmentDragSource } from "@/hooks/use-attachment-drag"; import type { IJMAPClient } from "@/lib/jmap/client-interface"; import { useArchiveActionEnabled } from "@/lib/archive-gate"; -import { useNoThirdPartyTraffic } from "@/hooks/use-third-party-mode"; +import { useRemoteContentBlockedByOrg } from "@/hooks/use-third-party-mode"; import { useSenderWarning } from "@/hooks/use-sender-warning"; import { BrandVerifiedBadge } from "@/components/email/brand-verified-badge"; @@ -1313,9 +1313,9 @@ export function EmailViewer({ const tWelcome = useTranslations('welcome'); const quickReplyEnabled = useSettingsStore((state) => state.quickReplyEnabled); const externalContentPolicy = useSettingsStore((state) => state.externalContentPolicy); - // No third-party traffic (#903): remote content never loads, whatever the - // policy or trust says, and nothing offers to load it. - const remoteBlockedByOrg = useNoThirdPartyTraffic(); + // No third-party traffic (#903), or the organisation locked "block": remote + // content never loads, whatever trust says, and nothing offers to load it. + const remoteBlockedByOrg = useRemoteContentBlockedByOrg(); const tThirdParty = useTranslations('third_party'); const messageSpacing = useSettingsStore((state) => state.messageSpacing); const plainTextFont = useSettingsStore((state) => state.plainTextFont); diff --git a/components/email/thread-conversation-view.tsx b/components/email/thread-conversation-view.tsx index 8a5861dc2..f4c65ec09 100644 --- a/components/email/thread-conversation-view.tsx +++ b/components/email/thread-conversation-view.tsx @@ -66,7 +66,7 @@ import { useDeviceDetection } from "@/hooks/use-media-query"; import { usePluginRenderedBody } from "@/hooks/use-plugin-rendered-body"; import { emailToReadView } from "@/lib/plugin-projection"; import { AttachmentPluginMenu, ReaderAboveQuoteSlot } from "./reader-plugin-surfaces"; -import { useNoThirdPartyTraffic } from "@/hooks/use-third-party-mode"; +import { useRemoteContentBlockedByOrg } from "@/hooks/use-third-party-mode"; import { ConversationHeaderActions, type ConversationActions } from "./conversation-header-actions"; export type ConversationAttachmentHandler = ( @@ -107,8 +107,9 @@ export function useConversationExternalContent() { const addToTrustedSendersBook = useContactStore((state) => state.addToTrustedSendersBook); const { client } = useAuthStore(useShallow((state) => ({ client: state.client }))); const [allowedIds, setAllowedIds] = useState>(new Set()); - // No third-party traffic (#903): nothing remote loads and nothing offers to. - const remoteBlockedByOrg = useNoThirdPartyTraffic(); + // No third-party traffic (#903), or the organisation locked "block": + // nothing remote loads and nothing offers to. + const remoteBlockedByOrg = useRemoteContentBlockedByOrg(); const allow = useCallback((emailId: string) => { setAllowedIds(prev => new Set(prev).add(emailId)); @@ -542,7 +543,7 @@ export function ConversationEmailBody({ bodyStyle, className, }: ConversationEmailBodyProps) { - const remoteBlockedByOrg = useNoThirdPartyTraffic(); + const remoteBlockedByOrg = useRemoteContentBlockedByOrg(); const t = useTranslations(); const resolvedTheme = useThemeStore((state) => state.resolvedTheme); const mailAttachmentAction = useSettingsStore((state) => state.mailAttachmentAction); diff --git a/hooks/use-third-party-mode.ts b/hooks/use-third-party-mode.ts index 5f81f5e9b..db949ac1d 100644 --- a/hooks/use-third-party-mode.ts +++ b/hooks/use-third-party-mode.ts @@ -2,6 +2,7 @@ import { useSyncExternalStore } from 'react'; import { noThirdPartyTraffic, subscribeThirdPartyMode } from '@/lib/third-party-mode'; +import { usePolicyStore } from '@/stores/policy-store'; /** * Whether the browser must stay off built-in outside hosts (#903), as React @@ -11,3 +12,33 @@ import { noThirdPartyTraffic, subscribeThirdPartyMode } from '@/lib/third-party- export function useNoThirdPartyTraffic(): boolean { return useSyncExternalStore(subscribeThirdPartyMode, noThirdPartyTraffic, () => true); } + +type PolicyState = ReturnType; + +/** The external content setting locked to "block" by the organisation. */ +function externalContentLockedToBlock(state: PolicyState): boolean { + const restriction = state.getRestriction('externalContentPolicy'); + if (restriction?.locked !== true) return false; + // A lock without a value locks the organisation's default (as the + // settings store applies it). + const value = Object.prototype.hasOwnProperty.call(restriction, 'value') + ? restriction.value + : state.getEffectiveDefault('externalContentPolicy'); + return value === 'block'; +} + +/** + * Whether the organisation keeps remote content in mail from loading at all: + * no third-party traffic, or the external content setting locked to "block". + * Then neither a one-off load nor a trusted sender lets it through. + */ +export function useRemoteContentBlockedByOrg(): boolean { + const noThirdParty = useNoThirdPartyTraffic(); + const lockedToBlock = usePolicyStore(externalContentLockedToBlock); + return noThirdParty || lockedToBlock; +} + +/** useRemoteContentBlockedByOrg, read once outside React. */ +export function remoteContentBlockedByOrg(): boolean { + return noThirdPartyTraffic() || externalContentLockedToBlock(usePolicyStore.getState()); +} From 73a7bb7bcd5de51aec10b561b2caf202ba586030 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sat, 31 Oct 2026 17:40:20 +0100 Subject: [PATCH 2/5] fix: send the headers plugins add to an outgoing message instead of dropping them --- .../__tests__/composer-plugin-send.test.tsx | 15 ++- components/email/email-composer.tsx | 3 + components/mail/mail-app.tsx | 3 +- lib/jmap/client-interface.ts | 2 +- lib/jmap/client.ts | 4 + lib/outgoing/__tests__/custom-headers.test.ts | 126 ++++++++++++++++++ lib/outgoing/custom-headers.ts | 67 ++++++++++ stores/email-store.ts | 2 +- 8 files changed, 218 insertions(+), 4 deletions(-) create mode 100644 lib/outgoing/__tests__/custom-headers.test.ts create mode 100644 lib/outgoing/custom-headers.ts diff --git a/components/email/__tests__/composer-plugin-send.test.tsx b/components/email/__tests__/composer-plugin-send.test.tsx index 308d766b7..b95a9f450 100644 --- a/components/email/__tests__/composer-plugin-send.test.tsx +++ b/components/email/__tests__/composer-plugin-send.test.tsx @@ -6,6 +6,7 @@ import { EmailComposer } from '../email-composer'; const hoisted = vi.hoisted(() => ({ takeOver: vi.fn(), toastError: vi.fn(), + transform: vi.fn(async (email: unknown) => email), })); // ─── Heavy component mocks (mirrors composer-from-override-envelope.test.tsx) ─ @@ -158,7 +159,7 @@ vi.mock('@/lib/plugin-hooks', () => ({ onBeforeDraftAutoSave: { transform: async (draft: unknown) => draft }, onBeforeEmailSend: { intercept: async () => true, interceptOutcome: async () => 'allowed' }, onComposeSend: { takeOver: hoisted.takeOver }, - onTransformOutgoingEmail: { transform: async (email: unknown) => email }, + onTransformOutgoingEmail: { transform: hoisted.transform }, }, contactHooks: { search: { call: async () => [] }, @@ -254,4 +255,16 @@ describe('composer send taken over by a plugin (#1207)', () => { await waitFor(() => expect(onSend).toHaveBeenCalledTimes(1)); }); + + it('sends the headers a plugin added to the outgoing message', async () => { + hoisted.takeOver.mockResolvedValue('default'); + hoisted.transform.mockImplementationOnce(async (email: unknown) => ({ + ...(email as Record), + headers: { 'X-Classification': 'Internal' }, + })); + const { onSend } = renderComposer(); + + await waitFor(() => expect(onSend).toHaveBeenCalledTimes(1)); + expect(onSend.mock.calls[0][0].headers).toEqual({ 'X-Classification': 'Internal' }); + }); }); diff --git a/components/email/email-composer.tsx b/components/email/email-composer.tsx index 85fb6d1c0..9ff4bf140 100644 --- a/components/email/email-composer.tsx +++ b/components/email/email-composer.tsx @@ -297,6 +297,8 @@ interface EmailComposerProps { requireTls?: boolean; /** Importance / X-Priority; absent for normal (#970). */ priority?: MessagePriority; + /** Extra headers plugins added in onTransformOutgoingEmail. */ + headers?: Record; /** The id plugins know this composer by, for onAfterEmailSend. */ composerId?: string; }) => void | false | Promise; @@ -3697,6 +3699,7 @@ export function EmailComposer({ requestDsn: requestDsn || undefined, requireTls: requireTls || undefined, priority: priority !== 'normal' ? priority : undefined, + headers: outgoing.headers, delayedUntil: effectiveDelayedUntil, composerId: pluginComposerIdRef.current ?? undefined, }); diff --git a/components/mail/mail-app.tsx b/components/mail/mail-app.tsx index ea048013a..9b37c8029 100644 --- a/components/mail/mail-app.tsx +++ b/components/mail/mail-app.tsx @@ -2154,6 +2154,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { requestDsn?: boolean; requireTls?: boolean; priority?: MessagePriority; + headers?: Record; composerId?: string; }) => { // Nothing is sent: the composer keeps what only goes once the mail is out. @@ -2178,7 +2179,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { const effectiveMode = pendingDraft?.mode ?? composerMode; const originalEmailId = composerSourceEmail?.id; - const result = await sendEmail(sendClient, data.to, data.subject, data.body, data.cc, data.bcc, data.identityId, data.fromEmail, data.draftId, data.fromName, data.htmlBody, data.attachments, data.inReplyTo, data.references, data.delayedUntil, data.envelopeMailFrom, { requestReadReceipt: data.requestReadReceipt, requestDsn: data.requestDsn, requireTls: data.requireTls, priority: data.priority, localAccountId: data.localAccountId, draftAccountId: data.draftAccountId }); + const result = await sendEmail(sendClient, data.to, data.subject, data.body, data.cc, data.bcc, data.identityId, data.fromEmail, data.draftId, data.fromName, data.htmlBody, data.attachments, data.inReplyTo, data.references, data.delayedUntil, data.envelopeMailFrom, { requestReadReceipt: data.requestReadReceipt, requestDsn: data.requestDsn, requireTls: data.requireTls, priority: data.priority, headers: data.headers, localAccountId: data.localAccountId, draftAccountId: data.draftAccountId }); submitted = true; announceSent(result, data); setShowComposer(false); diff --git a/lib/jmap/client-interface.ts b/lib/jmap/client-interface.ts index f806987eb..8e3ae5177 100644 --- a/lib/jmap/client-interface.ts +++ b/lib/jmap/client-interface.ts @@ -449,7 +449,7 @@ export interface IJMAPClient { // supportsSubmissionExtension). // `draftAccountId`: the account `draftId` lives in, so the sent draft is // destroyed there (default: the primary account). - options?: { requestReadReceipt?: boolean; requestDsn?: boolean; requireTls?: boolean; priority?: MessagePriority; draftAccountId?: string }, + options?: { requestReadReceipt?: boolean; requestDsn?: boolean; requireTls?: boolean; priority?: MessagePriority; headers?: Record; draftAccountId?: string }, ): Promise; /** Whether the submission account advertises an SMTP extension ("DSN", "REQUIRETLS", …). */ supportsSubmissionExtension?(extension: string, accountId?: string): boolean; diff --git a/lib/jmap/client.ts b/lib/jmap/client.ts index cf4bd6a55..ae878398b 100644 --- a/lib/jmap/client.ts +++ b/lib/jmap/client.ts @@ -44,6 +44,7 @@ import { parseAuthenticationResults } from "@/lib/email-headers"; import { IS_LITE_STALWART } from "@/lib/lite"; import { toStorageQuota, type JMAPQuota, type StorageQuota } from "@/lib/quota"; import { messagePriorityHeaders, type MessagePriority } from "@/lib/message-priority"; +import { customHeaderProperties } from "@/lib/outgoing/custom-headers"; import { attachmentParts, HTML_PART_ID, relatedBodyStructure, TEXT_PART_ID, type OutgoingBodyPart } from "./outgoing-body"; // Cap for the follow-up Email/get issued when a displayed body part comes @@ -1420,6 +1421,8 @@ export interface SubmissionSendOptions { requireTls?: boolean; /** Importance / X-Priority headers (#970); see lib/message-priority.ts. */ priority?: MessagePriority; + /** Extra headers plugins added; see lib/outgoing/custom-headers.ts. */ + headers?: Record; /** The account the replaced draft (`draftId`) lives in; default the primary. */ draftAccountId?: string; } @@ -4943,6 +4946,7 @@ export class JMAPClient implements IJMAPClient { } Object.assign(emailCreate, messagePriorityHeaders(options?.priority)); + Object.assign(emailCreate, customHeaderProperties(options?.headers)); // Images the HTML shows by cid: go into a multipart/related beside it, // or recipients don't see them (#1451). diff --git a/lib/outgoing/__tests__/custom-headers.test.ts b/lib/outgoing/__tests__/custom-headers.test.ts new file mode 100644 index 000000000..785857783 --- /dev/null +++ b/lib/outgoing/__tests__/custom-headers.test.ts @@ -0,0 +1,126 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { JMAPClient } from '@/lib/jmap/client'; +import { customHeaderProperties } from '../custom-headers'; + +// Headers a plugin adds in onTransformOutgoingEmail (a classification label, +// for one) go out with the message. Ones that would change the sender, clash +// with what the client writes itself or forge trace headers don't. + +describe('customHeaderProperties', () => { + it('turns plugin headers into JMAP header properties', () => { + expect(customHeaderProperties({ 'X-Classification': 'Confidential', Sensitivity: 'Company-Confidential' })).toEqual({ + 'header:X-Classification:asText': 'Confidential', + 'header:Sensitivity:asText': 'Company-Confidential', + }); + }); + + it('drops headers that say who sent the mail, how it travelled, or that the client writes', () => { + expect(customHeaderProperties({ + From: 'boss@example.com', + sender: 'boss@example.com', + 'Reply-To': 'x@evil.example', + Bcc: 'spy@evil.example', + 'Message-ID': '', + 'Content-Type': 'text/plain', + Received: 'from somewhere', + 'Authentication-Results': 'mx; dkim=pass', + 'DKIM-Signature': 'v=1', + 'ARC-Seal': 'i=1', + 'Resent-From': 'a@example.com', + 'X-Priority': '1', + Importance: 'high', + 'Disposition-Notification-To': 'me@example.com', + })).toEqual({}); + }); + + it('drops values with line breaks or control characters', () => { + expect(customHeaderProperties({ + 'X-Label': 'ok\r\nBcc: spy@evil.example', + 'X-Other': 'a\nb', + 'X-Nul': 'a\u0000b', + })).toEqual({}); + }); + + it('drops invalid names, empty or non-text values and repeats in another case', () => { + expect(customHeaderProperties({ + 'X Bad': 'space', + 'X:Bad': 'colon', + 'X-Ümlaut': 'non-ascii', + 'X-Empty': ' ', + 'X-Number': 5 as unknown as string, + 'X-Label': 'first', + 'x-label': 'second', + })).toEqual({ 'header:X-Label:asText': 'first' }); + }); + + it('ignores anything that is not a plain header map', () => { + expect(customHeaderProperties(undefined)).toEqual({}); + expect(customHeaderProperties(['X-A'])).toEqual({}); + expect(customHeaderProperties('X-A: b')).toEqual({}); + }); + + it('caps the number of headers', () => { + const many = Object.fromEntries(Array.from({ length: 30 }, (_, i) => [`X-H${i}`, 'v'])); + expect(Object.keys(customHeaderProperties(many))).toHaveLength(20); + }); +}); + +describe('sendEmail with plugin headers', () => { + interface CapturedRequest { methodCalls: Array<[string, Record, string]> } + + beforeEach(() => { + vi.restoreAllMocks(); + vi.spyOn(console, 'error').mockImplementation(() => {}); + }); + + function mockFlow() { + const captured: CapturedRequest[] = []; + vi.spyOn(globalThis, 'fetch').mockImplementation(async (_url, init) => { + const body = JSON.parse((init as { body: string }).body) as CapturedRequest; + captured.push(body); + const methodResponses: unknown[] = []; + for (const [method, args, callId] of body.methodCalls) { + if (method === 'Mailbox/get') { + methodResponses.push(['Mailbox/get', { + list: [ + { id: 'mb-drafts', name: 'Drafts', role: 'drafts' }, + { id: 'mb-sent', name: 'Sent', role: 'sent' }, + ], + }, callId]); + } else if (method === 'Identity/get') { + methodResponses.push(['Identity/get', { list: [{ id: 'identity-1', email: 'user@example.com' }] }, callId]); + } else if (method === 'Email/set') { + const key = Object.keys((args.create ?? {}) as Record)[0]; + methodResponses.push(['Email/set', key ? { created: { [key]: { id: 'email-new-1' } } } : {}, callId]); + } else if (method === 'EmailSubmission/set') { + methodResponses.push(['EmailSubmission/set', { created: { '1': { id: 'sub-1' } } }, callId]); + } + } + const payload = { methodResponses }; + return { + ok: true, + status: 200, + text: () => Promise.resolve(JSON.stringify(payload)), + json: () => Promise.resolve(payload), + } as Response; + }); + return captured; + } + + it('puts the plugin headers on the created message and leaves the sender alone', async () => { + const client = new JMAPClient('https://jmap.example.com', 'user@example.com', 'pass'); + Object.assign(client, { apiUrl: 'https://jmap.example.com/api', accountId: 'account-1', username: 'user@example.com' }); + const captured = mockFlow(); + await client.sendEmail( + ['to@example.com'], 'Subject', 'body', + undefined, undefined, 'identity-1', 'user@example.com', + undefined, undefined, undefined, undefined, undefined, undefined, undefined, undefined, + { headers: { 'X-Classification': 'Internal', From: 'boss@example.com' } }, + ); + const call = captured.flatMap(r => r.methodCalls).find(c => c[0] === 'Email/set' && c[1].create); + const created = Object.values(call![1].create as Record>)[0]; + expect(created['header:X-Classification:asText']).toBe('Internal'); + expect(created['header:From:asText']).toBeUndefined(); + expect(created.from).toEqual([{ email: 'user@example.com' }]); + }); +}); diff --git a/lib/outgoing/custom-headers.ts b/lib/outgoing/custom-headers.ts new file mode 100644 index 000000000..0058d8aee --- /dev/null +++ b/lib/outgoing/custom-headers.ts @@ -0,0 +1,67 @@ +import { debug } from '@/lib/debug'; + +/** + * Extra headers plugins add to an outgoing message (OutgoingEmail.headers), + * such as a classification label, as JMAP Email/set create properties + * (RFC 8621 §4.1.3, `header:Name:asText`, which the server encodes and folds). + * + * Only headers of their own get through. Names the client writes from other + * properties are dropped, because JMAP refuses a create that sets the same + * header twice, and so are names that say who sent the mail or how it + * travelled: a plugin must not change the sender or forge trace and + * authentication headers. + */ + +const RESERVED = new Set([ + 'from', 'sender', 'reply-to', 'to', 'cc', 'bcc', 'subject', 'date', + 'message-id', 'in-reply-to', 'references', 'mime-version', + 'return-path', 'received', 'received-spf', 'authentication-results', + 'dkim-signature', 'domainkey-signature', + 'disposition-notification-to', 'importance', 'x-priority', 'priority', +]); +const RESERVED_PREFIXES = ['content-', 'resent-', 'arc-']; + +/** RFC 5322 §3.6.8 field-name: printable US-ASCII except the colon. */ +const FIELD_NAME = /^[\x21-\x39\x3b-\x7e]{1,76}$/; +/** Line breaks or other control characters but tab (header injection). */ +function hasControlCharacter(value: string): boolean { + for (let i = 0; i < value.length; i++) { + const code = value.charCodeAt(i); + if ((code < 0x20 && code !== 0x09) || code === 0x7f) return true; + } + return false; +} +const MAX_HEADERS = 20; +// Room for an Autocrypt header with a whole key in it; the server folds it. +const MAX_VALUE_LENGTH = 8192; + +function isReserved(lowerName: string): boolean { + return RESERVED.has(lowerName) || RESERVED_PREFIXES.some((prefix) => lowerName.startsWith(prefix)); +} + +export function customHeaderProperties(headers: unknown): Record { + const out: Record = {}; + if (!headers || typeof headers !== 'object' || Array.isArray(headers)) return out; + const seen = new Set(); + for (const [name, raw] of Object.entries(headers as Record)) { + const lower = name.toLowerCase(); + if (!FIELD_NAME.test(name) || isReserved(lower) || seen.has(lower)) { + debug.warn(`Outgoing header "${name}" from a plugin was not sent`); + continue; + } + if (typeof raw !== 'string') continue; + const value = raw.trim(); + if (!value) continue; + if (hasControlCharacter(value) || value.length > MAX_VALUE_LENGTH) { + debug.warn(`Outgoing header "${name}" from a plugin was not sent: invalid value`); + continue; + } + if (seen.size >= MAX_HEADERS) { + debug.warn(`Outgoing header "${name}" from a plugin was not sent: more than ${MAX_HEADERS} headers`); + continue; + } + seen.add(lower); + out[`header:${name}:asText`] = value; + } + return out; +} diff --git a/stores/email-store.ts b/stores/email-store.ts index e0f0ee36c..8f4583738 100644 --- a/stores/email-store.ts +++ b/stores/email-store.ts @@ -437,7 +437,7 @@ interface EmailStore { /** `ownerAccountId`: the JMAP account a link names for the message (#1351). */ fetchEmailContent: (client: IJMAPClient, emailId: string, ownerAccountId?: string) => Promise; fetchQuota: (client: IJMAPClient) => Promise; - sendEmail: (client: IJMAPClient, to: string[], subject: string, body: string, cc?: string[], bcc?: string[], identityId?: string, fromEmail?: string, draftId?: string, fromName?: string, htmlBody?: string, attachments?: Array<{ blobId: string; name: string; type: string; size: number; disposition?: 'attachment' | 'inline'; cid?: string }>, inReplyTo?: string[], references?: string[], delayedUntil?: string, envelopeMailFrom?: string, options?: { requestReadReceipt?: boolean; requestDsn?: boolean; requireTls?: boolean; priority?: MessagePriority; localAccountId?: string; draftAccountId?: string }) => Promise; + sendEmail: (client: IJMAPClient, to: string[], subject: string, body: string, cc?: string[], bcc?: string[], identityId?: string, fromEmail?: string, draftId?: string, fromName?: string, htmlBody?: string, attachments?: Array<{ blobId: string; name: string; type: string; size: number; disposition?: 'attachment' | 'inline'; cid?: string }>, inReplyTo?: string[], references?: string[], delayedUntil?: string, envelopeMailFrom?: string, options?: { requestReadReceipt?: boolean; requestDsn?: boolean; requireTls?: boolean; priority?: MessagePriority; headers?: Record; localAccountId?: string; draftAccountId?: string }) => Promise; sendRawEmail: (client: IJMAPClient, rawMimeBlob: Blob, identityId: string, delayedUntil?: string, envelopeRecipients?: string[]) => Promise; /** `emailKey` (emailKeyFor of the row) picks the right row when the id repeats across accounts. */ deleteEmail: (client: IJMAPClient, emailId: string, forceDelete?: boolean, emailKey?: string) => Promise; From a9e93881b563a22d714a92e14c815443a568ecb5 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sat, 31 Oct 2026 17:45:26 +0100 Subject: [PATCH 3/5] feat: match filter conditions against a regular expression #885 --- .../filter-rule-modal-regex.test.tsx | 69 +++++++++ components/filters/filter-rule-modal.tsx | 47 +++++-- components/filters/quick-rule-host.tsx | 2 + components/settings/filter-settings.tsx | 2 + lib/jmap/sieve-types.ts | 3 + lib/sieve/__tests__/regex.test.ts | 132 ++++++++++++++++++ lib/sieve/generator.ts | 16 ++- lib/sieve/parser.ts | 12 +- lib/sieve/regex.ts | 42 ++++++ locales/ar/common.json | 3 + locales/ca/common.json | 3 + locales/cs/common.json | 3 + locales/da/common.json | 3 + locales/de/common.json | 3 + locales/en/common.json | 3 + locales/es/common.json | 3 + locales/et/common.json | 3 + locales/fa/common.json | 3 + locales/fr/common.json | 3 + locales/he/common.json | 3 + locales/hu/common.json | 3 + locales/it/common.json | 3 + locales/ja/common.json | 3 + locales/ko/common.json | 3 + locales/lv/common.json | 3 + locales/mn/common.json | 3 + locales/nb/common.json | 3 + locales/nl/common.json | 3 + locales/pl/common.json | 3 + locales/pt/common.json | 3 + locales/ro/common.json | 3 + locales/ru/common.json | 3 + locales/sk/common.json | 3 + locales/th/common.json | 3 + locales/tr/common.json | 3 + locales/uk/common.json | 3 + locales/vi/common.json | 3 + locales/zh-TW/common.json | 3 + locales/zh/common.json | 3 + 39 files changed, 400 insertions(+), 15 deletions(-) create mode 100644 components/filters/__tests__/filter-rule-modal-regex.test.tsx create mode 100644 lib/sieve/__tests__/regex.test.ts create mode 100644 lib/sieve/regex.ts diff --git a/components/filters/__tests__/filter-rule-modal-regex.test.tsx b/components/filters/__tests__/filter-rule-modal-regex.test.tsx new file mode 100644 index 000000000..5e31d6533 --- /dev/null +++ b/components/filters/__tests__/filter-rule-modal-regex.test.tsx @@ -0,0 +1,69 @@ +import { fireEvent, render, screen } from '@testing-library/react'; +import { describe, expect, it, vi } from 'vitest'; +import React from 'react'; +import type { FilterRule } from '@/lib/jmap/sieve-types'; +import { FilterRuleModal } from '../filter-rule-modal'; + +vi.mock('@/stores/toast-store', () => ({ toast: { error: vi.fn(), success: vi.fn(), info: vi.fn() } })); + +const rule: FilterRule = { + id: 'r1', + name: 'Cron mail', + enabled: true, + matchType: 'all', + conditions: [{ field: 'subject', comparator: 'contains', value: '' }], + actions: [{ type: 'mark_read' }], + stopProcessing: false, +}; + +function openModal(props: Partial> = {}) { + const onSave = vi.fn(); + render(); + return { onSave, saved: () => onSave.mock.calls[0][0] as FilterRule }; +} + +const fieldSelect = () => screen.getByLabelText('conditions') as HTMLSelectElement; +const comparatorSelect = () => screen.getByLabelText('comparators.contains') as HTMLSelectElement; +const comparatorOptions = () => [...comparatorSelect().options].map((o) => o.value); +const valueInput = () => screen.getByLabelText('condition_value') as HTMLInputElement; + +describe('the "matches regex" comparator', () => { + it('is offered only when the server lists the regex extension', () => { + openModal(); + expect(comparatorOptions()).not.toContain('regex'); + }); + + it('is offered on header fields and the body, not on size or attachments', () => { + openModal({ regexSupported: true }); + expect(comparatorOptions()).toContain('regex'); + fireEvent.change(fieldSelect(), { target: { value: 'body' } }); + expect(comparatorOptions()).toContain('regex'); + fireEvent.change(fieldSelect(), { target: { value: 'size' } }); + expect(comparatorOptions()).not.toContain('regex'); + }); + + it('keeps commas inside the expression instead of splitting it into values', () => { + const { saved } = openModal({ regexSupported: true }); + fireEvent.change(comparatorSelect(), { target: { value: 'regex' } }); + fireEvent.change(valueInput(), { target: { value: '^Cron, job{1,3}' } }); + fireEvent.blur(valueInput()); + expect(valueInput().value).toBe('^Cron, job{1,3}'); + fireEvent.click(screen.getByText('save')); + expect(saved().conditions).toEqual([{ field: 'subject', comparator: 'regex', value: '^Cron, job{1,3}' }]); + }); + + it('does not save an expression the server would refuse', () => { + const { onSave } = openModal({ regexSupported: true }); + fireEvent.change(comparatorSelect(), { target: { value: 'regex' } }); + fireEvent.change(valueInput(), { target: { value: '(unclosed' } }); + fireEvent.click(screen.getByText('save')); + expect(onSave).not.toHaveBeenCalled(); + expect(screen.getByText('validation_regex')).toBeTruthy(); + expect(valueInput().getAttribute('aria-invalid')).toBe('true'); + }); + + it('stays on a saved regex rule when the server does not list the extension', () => { + openModal({ initialRule: { ...rule, conditions: [{ field: 'subject', comparator: 'regex', value: '^Cron' }] } }); + expect(comparatorSelect().value).toBe('regex'); + }); +}); diff --git a/components/filters/filter-rule-modal.tsx b/components/filters/filter-rule-modal.tsx index e1d8ffe8a..71a23aac4 100644 --- a/components/filters/filter-rule-modal.tsx +++ b/components/filters/filter-rule-modal.tsx @@ -18,6 +18,7 @@ import type { } from "@/lib/jmap/sieve-types"; import type { Mailbox } from "@/lib/jmap/types"; import { isValidSieveHeaderName } from "@/lib/sieve/generator"; +import { REGEX_FIELDS, isValidSieveRegex, usesRegex } from "@/lib/sieve/regex"; import { buildMailboxTree, flattenMailboxTree, type MailboxNode, generateUUID, cn } from "@/lib/utils"; import type { RuleSuggestion } from "@/lib/filters/quick-rules"; import { retroactiveSupport } from "@/lib/filters/retroactive"; @@ -50,6 +51,8 @@ interface FilterRuleModalProps { forwardsAfter?: number; /** The server can confine a rule to a period (Sieve "date" and "relational"). */ periodsSupported?: boolean; + /** The server lists the Sieve "regex" extension. */ + regexSupported?: boolean; onSave: (rule: FilterRule, options?: { applyToExisting: boolean }) => void; onClose: () => void; } @@ -72,7 +75,7 @@ const ATTACHMENT_COMPARATORS: FilterComparator[] = ["has_any", "has_type"]; // "All messages" has nothing to choose: its one comparator is not shown. const ALL_MESSAGES_COMPARATORS: FilterComparator[] = ["any"]; -function comparatorsFor(field: FilterConditionField): FilterComparator[] { +function baseComparatorsFor(field: FilterConditionField): FilterComparator[] { if (field === "size") return SIZE_COMPARATORS; if (field === "attachment") return ATTACHMENT_COMPARATORS; if (field === "all") return ALL_MESSAGES_COMPARATORS; @@ -80,6 +83,13 @@ function comparatorsFor(field: FilterConditionField): FilterComparator[] { return TEXT_COMPARATORS; } +// "matches regex" only where the server can run it, or on a row that already +// uses it, so editing such a rule does not change its comparator. +function comparatorsFor(field: FilterConditionField, regex: boolean): FilterComparator[] { + const base = baseComparatorsFor(field); + return regex && REGEX_FIELDS.has(field) ? [...base, "regex"] : base; +} + const ALL_ACTION_TYPES: FilterActionType[] = [ "move", "copy", "forward", "mark_read", "star", "add_label", "discard", "reject", "keep", "stop", ]; @@ -117,6 +127,13 @@ function takesNoValue(c: Pick): boolean return c.field === "all" || (c.field === "attachment" && c.comparator === "has_any"); } +// A regex row whose expression (or one of them) would not compile. +function regexInvalid(c: FilterCondition): boolean { + if (!usesRegex(c) || isConditionValueEmpty(c.value)) return false; + const values = Array.isArray(c.value) ? c.value : [c.value]; + return values.some((v) => !isValidSieveRegex(v)); +} + function makeEmptyAction(): FilterAction { return { type: "move", value: "" }; } @@ -162,6 +179,7 @@ export function FilterRuleModal({ forwardsBefore = 0, forwardsAfter = 0, periodsSupported = false, + regexSupported = false, onSave, onClose, }: FilterRuleModalProps) { @@ -255,6 +273,9 @@ export function FilterRuleModal({ if (takesNoValue(c)) return c; if (c.field === "size") return c; // numeric, single-value only if (typeof c.value !== "string") return c; // already structured + // A comma belongs to the expression ("a{1,3}"); several patterns are + // written with "|". + if (usesRegex(c)) return c; const parsed = inputStringToValue(c.value); return { ...c, value: parsed }; }) @@ -306,7 +327,11 @@ export function FilterRuleModal({ // it the whole script, so every other rule would stop working too. : validConditions.some((c) => c.field === "header" && !isValidSieveHeaderName(c.headerName ?? "")) ? t("validation_header_name") - : null, + // The server compiles every expression on save and refuses the + // whole script for one that does not compile. + : validConditions.some(regexInvalid) + ? t("validation_regex") + : null, // A row that is there but unfinished says what it misses; "at least one // action" is left for when no row has a type at all. actions: firstIncompleteAction @@ -339,6 +364,7 @@ export function FilterRuleModal({ const headerInvalid = (c: FilterCondition) => !!shown(errors.conditions) && c.field === "header" && !isConditionValueEmpty(c.value) && !isValidSieveHeaderName((c.headerName ?? "").trim()); + const regexInvalidShown = (c: FilterCondition) => !!shown(errors.conditions) && regexInvalid(c); const actionInvalid = (index: number) => !!shown(errors.actions) && actionIncomplete[index]; const handleSave = useCallback(() => { @@ -394,7 +420,7 @@ export function FilterRuleModal({ // with e.g. (field=attachment, comparator=contains) — invalid for the // Sieve generator. Each field has its own valid comparator set. if (updates.field && updates.field !== c.field) { - const allowed = comparatorsFor(updates.field); + const allowed = comparatorsFor(updates.field, regexSupported); if (!allowed.includes(c.comparator)) { updated.comparator = allowed[0]; } @@ -579,7 +605,7 @@ export function FilterRuleModal({ className={selectClass} aria-label={t("comparators.contains")} > - {comparatorsFor(condition.field).map((c) => ( + {comparatorsFor(condition.field, regexSupported || condition.comparator === "regex").map((c) => ( @@ -607,6 +633,7 @@ export function FilterRuleModal({ // stays numeric/single-value; attachment-has_any has // no value at all. if (condition.field === "size") return; + if (usesRegex(condition)) return; if ( condition.field === "attachment" && condition.comparator === "has_any" @@ -627,13 +654,15 @@ export function FilterRuleModal({ ? t("size_placeholder") : condition.field === "attachment" ? t("attachment_type_placeholder") - : t("value_placeholder_multi") + : usesRegex(condition) + ? t("regex_placeholder") + : t("value_placeholder_multi") } aria-label={t("condition_value")} - aria-invalid={valueMissing(condition)} - aria-describedby={valueMissing(condition) ? "rule-conditions-error" : undefined} - data-rule-error={valueMissing(condition)} - className={cn("flex-1 min-w-[120px]", valueMissing(condition) && "border-destructive")} + aria-invalid={valueMissing(condition) || regexInvalidShown(condition)} + aria-describedby={valueMissing(condition) || regexInvalidShown(condition) ? "rule-conditions-error" : undefined} + data-rule-error={valueMissing(condition) || regexInvalidShown(condition)} + className={cn("flex-1 min-w-[120px]", (valueMissing(condition) || regexInvalidShown(condition)) && "border-destructive")} type={condition.field === "size" ? "number" : "text"} /> )} diff --git a/components/filters/quick-rule-host.tsx b/components/filters/quick-rule-host.tsx index 74640e5e9..41fb33acc 100644 --- a/components/filters/quick-rule-host.tsx +++ b/components/filters/quick-rule-host.tsx @@ -10,6 +10,7 @@ import { readAccountFilters } from "@/lib/filters/account-filters"; import { forwardsAround, inRunOrder } from "@/lib/filters/forward-limit"; import { createFolderAndRunPreset, saveEditorRule } from "@/lib/filters/quick-rule-flow"; import { supportsPeriods } from "@/lib/sieve/period"; +import { supportsRegex } from "@/lib/sieve/regex"; /** * Draws what a rule made from a message opens: the rule editor ("Create @@ -61,6 +62,7 @@ export function QuickRuleHost() { forwardsBefore={forwards.before} forwardsAfter={forwards.after} periodsSupported={supportsPeriods(editor.target.client.getSieveCapabilities(editor.target.sieveAccountId)?.sieveExtensions)} + regexSupported={supportsRegex(editor.target.client.getSieveCapabilities(editor.target.sieveAccountId)?.sieveExtensions)} onSave={(rule, options) => { closeEditor(); void saveEditorRule(editor, rule, options?.applyToExisting ?? false, text); diff --git a/components/settings/filter-settings.tsx b/components/settings/filter-settings.tsx index 02ba079f2..299dd08dd 100644 --- a/components/settings/filter-settings.tsx +++ b/components/settings/filter-settings.tsx @@ -20,6 +20,7 @@ import type { FilterRule } from "@/lib/jmap/sieve-types"; import type { Mailbox } from "@/lib/jmap/types"; import { forwardsAround, inRunOrder, worstCaseForwards } from "@/lib/filters/forward-limit"; import { isPeriodBoundary, periodStatus, supportsPeriods } from "@/lib/sieve/period"; +import { supportsRegex } from "@/lib/sieve/regex"; import { formatDateTime } from "@/lib/utils"; import { useVacationStore } from "@/stores/vacation-store"; import { useManagedAccountStore } from "@/stores/managed-account-store"; @@ -787,6 +788,7 @@ export function FilterSettings() { mailboxes={mailboxes} maxRedirects={sieveCapabilities?.maxNumberRedirects} periodsSupported={supportsPeriods(sieveCapabilities?.sieveExtensions)} + regexSupported={supportsRegex(sieveCapabilities?.sieveExtensions)} forwardsBefore={forwardsOfEdited.before} forwardsAfter={forwardsOfEdited.after} onSave={handleSaveRule} diff --git a/lib/jmap/sieve-types.ts b/lib/jmap/sieve-types.ts index 8713f585f..a9ff53a91 100644 --- a/lib/jmap/sieve-types.ts +++ b/lib/jmap/sieve-types.ts @@ -26,6 +26,9 @@ export type FilterComparator = | 'is' | 'not_is' | 'starts_with' | 'ends_with' | 'matches' + // Sieve `:regex` (the "regex" extension), on header fields and the body. + // The value is one regular expression; commas in it are not separators. + | 'regex' | 'greater_than' | 'less_than' // For field === 'attachment': // has_any → message has any attachment (Content-Disposition: attachment) diff --git a/lib/sieve/__tests__/regex.test.ts b/lib/sieve/__tests__/regex.test.ts new file mode 100644 index 000000000..3013da3e9 --- /dev/null +++ b/lib/sieve/__tests__/regex.test.ts @@ -0,0 +1,132 @@ +import { describe, it, expect } from 'vitest'; +import { generateScript } from '../generator'; +import { parseScript } from '../parser'; +import { isValidSieveRegex, supportsRegex } from '../regex'; +import type { FilterCondition, FilterRule } from '@/lib/jmap/sieve-types'; + +function makeRule(conditions: FilterCondition[], overrides: Partial = {}): FilterRule { + return { + id: 'rule-1', + name: 'Cron mail', + enabled: true, + matchType: 'any', + conditions, + actions: [{ type: 'move', value: 'wartung' }], + stopProcessing: true, + ...overrides, + }; +} + +function requireLine(script: string): string { + return script.split('\n').find((l) => l.startsWith('require ')) ?? ''; +} + +function metadataVersion(script: string): number { + const json = script.split('\n')[1]; + return JSON.parse(json).version; +} + +describe('regex conditions in the generator', () => { + it('writes header :regex and requires "regex"', () => { + const script = generateScript([ + makeRule([{ field: 'subject', comparator: 'regex', value: '^Cron.*|^Anacron.*Job.*|.*mailman.*moderator.*request.*' }]), + ]); + expect(script).toContain('if header :regex "Subject" "^Cron.*|^Anacron.*Job.*|.*mailman.*moderator.*request.*" {'); + expect(requireLine(script)).toContain('"regex"'); + }); + + it('doubles backslashes so Sieve hands the server the expression as typed', () => { + const script = generateScript([ + makeRule([{ field: 'from', comparator: 'regex', value: '@(onedomain\\.com|otherdomain\\.de)' }]), + ]); + expect(script).toContain('header :regex "From" "@(onedomain\\\\.com|otherdomain\\\\.de)"'); + }); + + it('writes a custom header and the body with :regex', () => { + const script = generateScript([ + makeRule([ + { field: 'header', headerName: 'List-Id', comparator: 'regex', value: 'dev-[0-9]+' }, + { field: 'body', comparator: 'regex', value: 'inv[0-9]{4,6}' }, + ]), + ]); + expect(script).toContain('header :regex "List-Id" "dev-[0-9]+"'); + expect(script).toContain('body :regex "inv[0-9]{4,6}"'); + expect(requireLine(script)).toContain('"body"'); + expect(requireLine(script)).toContain('"regex"'); + }); + + it('requires "regex" only when an enabled rule uses it', () => { + const plain = generateScript([makeRule([{ field: 'subject', comparator: 'contains', value: 'Cron' }])]); + expect(requireLine(plain)).not.toContain('"regex"'); + const disabled = generateScript([ + makeRule([{ field: 'subject', comparator: 'regex', value: '^Cron' }], { enabled: false }), + ]); + expect(requireLine(disabled)).not.toContain('"regex"'); + expect(disabled).not.toContain(':regex'); + }); + + it('marks the metadata version 2 so older builds leave the rule alone', () => { + expect(metadataVersion(generateScript([makeRule([{ field: 'subject', comparator: 'contains', value: 'Cron' }])]))).toBe(1); + expect(metadataVersion(generateScript([makeRule([{ field: 'subject', comparator: 'regex', value: '^Cron' }])]))).toBe(2); + }); +}); + +describe('regex conditions in the parser', () => { + it('reads a saved regex rule back unchanged', () => { + const rule = makeRule([ + { field: 'from', comparator: 'regex', value: '@(onedomain\\.com|otherdomain\\.de)' }, + { field: 'body', comparator: 'regex', value: 'a{1,3}' }, + ]); + const parsed = parseScript(generateScript([rule])); + expect(parsed.isOpaque).toBe(false); + expect(parsed.rules[0].conditions).toEqual(rule.conditions); + }); + + it('imports a hand-written header :regex rule as a regex condition', () => { + const parsed = parseScript([ + 'require ["fileinto", "regex"];', + 'if anyof(header :regex "Subject" "^Cron.*|^Anacron.*Job.*") {', + ' fileinto "wartung";', + ' stop;', + '}', + ].join('\n')); + expect(parsed.isOpaque).toBe(false); + expect(parsed.rules[0].conditions).toEqual([ + { field: 'subject', comparator: 'regex', value: '^Cron.*|^Anacron.*Job.*' }, + ]); + }); + + it('does not read "not header :regex" as a positive match', () => { + const parsed = parseScript([ + 'require ["fileinto", "regex"];', + 'if not header :regex "Subject" "^Cron" {', + ' fileinto "wartung";', + '}', + ].join('\n')); + const conditions = parsed.rules.flatMap((r) => r.conditions); + expect(conditions).not.toContainEqual({ field: 'subject', comparator: 'regex', value: '^Cron' }); + }); +}); + +describe('regex helpers', () => { + it('needs the server to list the extension', () => { + expect(supportsRegex(['fileinto', 'regex'])).toBe(true); + expect(supportsRegex(['fileinto'])).toBe(false); + expect(supportsRegex(undefined)).toBe(false); + }); + + it('accepts expressions the server compiles, including Rust inline flags', () => { + expect(isValidSieveRegex('^Cron.*|mailman')).toBe(true); + expect(isValidSieveRegex('a{1,3}')).toBe(true); + expect(isValidSieveRegex('(?i)^cron')).toBe(true); + expect(isValidSieveRegex('(?i:cron)job')).toBe(true); + expect(isValidSieveRegex('(?P[a-z]+)@')).toBe(true); + }); + + it('rejects expressions the server refuses', () => { + expect(isValidSieveRegex('(unclosed')).toBe(false); + expect(isValidSieveRegex('[abc')).toBe(false); + expect(isValidSieveRegex('*abc')).toBe(false); + expect(isValidSieveRegex('')).toBe(false); + }); +}); diff --git a/lib/sieve/generator.ts b/lib/sieve/generator.ts index de7e442f1..7d879dc0c 100644 --- a/lib/sieve/generator.ts +++ b/lib/sieve/generator.ts @@ -3,6 +3,7 @@ import { debug } from '@/lib/debug'; import { PERIOD_REQUIRES, hasPeriod, periodTests } from '@/lib/sieve/period'; import { VACATION_FORWARD_MARKER, isValidVacationForward, normalizeVacationForward } from '@/lib/sieve/vacation-forward'; import { isValidVacationAudience, normalizeVacationAudience } from '@/lib/sieve/vacation-audience'; +import { usesRegex } from '@/lib/sieve/regex'; const HEADER_MAP: Record = { from: 'From', @@ -73,7 +74,7 @@ function generateCondition(condition: FilterCondition): string { const values = toValueList(value); if (field === 'body') { - const matchType = comparator === 'is' ? ':is' : ':contains'; + const matchType = comparator === 'is' ? ':is' : comparator === 'regex' ? ':regex' : ':contains'; return `body ${matchType} ${formatStringArg(values)}`; } @@ -123,6 +124,8 @@ function generateCondition(condition: FilterCondition): string { return `header :matches "${headerName}" ${formatStringArg(values, (v) => `*${v}`)}`; case 'matches': return `header :matches "${headerName}" ${formatStringArg(values)}`; + case 'regex': + return `header :regex "${headerName}" ${formatStringArg(values)}`; default: return `header :contains "${headerName}" ${formatStringArg(values)}`; } @@ -219,6 +222,7 @@ function computeRequires( for (const condition of rule.conditions) { if (condition.field === 'body') extensions.add('body'); if (condition.field === 'attachment') extensions.add('mime'); + if (usesRegex(condition)) extensions.add('regex'); } for (const action of rule.actions) { switch (action.type) { @@ -342,8 +346,14 @@ export function generateScript( // without what it does not know: a rule without its period forwards for // good, the auto-reply goes to everyone. It leaves version 2 alone. A // forward that is off only loses its remembered address that way, so it - // stays version 1: the native app still edits version 1 only. - if (metadata.rules.some(hasPeriod) || metadata.vacationForward?.enabled || metadata.vacationAudience) { + // stays version 1: the native app still edits version 1 only. A regex + // condition would come back from those as a plain "contains". + if ( + metadata.rules.some(hasPeriod) || + metadata.rules.some(r => r.conditions.some(usesRegex)) || + metadata.vacationForward?.enabled || + metadata.vacationAudience + ) { metadata.version = 2; } // The JSON sits inside a /* ... */ comment: a "*/" in any string (a rule diff --git a/lib/sieve/parser.ts b/lib/sieve/parser.ts index 67641deff..c38be1b80 100644 --- a/lib/sieve/parser.ts +++ b/lib/sieve/parser.ts @@ -497,11 +497,13 @@ function parseAtom(raw: string): FilterCondition | null { return { field, comparator: part === ':domain' ? 'domain_is' : 'address_is', value }; } - m = /^header\s+:(contains|is|matches)\s+"((?:[^"\\]|\\.)*)"\s+([\s\S]+)$/.exec(s); + m = /^header\s+:(contains|is|matches|regex)\s+"((?:[^"\\]|\\.)*)"\s+([\s\S]+)$/.exec(s); if (m) { const [, tag, headerName, rawTail] = m; const value = parseValueTail(rawTail); if (value === null) return null; + // The builder has no "does not match regex". + if (tag === 'regex' && negated) return null; const { field, headerName: customHeaderName } = normalizeHeaderName(unescapeSieveString(headerName)); let comparator: FilterComparator; @@ -512,6 +514,9 @@ function parseAtom(raw: string): FilterCondition | null { } else if (tag === 'is') { comparator = negated ? 'not_is' : 'is'; finalValue = value; + } else if (tag === 'regex') { + comparator = 'regex'; + finalValue = value; } else { const classified = classifyMatches(value); comparator = classified.comparator; @@ -523,11 +528,12 @@ function parseAtom(raw: string): FilterCondition | null { return cond; } - m = /^body\s+:(contains|is)\s+([\s\S]+)$/.exec(s); + m = /^body\s+:(contains|is|regex)\s+([\s\S]+)$/.exec(s); if (m) { const value = parseValueTail(m[2]); if (value === null) return null; - return { field: 'body', comparator: m[1] === 'is' ? 'is' : 'contains', value }; + const comparator = m[1] === 'is' ? 'is' : m[1] === 'regex' ? 'regex' : 'contains'; + return { field: 'body', comparator, value }; } m = /^size\s+:(over|under)\s+(\d+)$/.exec(s); diff --git a/lib/sieve/regex.ts b/lib/sieve/regex.ts new file mode 100644 index 000000000..9a3f3e4a0 --- /dev/null +++ b/lib/sieve/regex.ts @@ -0,0 +1,42 @@ +import type { FilterCondition, FilterConditionField } from '@/lib/jmap/sieve-types'; + +/** + * Regular expression conditions: Sieve `:regex` from the "regex" extension + * (draft-ietf-sieve-regex). Stalwart lists it in the account's + * sieveExtensions and compiles the expression when the script is saved, so + * one broken expression makes the server refuse the whole script. + */ + +/** Fields the generator writes `:regex` for: header tests and `body`. */ +export const REGEX_FIELDS: ReadonlySet = new Set([ + 'from', 'to', 'cc', 'subject', 'header', 'body', +]); + +export function supportsRegex(extensions: readonly string[] | undefined | null): boolean { + return !!extensions?.includes('regex'); +} + +export function usesRegex(condition: Pick): boolean { + return condition.comparator === 'regex' && REGEX_FIELDS.has(condition.field); +} + +/** + * Whether `pattern` is likely to compile on the server. Stalwart uses Rust + * regex syntax, so its inline flags ("(?i)", "(?s:...)") and "(?P" + * groups are rewritten into something JavaScript parses before the check. + * JavaScript accepts a few things Rust does not; the server still has the + * last word on those. + */ +export function isValidSieveRegex(pattern: string): boolean { + if (!pattern) return false; + const js = pattern + .replace(/\(\?[a-zA-Z]+\)/g, '') + .replace(/\(\?[a-zA-Z-]+:/g, '(?:') + .replace(/\(\?P Date: Sat, 31 Oct 2026 17:50:07 +0100 Subject: [PATCH 4/5] fix: keep plugin credentials when the docker container is recreated --- .env.example | 5 ++ Dockerfile | 2 +- docker-compose.yml | 4 ++ lib/__tests__/docker-data-volumes.test.ts | 66 +++++++++++++++++++++++ lib/plugin-secrets/vault.ts | 2 +- lib/settings-sync.ts | 2 +- lib/telemetry/state.ts | 2 +- 7 files changed, 79 insertions(+), 4 deletions(-) create mode 100644 lib/__tests__/docker-data-volumes.test.ts diff --git a/.env.example b/.env.example index d30fa4c61..e76b3c38d 100644 --- a/.env.example +++ b/.env.example @@ -148,6 +148,11 @@ JMAP_SERVER_URL=https://your-jmap-server.com # /app/data/settings - mount a persistent volume there (see docker-compose.yml). # SETTINGS_DATA_DIR=./data/settings +# Directory for credentials users save for plugins, encrypted (default: +# plugin-secrets next to SETTINGS_DATA_DIR, so /app/data/plugin-secrets in +# Docker) - mount a persistent volume there (see docker-compose.yml). +# PLUGIN_SECRETS_DIR=./data/plugin-secrets + # ============================================================================= # Admin Dashboard Data # ============================================================================= diff --git a/Dockerfile b/Dockerfile index 8b3346acc..5ab63eddb 100644 --- a/Dockerfile +++ b/Dockerfile @@ -56,7 +56,7 @@ RUN apk upgrade --no-cache && \ COPY --from=builder /app/public ./public COPY --from=builder /app/.next/standalone ./ COPY --from=builder /app/.next/static ./.next/static -RUN mkdir -p /app/.next/cache /app/data/settings /app/data/admin /app/data/admin-state /app/data/telemetry && \ +RUN mkdir -p /app/.next/cache /app/data/settings /app/data/admin /app/data/admin-state /app/data/telemetry /app/data/plugin-secrets && \ chown -R nextjs:nodejs /app/.next/cache /app/data USER nextjs EXPOSE 3000 diff --git a/docker-compose.yml b/docker-compose.yml index 928b41b16..dea974dc2 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -21,6 +21,9 @@ services: # Anonymous telemetry: instance id, consent state, login HMACs (TELEMETRY_DATA_DIR). # Persisting this preserves the admin's consent choice and stable instance id across upgrades. - bulwark-telemetry:/app/data/telemetry + # Credentials users save for plugins, encrypted (PLUGIN_SECRETS_DIR). + # Without this volume they are lost whenever the container is recreated. + - bulwark-plugin-secrets:/app/data/plugin-secrets healthcheck: test: [ @@ -42,3 +45,4 @@ volumes: bulwark-admin: bulwark-admin-state: bulwark-telemetry: + bulwark-plugin-secrets: diff --git a/lib/__tests__/docker-data-volumes.test.ts b/lib/__tests__/docker-data-volumes.test.ts new file mode 100644 index 000000000..354af4903 --- /dev/null +++ b/lib/__tests__/docker-data-volumes.test.ts @@ -0,0 +1,66 @@ +import { readFileSync } from 'node:fs'; +import path from 'node:path'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { getConfigDir, getStateDir } from '@/lib/admin/paths'; +import { getSettingsDir } from '@/lib/settings-sync'; +import { getDir as getTelemetryDir } from '@/lib/telemetry/state'; +import { vaultDir } from '@/lib/plugin-secrets/vault'; + +// Every folder the server keeps state in by default must be a volume in the +// shipped docker-compose.yml, or it is lost when the container is recreated, +// and must exist in the image, or Docker creates the empty volume owned by +// root and the app can't write to it. Plugin secrets were missing from both. + +const ROOT = path.resolve(__dirname, '..', '..'); +const ENV_KEYS = [ + 'ADMIN_CONFIG_DIR', 'ADMIN_DATA_DIR', 'ADMIN_STATE_DIR', 'SETTINGS_DATA_DIR', + 'TELEMETRY_DATA_DIR', 'PLUGIN_SECRETS_DIR', +]; +const savedEnv: Record = {}; + +beforeEach(() => { + for (const key of ENV_KEYS) { + savedEnv[key] = process.env[key]; + delete process.env[key]; + } + vi.spyOn(process, 'cwd').mockReturnValue('/app'); +}); + +afterEach(() => { + for (const key of ENV_KEYS) { + if (savedEnv[key] === undefined) delete process.env[key]; + else process.env[key] = savedEnv[key]; + } + vi.restoreAllMocks(); +}); + +/** The default state folders inside the container, as the server resolves them. */ +function defaultStateDirs(): string[] { + return [getConfigDir(), getStateDir(), getSettingsDir(), getTelemetryDir(), vaultDir()] + .map((dir) => path.resolve(dir).replace(/\\/g, '/').replace(/^[A-Za-z]:/, '')); +} + +describe('docker data volumes', () => { + it('resolves the plugin secrets folder inside /app/data', () => { + expect(defaultStateDirs()).toContain('/app/data/plugin-secrets'); + }); + + it('mounts a volume on every default state folder in docker-compose.yml', () => { + const compose = readFileSync(path.join(ROOT, 'docker-compose.yml'), 'utf8'); + const mounted = [...compose.matchAll(/^\s*-\s*[\w.-]+:(\/app\/[\w/.-]+?)(?::ro)?\s*$/gm)].map((m) => m[1]); + for (const dir of defaultStateDirs()) expect(mounted).toContain(dir); + }); + + it('declares every named volume the service uses', () => { + const compose = readFileSync(path.join(ROOT, 'docker-compose.yml'), 'utf8'); + const used = [...compose.matchAll(/^\s*-\s*([\w-]+):\/app\//gm)].map((m) => m[1]); + const declared = compose.split(/^volumes:\s*$/m)[1] ?? ''; + for (const name of used) expect(declared).toMatch(new RegExp(`^\\s+${name}:`, 'm')); + }); + + it('creates every default state folder in the image', () => { + const dockerfile = readFileSync(path.join(ROOT, 'Dockerfile'), 'utf8'); + const created = dockerfile.match(/RUN mkdir -p ([^&\n]+)/)?.[1].trim().split(/\s+/) ?? []; + for (const dir of defaultStateDirs()) expect(created).toContain(dir); + }); +}); diff --git a/lib/plugin-secrets/vault.ts b/lib/plugin-secrets/vault.ts index 063521cae..5a1eee25c 100644 --- a/lib/plugin-secrets/vault.ts +++ b/lib/plugin-secrets/vault.ts @@ -40,7 +40,7 @@ type VaultData = Record>; /** Cap on credentials per user, across all plugins. */ export const MAX_USER_CREDENTIALS = 64; -function vaultDir(): string { +export function vaultDir(): string { return process.env.PLUGIN_SECRETS_DIR || path.join(process.env.SETTINGS_DATA_DIR || path.join(process.cwd(), 'data', 'settings'), '..', 'plugin-secrets'); } diff --git a/lib/settings-sync.ts b/lib/settings-sync.ts index 6a5fb1be4..dc819046b 100644 --- a/lib/settings-sync.ts +++ b/lib/settings-sync.ts @@ -15,7 +15,7 @@ function getKey(): Buffer { return createHash('sha256').update(secret).digest(); } -function getSettingsDir(): string { +export function getSettingsDir(): string { return process.env.SETTINGS_DATA_DIR || path.join(process.cwd(), 'data', 'settings'); } diff --git a/lib/telemetry/state.ts b/lib/telemetry/state.ts index ef211e8f6..57dc68db5 100644 --- a/lib/telemetry/state.ts +++ b/lib/telemetry/state.ts @@ -7,7 +7,7 @@ import type { TelemetryStateFile, ConsentState } from './types'; import { DEFAULT_ENDPOINT } from './types'; import { isNoThirdPartyTrafficOn } from '@/lib/admin/third-party-mode'; -function getDir(): string { +export function getDir(): string { return process.env.TELEMETRY_DATA_DIR || path.join(process.cwd(), 'data', 'telemetry'); } From ddc24e4fc71fe4ff0ec5b7d31a9a0161fd49428d Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sat, 31 Oct 2026 17:55:29 +0100 Subject: [PATCH 5/5] fix: let a browser an admin signed in from before past the shared admin login limit --- app/api/admin/auth/route.ts | 12 +- .../admin-auth-stalwart-access.test.ts | 3 + .../admin-login-device-limit.test.ts | 106 +++++++++++++++ lib/admin/rate-limit.ts | 35 ++++- lib/admin/session.ts | 127 +++++++++++++----- 5 files changed, 245 insertions(+), 38 deletions(-) create mode 100644 lib/__tests__/admin-login-device-limit.test.ts diff --git a/app/api/admin/auth/route.ts b/app/api/admin/auth/route.ts index baaf584b8..8dd405cc2 100644 --- a/app/api/admin/auth/route.ts +++ b/app/api/admin/auth/route.ts @@ -1,8 +1,8 @@ import { NextRequest, NextResponse } from 'next/server'; import { createHash } from 'crypto'; import { initAdminPassword, verifyAdminPassword, updateLastLogin, isAdminEnabled, getAdminMeta } from '@/lib/admin/password'; -import { setAdminSessionCookie, clearAdminSessionCookie, requireAdminAuth, getClientIP, isSameOriginRequest } from '@/lib/admin/session'; -import { checkRateLimit } from '@/lib/admin/rate-limit'; +import { setAdminSessionCookie, clearAdminSessionCookie, requireAdminAuth, getClientIP, isSameOriginRequest, getAdminDeviceId, setAdminDeviceCookie } from '@/lib/admin/session'; +import { checkRateLimit, checkDeviceRateLimit } from '@/lib/admin/rate-limit'; import { auditLog } from '@/lib/admin/audit'; import { logger } from '@/lib/logger'; import { getStalwartCredentials } from '@/lib/stalwart/credentials'; @@ -160,6 +160,7 @@ export async function POST(request: NextRequest) { } await setAdminSessionCookie(request); + await setAdminDeviceCookie(request); await auditLog('admin.login', { method: 'stalwart' }, ip); return NextResponse.json({ ok: true }); } @@ -170,8 +171,10 @@ export async function POST(request: NextRequest) { return NextResponse.json({ error: 'Admin dashboard is not configured' }, { status: 404 }); } - // Rate limit check - const limit = checkRateLimit(ip); + // Rate limit check. A browser an admin signed in from before is limited + // on its own, so others using up the shared budget can't lock it out. + const deviceId = await getAdminDeviceId(); + const limit = deviceId ? checkDeviceRateLimit(deviceId) : checkRateLimit(ip); if (!limit.allowed) { const retryAfter = Math.ceil(limit.retryAfterMs / 1000); await auditLog('admin.login_blocked', { reason: 'rate_limit' }, ip); @@ -195,6 +198,7 @@ export async function POST(request: NextRequest) { } await setAdminSessionCookie(request); + await setAdminDeviceCookie(request); await updateLastLogin(); await auditLog('admin.login', {}, ip); diff --git a/lib/__tests__/admin-auth-stalwart-access.test.ts b/lib/__tests__/admin-auth-stalwart-access.test.ts index 3958c544f..cf56d0ab4 100644 --- a/lib/__tests__/admin-auth-stalwart-access.test.ts +++ b/lib/__tests__/admin-auth-stalwart-access.test.ts @@ -48,10 +48,13 @@ vi.mock('@/lib/admin/session', () => ({ requireAdminAuth: async () => (state.hasAdminSession ? { role: 'admin' } : { error: 'unauthorized' }), getClientIP: () => '127.0.0.1', isSameOriginRequest: () => true, + getAdminDeviceId: async () => null, + setAdminDeviceCookie: vi.fn(), })); vi.mock('@/lib/admin/rate-limit', () => ({ checkRateLimit: () => ({ allowed: true, retryAfterMs: 0 }), + checkDeviceRateLimit: () => ({ allowed: true, retryAfterMs: 0 }), })); const auditLog = vi.fn(); diff --git a/lib/__tests__/admin-login-device-limit.test.ts b/lib/__tests__/admin-login-device-limit.test.ts new file mode 100644 index 000000000..dd29b2eb2 --- /dev/null +++ b/lib/__tests__/admin-login-device-limit.test.ts @@ -0,0 +1,106 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +// Admin password login allows 50 attempts across all clients per 15 minutes, +// because a client without a proxy in front can make up its X-Forwarded-For +// address. Anyone could spend that budget and lock every admin out. A browser +// an admin signed in from before carries a device cookie and is limited on +// its own (OWASP device cookies), so it still gets in. + +vi.mock('next/server', () => ({ + NextResponse: { + json: (data: unknown, init?: { status?: number }) => ({ json: async () => data, status: init?.status ?? 200 }), + }, +})); + +// One cookie jar per browser; `current` is the browser making the request. +const browsers = { current: new Map() }; +vi.mock('next/headers', () => ({ + cookies: async () => ({ + get: (name: string) => (browsers.current.has(name) ? { name, value: browsers.current.get(name)! } : undefined), + set: (name: string, value: string) => { browsers.current.set(name, value); }, + delete: (name: string) => { browsers.current.delete(name); }, + }), +})); + +vi.mock('@/lib/logger', () => ({ logger: { warn: vi.fn(), error: vi.fn(), debug: vi.fn(), info: vi.fn() } })); +vi.mock('@/lib/admin/audit', () => ({ auditLog: vi.fn() })); +vi.mock('@/lib/security/same-origin', () => ({ isSameOriginRequest: () => true })); +vi.mock('@/lib/admin/config-manager', () => ({ + configManager: { ensureLoaded: vi.fn(), get: (_key: string, fallback?: unknown) => fallback }, +})); +vi.mock('@/lib/stalwart/credentials', () => ({ getStalwartCredentials: async () => null })); +vi.mock('@/lib/admin/stalwart-admin-access', () => ({ getStalwartAdminAccessMode: async () => 'password' })); +vi.mock('@/lib/admin/password', () => ({ + initAdminPassword: vi.fn().mockResolvedValue(true), + isAdminEnabled: () => true, + verifyAdminPassword: async (password: string) => password === 'right-password', + updateLastLogin: vi.fn(), + getAdminMeta: () => null, + getAdminSessionGeneration: async () => 0, + isAdminSessionRevoked: async () => false, + revokeAdminSession: vi.fn(), +})); + +process.env.SESSION_SECRET = 'a'.repeat(48); + +type Route = typeof import('@/app/api/admin/auth/route'); +let POST: Route['POST']; + +beforeEach(async () => { + // Fresh rate limiter state for every test. + vi.resetModules(); + ({ POST } = await import('@/app/api/admin/auth/route')); +}); + +async function login(browser: Map, password: string, ip: string): Promise { + browsers.current = browser; + const request = new Request('https://bulwark.example/api/admin/auth', { + method: 'POST', + headers: { 'content-type': 'application/json', 'x-forwarded-for': ip }, + body: JSON.stringify({ password }), + }); + const response = await POST(request as never); + return response.status; +} + +/** Wrong passwords from 50 made-up addresses: the whole shared budget. */ +async function spendSharedBudget() { + const attacker = new Map(); + for (let i = 0; i < 50; i++) await login(attacker, 'guess', `198.51.100.${i}`); + expect(await login(attacker, 'guess', '198.51.100.200')).toBe(429); +} + +describe('admin login device cookie', () => { + it('lets a browser an admin signed in from before in after others spent the shared budget', async () => { + const admin = new Map(); + expect(await login(admin, 'right-password', '203.0.113.7')).toBe(200); + expect(admin.has('admin_device')).toBe(true); + + await spendSharedBudget(); + + expect(await login(admin, 'right-password', '203.0.113.7')).toBe(200); + }); + + it('still blocks a new browser once the shared budget is spent', async () => { + await spendSharedBudget(); + expect(await login(new Map(), 'right-password', '203.0.113.8')).toBe(429); + }); + + it('limits a device on its own to 5 attempts', async () => { + const admin = new Map(); + expect(await login(admin, 'right-password', '203.0.113.7')).toBe(200); + for (let i = 0; i < 5; i++) expect(await login(admin, 'wrong', `192.0.2.${i}`)).toBe(401); + expect(await login(admin, 'right-password', '192.0.2.99')).toBe(429); + }); + + it('does not take an admin session token as a device cookie', async () => { + const { createAdminSession, createAdminDeviceToken, verifyAdminDeviceToken, verifyAdminSession } = await import('@/lib/admin/session'); + expect(verifyAdminDeviceToken(createAdminSession())).toBeNull(); + expect(verifyAdminSession(createAdminDeviceToken())).toBeNull(); + expect(verifyAdminDeviceToken(createAdminDeviceToken())).toEqual(expect.any(String)); + + await spendSharedBudget(); + const forged = new Map([['admin_device', createAdminSession()]]); + expect(await login(forged, 'right-password', '203.0.113.9')).toBe(429); + }); +}); diff --git a/lib/admin/rate-limit.ts b/lib/admin/rate-limit.ts index c4b13930a..57dcb5065 100644 --- a/lib/admin/rate-limit.ts +++ b/lib/admin/rate-limit.ts @@ -1,6 +1,8 @@ /** * In-memory rate limiter for admin login. - * Max 5 attempts per IP per 15 minutes, and 50 across all clients. + * Max 5 attempts per IP per 15 minutes, and 50 across all clients. A browser + * with the admin device cookie (lib/admin/session.ts) gets 5 per device + * instead and stays out of the shared ceiling, which anyone can use up. */ const MAX_ATTEMPTS = 5; @@ -22,14 +24,17 @@ interface RateLimitEntry { } const attempts = new Map(); +const deviceAttempts = new Map(); let globalAttempts: RateLimitEntry | null = null; // Clean up expired entries periodically setInterval(() => { const now = Date.now(); - for (const [key, entry] of attempts) { - if (entry.resetAt <= now) { - attempts.delete(key); + for (const map of [attempts, deviceAttempts]) { + for (const [key, entry] of map) { + if (entry.resetAt <= now) { + map.delete(key); + } } } }, 60_000).unref(); @@ -64,3 +69,25 @@ export function checkRateLimit(ip: string): { allowed: boolean; remaining: numbe globalAttempts.count++; return { allowed: true, remaining: MAX_ATTEMPTS - entry.count, retryAfterMs: 0 }; } + +/** + * Check a browser with a valid admin device cookie, by its device id: 5 + * attempts per 15 minutes, not counted against or stopped by the shared + * ceiling. + */ +export function checkDeviceRateLimit(deviceId: string): { allowed: boolean; remaining: number; retryAfterMs: number } { + const now = Date.now(); + const entry = deviceAttempts.get(deviceId); + + if (!entry || entry.resetAt <= now) { + deviceAttempts.set(deviceId, { count: 1, resetAt: now + WINDOW_MS }); + return { allowed: true, remaining: MAX_ATTEMPTS - 1, retryAfterMs: 0 }; + } + + if (entry.count >= MAX_ATTEMPTS) { + return { allowed: false, remaining: 0, retryAfterMs: entry.resetAt - now }; + } + + entry.count++; + return { allowed: true, remaining: MAX_ATTEMPTS - entry.count, retryAfterMs: 0 }; +} diff --git a/lib/admin/session.ts b/lib/admin/session.ts index 29f9c64c7..ff690bef3 100644 --- a/lib/admin/session.ts +++ b/lib/admin/session.ts @@ -40,24 +40,12 @@ function getSessionTTL(): number { return isNaN(ttl) || ttl <= 0 ? DEFAULT_ADMIN_SESSION_TTL : ttl; } -/** - * Create an encrypted admin session token, in session generation - * `generation` (see getAdminSessionGeneration). - */ -export function createAdminSession(generation = 0): string { +/** Encrypt `payload` under the session key, bound to `aad`. */ +function sealToken(payload: object, aad: Buffer): string { const key = getKey(); const iv = randomBytes(IV_LENGTH); const cipher = createCipheriv(ALGORITHM, key, iv); - cipher.setAAD(ADMIN_SESSION_AAD); - - const now = Math.floor(Date.now() / 1000); - const payload: AdminSessionPayload = { - role: 'admin', - iat: now, - exp: now + getSessionTTL(), - sid: randomBytes(16).toString('base64url'), - gen: generation, - }; + cipher.setAAD(aad); const json = JSON.stringify(payload); const encrypted = Buffer.concat([cipher.update(json, 'utf8'), cipher.final()]); @@ -66,27 +54,48 @@ export function createAdminSession(generation = 0): string { return Buffer.concat([iv, tag, encrypted]).toString('base64'); } +/** Decrypt a token sealToken made with the same `aad`. Throws when it doesn't open. */ +function openToken(token: string, aad: Buffer): unknown { + const key = getKey(); + const data = Buffer.from(token, 'base64'); + if (data.length < IV_LENGTH + TAG_LENGTH) throw new Error('Token too short'); + + const iv = data.subarray(0, IV_LENGTH); + const tag = data.subarray(IV_LENGTH, IV_LENGTH + TAG_LENGTH); + const encrypted = data.subarray(IV_LENGTH + TAG_LENGTH); + + const decipher = createDecipheriv(ALGORITHM, key, iv); + decipher.setAAD(aad); + decipher.setAuthTag(tag); + + const decrypted = Buffer.concat([decipher.update(encrypted), decipher.final()]); + return JSON.parse(decrypted.toString('utf8')); +} + +/** + * Create an encrypted admin session token, in session generation + * `generation` (see getAdminSessionGeneration). + */ +export function createAdminSession(generation = 0): string { + const now = Math.floor(Date.now() / 1000); + const payload: AdminSessionPayload = { + role: 'admin', + iat: now, + exp: now + getSessionTTL(), + sid: randomBytes(16).toString('base64url'), + gen: generation, + }; + return sealToken(payload, ADMIN_SESSION_AAD); +} + /** * Verify and decode an admin session token. Returns null if invalid or expired. */ export function verifyAdminSession(token: string): AdminSessionPayload | null { try { - const key = getKey(); - const data = Buffer.from(token, 'base64'); - if (data.length < IV_LENGTH + TAG_LENGTH) return null; + const payload = openToken(token, ADMIN_SESSION_AAD) as AdminSessionPayload; - const iv = data.subarray(0, IV_LENGTH); - const tag = data.subarray(IV_LENGTH, IV_LENGTH + TAG_LENGTH); - const encrypted = data.subarray(IV_LENGTH + TAG_LENGTH); - - const decipher = createDecipheriv(ALGORITHM, key, iv); - decipher.setAAD(ADMIN_SESSION_AAD); - decipher.setAuthTag(tag); - - const decrypted = Buffer.concat([decipher.update(encrypted), decipher.final()]); - const payload = JSON.parse(decrypted.toString('utf8')) as AdminSessionPayload; - - if (payload.role !== 'admin') return null; + if (!payload || payload.role !== 'admin') return null; // Every token this version issues has both; ADMIN_SESSION_AAD already // signed the ones from before out. if (typeof payload.sid !== 'string' || typeof payload.gen !== 'number') return null; @@ -167,6 +176,64 @@ export async function setAdminSessionCookie(request?: Request): Promise { }); } +/** + * A browser that has signed in to the admin dashboard before (OWASP's + * "device cookie"). Password attempts from it are limited per device instead + * of by the budget every other client shares (lib/admin/rate-limit.ts), so + * wrong passwords sent from anywhere else can't lock an admin's own browser + * out. It grants nothing else. Sealed like the session token, under its own + * associated data, so neither passes for the other. + */ +export const ADMIN_DEVICE_COOKIE = 'admin_device'; +const ADMIN_DEVICE_AAD = Buffer.from('bulwark-admin-device', 'utf8'); +const ADMIN_DEVICE_TTL = 180 * 24 * 60 * 60; + +interface AdminDevicePayload { + typ: 'admin-device'; + id: string; + exp: number; +} + +export function createAdminDeviceToken(): string { + const payload: AdminDevicePayload = { + typ: 'admin-device', + id: randomBytes(16).toString('base64url'), + exp: Math.floor(Date.now() / 1000) + ADMIN_DEVICE_TTL, + }; + return sealToken(payload, ADMIN_DEVICE_AAD); +} + +/** The device id in an admin device token, or null if it's invalid or expired. */ +export function verifyAdminDeviceToken(token: string): string | null { + try { + const payload = openToken(token, ADMIN_DEVICE_AAD) as AdminDevicePayload; + if (!payload || payload.typ !== 'admin-device' || typeof payload.id !== 'string' || !payload.id) return null; + if (typeof payload.exp !== 'number' || payload.exp < Math.floor(Date.now() / 1000)) return null; + return payload.id; + } catch { + return null; + } +} + +/** The id of this browser's admin device cookie, if it has a valid one. */ +export async function getAdminDeviceId(): Promise { + const cookieStore = await cookies(); + const token = cookieStore.get(ADMIN_DEVICE_COOKIE)?.value; + return token ? verifyAdminDeviceToken(token) : null; +} + +/** Mark this browser as one an admin signed in from. Kept across sign-outs. */ +export async function setAdminDeviceCookie(request?: Request): Promise { + const cookieStore = await cookies(); + cookieStore.set(ADMIN_DEVICE_COOKIE, createAdminDeviceToken(), { + httpOnly: true, + secure: secureCookieFlag(request), + sameSite: 'strict', + path: '/', + maxAge: ADMIN_DEVICE_TTL, + }); +} + /** * Clear the admin session cookie and sign its token out, so a copy of the * cookie stops working too.