From d89e33110ab5d18961e35d05b4896eda8845cc4b Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Thu, 24 Sep 2026 18:35:50 +0200 Subject: [PATCH] fix: load remote content only for senders in the trusted senders book With "Sync trusted senders to address book" on, the viewer trusted every address in every address book, so remote images and tracking pixels loaded for any contact, including mail with a spoofed From that uses a contact's address. The toggle also switched itself on the first time Content & Senders was opened on a contacts-capable account, so merely visiting settings widened trust to the whole address book. Now only the local allow-list and, while sync is on, the dedicated "Trusted Senders" book count, as in the webmail viewer. The setting stays off until the user turns it on, and the viewer only fetches the book while sync is on. --- src/components/EmailBodyView.tsx | 29 ++++--------- .../settings/ContentSendersSettings.tsx | 11 +---- src/lib/__tests__/trusted-senders.test.ts | 41 +++++++++++++++++++ src/lib/trusted-senders.ts | 26 ++++++++++++ src/stores/settings-store.ts | 4 +- 5 files changed, 79 insertions(+), 32 deletions(-) create mode 100644 src/lib/__tests__/trusted-senders.test.ts create mode 100644 src/lib/trusted-senders.ts diff --git a/src/components/EmailBodyView.tsx b/src/components/EmailBodyView.tsx index 87c93405..e6d0ab34 100644 --- a/src/components/EmailBodyView.tsx +++ b/src/components/EmailBodyView.tsx @@ -12,6 +12,7 @@ import { hasTruncatedDisplayedBody, pickEmailBody, selectRenderableHtml } from ' import { buildQuoteCollapseScript, collapsePlainTextQuotes } from '../lib/quote-collapse'; import { parseMailtoUrl } from '../lib/unsubscribe'; import { fetchInlineImageDataUri } from '../lib/email-export'; +import { isSenderContentTrusted } from '../lib/trusted-senders'; import { useSettingsStore } from '../stores/settings-store'; import { useContactsStore } from '../stores/contacts-store'; import { useLocaleStore } from '../stores/locale-store'; @@ -522,7 +523,6 @@ export default function EmailBodyView({ const emailAlwaysLightMode = useSettingsStore((s) => s.emailAlwaysLightMode); const plainTextFont = useSettingsStore((s) => s.plainTextFont); const messageSpacing = useSettingsStore((s) => s.messageSpacing); - const contacts = useContactsStore((s) => s.contacts); const trustedSenderEmails = useContactsStore((s) => s.trustedSenderEmails); const trustedSendersLoaded = useContactsStore((s) => s.trustedSendersLoaded); const loadTrustedSendersBook = useContactsStore((s) => s.loadTrustedSendersBook); @@ -533,21 +533,8 @@ export default function EmailBodyView({ // entries (synced across devices) feed the external-content trust check. We // do not create the book here — it is created lazily when a sender is trusted. React.useEffect(() => { - if (!trustedSendersLoaded) void loadTrustedSendersBook(false); - }, [trustedSendersLoaded, loadTrustedSendersBook]); - // Index the address book once per render so the trusted-sender check below - // is O(1) per email. Only used when the matching setting is on. - const addressBookEmails = React.useMemo(() => { - if (!trustedSendersAddressBook) return null; - const out = new Set(); - for (const card of contacts) { - if (!card.emails) continue; - for (const e of Object.values(card.emails)) { - if (e?.address) out.add(e.address.toLowerCase()); - } - } - return out; - }, [contacts, trustedSendersAddressBook]); + if (trustedSendersAddressBook && !trustedSendersLoaded) void loadTrustedSendersBook(false); + }, [trustedSendersAddressBook, trustedSendersLoaded, loadTrustedSendersBook]); // Most marketing email is authored against a white background, so dark-mode // inversion can wreck logos/banners. With this flag the user opts to render // emails on a light surface even while the rest of the app is dark; the @@ -565,11 +552,11 @@ export default function EmailBodyView({ }, [email, bodyOverride]); const rawHtml = React.useMemo(() => selectRenderableHtml(picked), [picked]); const text = picked.text; - const trusted = senderEmail - ? isSenderTrusted(senderEmail) - || trustedSenderEmails.includes(senderEmail.toLowerCase()) - || (addressBookEmails?.has(senderEmail.toLowerCase()) ?? false) - : false; + const trusted = isSenderContentTrusted(senderEmail, { + isLocallyTrusted: isSenderTrusted, + syncEnabled: trustedSendersAddressBook, + trustedBookEmails: trustedSenderEmails, + }); // One-time override: user tapped "Load images" for this email only. const [allowOnce, setAllowOnce] = React.useState(false); diff --git a/src/components/settings/ContentSendersSettings.tsx b/src/components/settings/ContentSendersSettings.tsx index ec264257..a356a980 100644 --- a/src/components/settings/ContentSendersSettings.tsx +++ b/src/components/settings/ContentSendersSettings.tsx @@ -47,15 +47,8 @@ export function ContentSendersSettings() { useEffect(() => { if (!hydrated) void hydrate(); }, [hydrated, hydrate]); - // Sync to the address book once the account proves it supports contacts; - // the toggle below lets the user opt out again. Mirrors the webmail, where - // the setting starts unset (null) and flips on with the first - // contacts-capable session. - useEffect(() => { - if (!hydrated || !hasContacts) return; - if (trustedSendersAddressBook === null) updateSetting('trustedSendersAddressBook', true); - }, [hydrated, hasContacts, trustedSendersAddressBook, updateSetting]); - + // Sync stays off until the user turns it on below (null = never chosen): + // opening this pane must not change which senders the viewer trusts. const syncEnabled = !!trustedSendersAddressBook && hasContacts; useEffect(() => { diff --git a/src/lib/__tests__/trusted-senders.test.ts b/src/lib/__tests__/trusted-senders.test.ts new file mode 100644 index 00000000..4e5d0126 --- /dev/null +++ b/src/lib/__tests__/trusted-senders.test.ts @@ -0,0 +1,41 @@ +import { describe, it, expect } from 'vitest'; +import { isSenderContentTrusted } from '../trusted-senders'; + +const local = (list: string[]) => (email: string) => list.includes(email.toLowerCase()); + +describe('isSenderContentTrusted', () => { + it('trusts senders on the local allow-list whatever the sync setting', () => { + for (const syncEnabled of [null, false, true]) { + expect(isSenderContentTrusted('Alice@Example.com', { + isLocallyTrusted: local(['alice@example.com']), + syncEnabled, + trustedBookEmails: [], + })).toBe(true); + } + }); + + it('trusts the Trusted Senders book only while sync is on', () => { + const opts = { isLocallyTrusted: local([]), trustedBookEmails: ['bob@example.com'] }; + expect(isSenderContentTrusted('Bob@Example.com', { ...opts, syncEnabled: true })).toBe(true); + expect(isSenderContentTrusted('bob@example.com', { ...opts, syncEnabled: false })).toBe(false); + // Never chosen counts as off: the default must not widen trust. + expect(isSenderContentTrusted('bob@example.com', { ...opts, syncEnabled: null })).toBe(false); + }); + + it('does not trust a sender just because some other list knows them', () => { + // An ordinary contact (e.g. in the personal book) is not in either list, + // so a spoofed From with their address still gets remote content blocked. + expect(isSenderContentTrusted('carol@example.com', { + isLocallyTrusted: local(['alice@example.com']), + syncEnabled: true, + trustedBookEmails: ['bob@example.com'], + })).toBe(false); + }); + + it('never trusts a missing sender', () => { + const opts = { isLocallyTrusted: () => true, syncEnabled: true, trustedBookEmails: [''] }; + expect(isSenderContentTrusted(undefined, opts)).toBe(false); + expect(isSenderContentTrusted(null, opts)).toBe(false); + expect(isSenderContentTrusted(' ', opts)).toBe(false); + }); +}); diff --git a/src/lib/trusted-senders.ts b/src/lib/trusted-senders.ts new file mode 100644 index 00000000..3b5031f0 --- /dev/null +++ b/src/lib/trusted-senders.ts @@ -0,0 +1,26 @@ +/** + * Whether the viewer may load a sender's remote content (images, tracking + * pixels) without asking. + * + * Only two lists count: the local allow-list and, while "Sync trusted senders + * to address book" is on, the dedicated "Trusted Senders" address book. An + * ordinary contact is never trusted just for being in the address book: the + * From header is trivially spoofed, so that would let anyone who knows a + * contact's address load pixels (webmail `components/email/email-viewer.tsx`). + */ +export function isSenderContentTrusted( + senderEmail: string | null | undefined, + opts: { + /** The settings store's local allow-list check. */ + isLocallyTrusted: (email: string) => boolean; + /** The "Sync trusted senders to address book" setting (null = never chosen, off). */ + syncEnabled: boolean | null; + /** Lowercased addresses filed in the "Trusted Senders" book. */ + trustedBookEmails: readonly string[]; + }, +): boolean { + const email = senderEmail?.trim(); + if (!email) return false; + if (opts.isLocallyTrusted(email)) return true; + return !!opts.syncEnabled && opts.trustedBookEmails.includes(email.toLowerCase()); +} diff --git a/src/stores/settings-store.ts b/src/stores/settings-store.ts index 0bb3f829..9fc4b744 100644 --- a/src/stores/settings-store.ts +++ b/src/stores/settings-store.ts @@ -123,8 +123,8 @@ interface PersistedSettings { // Privacy & content externalContentPolicy: ExternalContentPolicy; trustedSenders: string[]; - // null = not decided yet; flips to true automatically once the account - // proves it supports JMAP contacts (webmail parity), false = user opted out. + // null = not decided yet and treated as off; only the user's toggle in + // Content & Senders turns it on, false = user opted out. trustedSendersAddressBook: boolean | null; senderFavicons: boolean; hideInlineImageAttachments: boolean;