From ddb596affc46c749a8c6073f64ce666a4b42c169 Mon Sep 17 00:00:00 2001 From: Stefan Hildebrandt <695494+hildebrandttk@users.noreply.github.com> Date: Wed, 17 Jun 2026 08:37:22 +0200 Subject: [PATCH 1/2] fix: isolate per-account state snapshots from leakage and mutation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit account-state-manager had two latent correctness issues: 1. Shared references: snapshotAccount stored the live store arrays/objects directly, so a later in-place mutation (array push/splice, or a shared email object being stamped) retroactively corrupted an earlier snapshot. Now copies the captured collections. 2. Incomplete restore: the snapshot only captures a subset of each store's fields, but restoreAccount applied it with a merge, leaving every other field (email selection, loading flags, tag counts, …) at the previously active account's values. It only worked because every caller happened to call clearAllStores() first. restoreAccount now resets the stores to baseline itself before layering the snapshot back on, so it is correct standalone and can't leak state across accounts. Adds tests pinning the isolation guarantees. --- lib/__tests__/account-state-manager.test.ts | 12 +++---- lib/account-state-manager.ts | 37 +++++++++++++++------ 2 files changed, 32 insertions(+), 17 deletions(-) diff --git a/lib/__tests__/account-state-manager.test.ts b/lib/__tests__/account-state-manager.test.ts index 658fe2c5..7f61265f 100644 --- a/lib/__tests__/account-state-manager.test.ts +++ b/lib/__tests__/account-state-manager.test.ts @@ -58,7 +58,7 @@ describe('snapshotAccount / restoreAccount', () => { expect(useVacationStore.getState().isEnabled).toBe(true); }); - it('CHARACTERISATION: only snapshotted fields are restored; others survive', () => { + it('resets fields outside the snapshot subset to their defaults (no cross-account leak)', () => { // isLoading is NOT part of the email snapshot subset. useEmailStore.setState({ selectedMailbox: 'a-in', isLoading: false }); snapshotAccount('A'); @@ -66,11 +66,11 @@ describe('snapshotAccount / restoreAccount', () => { restoreAccount('A'); - expect(useEmailStore.getState().selectedMailbox).toBe('a-in'); // restored - expect(useEmailStore.getState().isLoading).toBe(true); // NOT restored (merge) + expect(useEmailStore.getState().selectedMailbox).toBe('a-in'); // captured → restored + expect(useEmailStore.getState().isLoading).toBe(false); // uncaptured → reset, not leaked }); - it('CHARACTERISATION: snapshot stores array references, not deep clones', () => { + it('decouples the snapshot from later in-place mutation of the source array', () => { const arr = [makeEmail({ id: '1' })]; useEmailStore.setState({ emails: arr }); snapshotAccount('A'); @@ -78,8 +78,8 @@ describe('snapshotAccount / restoreAccount', () => { useEmailStore.setState({ emails: [] }); restoreAccount('A'); - // The post-snapshot mutation leaked into the snapshot. - expect(useEmailStore.getState().emails.map((e) => e.id)).toEqual(['1', '2']); + // The post-snapshot mutation did NOT leak into the snapshot. + expect(useEmailStore.getState().emails.map((e) => e.id)).toEqual(['1']); }); it('returns false and leaves stores untouched for an unknown account', () => { diff --git a/lib/account-state-manager.ts b/lib/account-state-manager.ts index 7fba8a3d..10dc9fe8 100644 --- a/lib/account-state-manager.ts +++ b/lib/account-state-manager.ts @@ -37,33 +37,37 @@ export function snapshotAccount(accountId: string): void { const identityState = useIdentityStore.getState(); const vacationState = useVacationStore.getState(); + // Copy the captured collections so the snapshot is decoupled from the live + // store: a later in-place mutation (e.g. an array push/splice, or stamping + // fields onto a shared email object) must not retroactively corrupt a + // snapshot taken earlier. cache.set(accountId, { email: { - emails: emailState.emails, - mailboxes: emailState.mailboxes, + emails: [...emailState.emails], + mailboxes: [...emailState.mailboxes], selectedEmail: emailState.selectedEmail, selectedMailbox: emailState.selectedMailbox, searchQuery: emailState.searchQuery, - quota: emailState.quota, + quota: emailState.quota ? { ...emailState.quota } : emailState.quota, }, contact: { - contacts: contactState.contacts, - addressBooks: contactState.addressBooks, + contacts: [...contactState.contacts], + addressBooks: [...contactState.addressBooks], supportsSync: contactState.supportsSync, }, calendar: { - calendars: calendarState.calendars, - events: calendarState.events, - selectedCalendarIds: calendarState.selectedCalendarIds, + calendars: [...calendarState.calendars], + events: [...calendarState.events], + selectedCalendarIds: [...calendarState.selectedCalendarIds], viewMode: calendarState.viewMode, supportsCalendar: calendarState.supportsCalendar, }, filter: { - rules: filterState.rules, + rules: [...filterState.rules], isSupported: filterState.isSupported, }, identity: { - identities: identityState.identities, + identities: [...identityState.identities], preferredPrimaryId: identityState.preferredPrimaryId, }, vacation: { @@ -73,11 +77,22 @@ export function snapshotAccount(accountId: string): void { }); } -/** Restore cached store states for the given account. Returns false if no cache exists. */ +/** + * Restore cached store states for the given account. Returns false if no cache + * exists. + * + * The snapshot only captures a subset of each store's fields (the loaded data), + * so we reset every store to its baseline first. Without this, fields outside + * the captured subset (e.g. email selection, loading flags, tag counts) would + * carry over from whatever account was active, leaking state across accounts. + * `setState` merges, so the captured fields are then layered back on top. + */ export function restoreAccount(accountId: string): boolean { const snapshot = cache.get(accountId); if (!snapshot) return false; + clearAllStores(); + useEmailStore.setState(snapshot.email); useContactStore.setState(snapshot.contact); useCalendarStore.setState(snapshot.calendar); From 5306f7c548658485ef2a4a22426e7e8d96b67a7c Mon Sep 17 00:00:00 2001 From: Stefan Hildebrandt <695494+hildebrandttk@users.noreply.github.com> Date: Wed, 17 Jun 2026 09:45:12 +0200 Subject: [PATCH 2/2] fix: cap filename tokens at the full 200-char limit, not 80 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit renderRaw (and the attachment-template renderer) sanitised each {token} with sanitizePart's default 80-char cap, so a single long token such as {subject} was truncated to 80 — well before the documented 200-char filename limit, which was therefore unreachable per token. Introduce a FILENAME_MAX_LEN (200) constant and use it for the per-token cap so the overall limit governs. Adds tests. --- lib/__tests__/download-filename.test.ts | 8 ++++---- lib/download-filename.ts | 21 +++++++++++++-------- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/lib/__tests__/download-filename.test.ts b/lib/__tests__/download-filename.test.ts index 455483c9..7302713f 100644 --- a/lib/__tests__/download-filename.test.ts +++ b/lib/__tests__/download-filename.test.ts @@ -41,11 +41,11 @@ describe('emailExportFilename', () => { expect(emailExportFilename(makeEmail({}), '{unknown_token}')).toBe('email.eml'); }); - it('CHARACTERISATION: per-token sanitise caps each value at 80 chars', () => { - // sanitizePart defaults to maxLen=80, applied per {token} during render — - // so a single long {subject} is truncated to 80 well before the 200 cap. + it('lets a single long token reach the 200-char filename cap', () => { + // Each {token} is now capped at FILENAME_MAX_LEN (200), so the overall + // filename limit governs instead of an earlier 80-char per-token cap. const out = emailExportFilename(makeEmail({ subject: 'a'.repeat(300) }), '{subject}'); - expect(out).toBe('a'.repeat(80) + '.eml'); + expect(out).toBe('a'.repeat(200) + '.eml'); }); }); diff --git a/lib/download-filename.ts b/lib/download-filename.ts index cbf4cad1..11dcc00e 100644 --- a/lib/download-filename.ts +++ b/lib/download-filename.ts @@ -64,6 +64,11 @@ export const BUNDLE_TOKENS: { token: string; description: string }[] = [ { token: "day", description: "2-digit day" }, ]; +// Overall cap for a generated filename stem. Also used as the per-token cap so +// a single long token (e.g. {subject}) isn't truncated earlier than the final +// filename would be. +const FILENAME_MAX_LEN = 200; + function sanitizePart(input: string, maxLen = 80): string { const cleaned = input .replace(SAFE_CHARS, "_") @@ -173,7 +178,7 @@ function renderRaw(template: string, vars: Record): string { return template.replace(/\{(\w+)\}/g, (_, key: string) => { const value = vars[key]; if (value === undefined) return ""; - return sanitizePart(value); + return sanitizePart(value, FILENAME_MAX_LEN); }); } @@ -184,9 +189,9 @@ export function emailExportFilename( const opts = typeof options === "string" ? { template: options } : options; const template = opts.template ?? DEFAULT_EMAIL_TEMPLATE; const rendered = renderRaw(template, emailVars(email)); - const cleaned = sanitizePart(rendered, 200); + const cleaned = sanitizePart(rendered, FILENAME_MAX_LEN); const transformed = applyTransforms(cleaned, opts); - const stem = transformed.slice(0, 200) || "email"; + const stem = transformed.slice(0, FILENAME_MAX_LEN) || "email"; return `${stem}.eml`; } @@ -199,7 +204,7 @@ export function attachmentDownloadFilename( const template = opts.template ?? DEFAULT_ATTACHMENT_TEMPLATE; if (!email) { const filename = (attachment.name || "attachment").trim(); - const cleaned = sanitizePart(filename, 200) || "attachment"; + const cleaned = sanitizePart(filename, FILENAME_MAX_LEN) || "attachment"; return applyTransforms(cleaned, opts) || cleaned; } const vars = attachmentVars(email, attachment); @@ -208,10 +213,10 @@ export function attachmentDownloadFilename( if (value === undefined) return ""; // Preserve dots in {filename} so the original extension survives the // sanitiser (it strips trailing dots otherwise). - return key === "filename" ? value.replace(SAFE_CHARS, "_") : sanitizePart(value); + return key === "filename" ? value.replace(SAFE_CHARS, "_") : sanitizePart(value, FILENAME_MAX_LEN); }); const templateMentionsExt = /\{(ext|filename)\}/.test(template); - const cleaned = sanitizePart(rendered, 200) || "attachment"; + const cleaned = sanitizePart(rendered, FILENAME_MAX_LEN) || "attachment"; if (templateMentionsExt) { return applyTransforms(cleaned, opts) || cleaned; } @@ -235,9 +240,9 @@ export function bundleExportFilename( const opts = typeof options === "string" ? { template: options } : options; const template = opts.template ?? DEFAULT_BUNDLE_TEMPLATE; const rendered = renderRaw(template, bundleVars(count, iso)); - const cleaned = sanitizePart(rendered, 200); + const cleaned = sanitizePart(rendered, FILENAME_MAX_LEN); const transformed = applyTransforms(cleaned, opts); - const stem = transformed.slice(0, 200) || "emails"; + const stem = transformed.slice(0, FILENAME_MAX_LEN) || "emails"; return `${stem}.zip`; }