DOMPurify URI-tests srcset as a single string, so an allowed first
candidate waved every later one through - an SVG data: URI could ride
in behind a raster candidate on <img srcset> and <picture><source>.
Check every data: occurrence against the raster allowlist and drop the
whole attribute on one disallowed hit, including the external blocker's
data-blocked-srcset stash, which shares the hook pass with no ordering
guarantee. The token is captured through the metadata-ending comma so
the plain (non-base64) raster form keeps the same verdict as in src.
next-intl has no "../" relative-namespace escape. A key like
t('../context_menu.not_spam') is not resolved relative to anything -
it is concatenated literally into the lookup key and always misses,
throwing MISSING_MESSAGE. email-list.tsx, about-data-settings.tsx,
and account-settings.tsx all used this pattern to reach keys outside
their own translation namespace.
Fixed by calling useTranslations() again for each additional
namespace needed, which is the supported way to reach sibling
namespaces, instead of trying to walk out of the current one.
The translations test already verifies that every key referenced in
source resolves in the en locale, but its extractor skipped any key
starting with a dot, which is exactly the shape of the broken calls,
so this class of bug never failed CI. Since no legitimate key can
start with a dot, the exclusion is removed. With it gone the test
reports all thirteen call sites fixed here when run against the old
code, and passes with the fix, so a reintroduced relative path fails
CI with the offending file listed.
contactUsing(), calendarUsing(), and fileUsing() checked the base
"principals" capability but then declared the separate
"principals:owner" sub-capability (RFC 9670) in the request's "using"
array. RFC 8620 section 3.3 requires a server to reject the whole
request if it declares a capability the server does not advertise, so
a server that supports base principals without the owner
sub-capability rejects every one of these requests with
unknownCapability.
Fixed to check the specific sub-capability being declared instead of
assuming it from the broader one. The calendar agenda route had the
same mismatch and is fixed the same way. Note that RFC 9670 section
1.5.2 places principals:owner in accountCapabilities rather than the
session capabilities, so on compliant servers the sub-capability is
simply never declared, which is the intent. Nothing is lost by
omitting it.
The file share test asserted the old behavior by expecting the owner
capability to be declared against a server that advertises only base
principals. It now asserts the capability is omitted in that case and
declared only when the server advertises it. The same pair of
assertions is added for the contacts, calendars, and agenda route
request builders, so each site this commit touches is covered against
regression.
RFC 8620 does not require Session object URLs to be absolute, but
rewriteSessionUrl() called new URL(url) with no base, which throws on a
relative apiUrl or downloadUrl. The throw was silently caught and the
raw relative string returned, so requests resolved against the app's
own origin instead of the JMAP server.
Resolving with the URL parser and reading back href or pathname is not
an option: downloadUrl, uploadUrl and eventSourceUrl are RFC 6570
level 1 URI Templates (RFC 8620 section 2) whose literal {accountId},
{blobId}, {name} and {type} placeholders would be percent-encoded to
%7B/%7D, silently defeating the string replace() expansion at every
call site. Instead, anchor the path, query and fragment of the original
string at the server origin, which also covers network-path references
(//host/path) and does not fold a path prefix on the configured server
URL into relative resolution.
An empty URL is returned untouched: rewriting it into the bare server
origin would make it truthy and defeat the "not available" guards on
apiUrl and downloadUrl downstream.
With multiple accounts and settings sync on, the composer's default sender
would switch depending on login order and get disturbed by logging a third
account off. Cause: the per-account maps preferredIdentityIds (#507) and
allMailFolderIds live in EVERY account's synced settings blob, and
loadFromServer(username, serverUrl) runs once per connected account calling
importSettings(), which REPLACED the whole map with that account's server copy.
So the last account to load clobbered the map (last-writer-wins by login
order), and a stale blob from a since-removed account could win too.
Fix: importSettings takes an optional { serverAccountId }; loadFromServer passes
generateAccountId(username, serverUrl). For the per-account maps, a server load
now merges only that account's OWN entry into the existing map instead of
replacing it - each account's server is authoritative only for itself, so other
accounts' entries survive. File imports (no serverAccountId) still replace
wholesale.
+ regression tests: a per-account load preserves other accounts' entries,
only trusts the loading account, file import still replaces, same for
allMailFolderIds. tsc + eslint clean.
Companion to the calendar spec for the cross-account id-namespacing fix. Logs
in two accounts, enables the Pro shell, and asserts both accounts' address books
stay aggregated (same set, no loss/duplication/corruption) after switching the
active account. Contacts keep no persisted id-selection, so this verifies the
aggregated list stays correct/usable rather than a selection reset.
Adds a data-testid on address-book rows (address-book-item + data-book-name/
-account) and openContacts / addressBookRows helpers.
Docker/Playwright regression for the cross-account id-namespacing fix: log in
two accounts, enable the Pro shell, deselect a calendar, switch the active
account, and assert the deselection survives (it reset before the fix because
the previously-active account's ids flipped raw<->namespaced on the switch).
Adds the minimal test hooks: data-testid on calendar sidebar rows
(calendar-item + data-calendar-name/-visible), the settings tab buttons
(settings-tab-<id>), and a testId passthrough on ToggleSwitch used for the Pro
interface toggle. New app helpers: openCalendar (in-app nav, since a hard goto
to a deep link bounces to login), calendarRows, enableProInterface (toggles via
the settings UI - the app doesn't survive a hard reload and seeding proInterface
before login destabilises the add-account popover).
Verified green in the dockerized Stalwart harness.
Calendar/contact aggregation left the ACTIVE account's ids raw while
namespacing background accounts (<localAccountId>::id). So an entity's id
depended on which account was active; switching flipped the id form and
invalidated persisted references (selectedCalendarIds), broke isSubscription
matching, and destabilised selection - the account-switch bug users saw in
Calendar and Address Book (mail was fine: it keys per account + stable
virtual ids).
Invariant now: an entity is namespaced iff it carries a localAccountId;
originalId always holds the raw JMAP id. Consumers/mutations already resolve
the raw id via `originalId || stripLocalAccountPrefix`, so they are unchanged.
- prefix* functions (calendar + contacts) always namespace + set originalId;
drop the isActiveAccount branch and the now-dead activeLocalAccountId params
(callers in the hooks + calendar-app/contacts-app updated).
- mapServerEventToStoreEvent namespaces the event id with the matched calendar's
localAccountId, so createEvent/updateEvent results are consistent.
- reconcileSelectedIds(): remap selectedCalendarIds by raw identity on every
fetch, so a raw->namespaced transition (switch, just-created calendar, or the
one-time migration from old persisted raw ids) keeps the selection instead of
resetting to "all".
- isSubscriptionCalendar(): namespace-robust and collision-safe - resolve the
calendar and match by raw id scoped to the owning JMAP accountId (Stalwart's
raw ids like "b"/"c" collide across accounts).
+9 unit tests (selection stability across switch/create, subscription
robustness, no cross-account false positives). tsc + eslint clean; 368 store
tests green.
The rights-first resolver already made EXISTING events in subscription /
read-only calendars read-only, but the create path still offered them:
event-modal's calendar dropdown listed every calendar. iCal subscriptions are
real Stalwart calendars (client-side feed written into a created calendar) with
writable myRights, so they appeared as selectable create destinations.
- canCreateEventsIn(calendar, isSubscriptionCalendar): writable AND not a
subscription. Exported from lib/calendar-editability.
- event-modal: the calendar picker now lists only creatable calendars (plus the
event's own calendar when editing), and new events never default into a
subscription / read-only calendar.
+4 unit tests. tsc + eslint clean; calendar-editability suite 30 green.
ALLOWED_URI_REGEXP excludes image/svg+xml on purpose - DOMPurify cannot
inspect the bytes inside a data: URI, so an SVG payload can carry
<script>/<foreignObject> the surrounding sanitizer never sees. That
exclusion never took effect on <img>, <video>, <audio>, <source> and
<track>: DOMPurify's attribute check short-circuits on DATA_URI_TAGS
before ALLOWED_URI_REGEXP is consulted, and that set can only be extended
via ADD_DATA_URI_TAGS, never trimmed. Any data: URI reached those tags,
including image/svg+xml and text/html; <a href> behaved as documented.
Not a live script vector - browsers render SVG-in-<img> in secure static
mode - but the sanitizer promised something it did not enforce, and the
print window renders sanitized bodies without the iframe sandbox and CSP
that otherwise back it up.
restrictDataUriResourcesOnNode re-applies the raster allowlist on those
tags, wired in through the existing afterSanitizeAttributes hooks. The
blocked-external-image placeholder becomes a 1x1 GIF: it is set in the
same hook pass, so an SVG placeholder would depend on hook order.
The three viewer call sites that hand-rolled
DOMPurify.sanitize(html, EMAIL_IFRAME_SANITIZE_CONFIG) now use
sanitizeEmailHtmlForIframe, which is the same thing and picks up the
guard automatically.
Event editability was inferred from identity (isOrigin + organizer match)
and never from the calendar's write rights. That opened events in read-only
shared / iCal-subscription calendars for editing, and once the address list
was widened with account aliases any alias-organized event became editable -
even in calendars the user cannot write.
- lib/calendar-editability.ts: getEventEditability() -> editable | rsvp-only
| read-only, gating on the calendar's myRights (mayWriteAll / mayWriteOwn /
mayRSVP) first, then organizer/participant identity. Subscriptions are
read-only. Aliases now only refine the write-own / RSVP branches, so they
can no longer over-open events (Stalwart: organizer identity does not
confer calendar write; the ACL does).
- collectUserCalendarAddresses(): merge identities + account aliases so an
alias organizer is recognised as the user's own; calendar-app fetches the
principal aliases and passes isSubscriptionCalendar through.
- event-detail-popover / event-modal: derive Edit/Delete/RSVP/read-only from
the resolver (three render states); read-only is new - no edit, no RSVP.
Tests: calendar-editability (26 cases over rights x role x subscription x
origin, incl. the alias-in-read-only regression) + calendar-participants.
tsc + eslint clean.
`npm run typecheck` failed with 8 TS2307 errors from Next's generated
route validators referencing old page paths (e.g. calendar/page.js) after
routes moved to catch-all segments. Two causes:
- The production validator (.next/types) was stale. Prepend `next typegen`
so `tsc` always runs against freshly generated route types.
- The dev validator (.next/dev/types) is a turbopack-dev-only artifact that
is absent from clean/CI checkouts and goes stale locally. `next typegen`
force-re-adds it to tsconfig `include`, so exclude it instead (exclude
overrides include), leaving route typing to the maintained .next/types.
tsc still catches real type errors; verified `npm run typecheck` is clean
and idempotent.
Both 04-all-mail and 04-shared-identity shared the same 04 prefix. Shift
shared-identity and everything after it up by one (05-shared-identity …
12-cross-view-retain), leaving 04-all-mail as the sole 04 spec. Pure file
renames; no content changes.
Adds 11-cross-view-retain.spec.ts: with the Unread cross view open, reading a
mail keeps it in the list (status + unread counter update only, no removal);
with the Starred cross view open, un-starring keeps it. Both force the
focus/push refresh that previously dropped the row and assert it survives.
Supporting test hooks:
- thread-list-item: expose data-starred on the row.
- email-context-menu: testId ctx-star / ctx-unstar on the star item.
- integration helpers: expectEmailStarred (app.ts), setFlagged (jmap.ts).
- integration policy.json: enable crossStarredViewEnabled.
Verified: 08-shared-moves-style run of 11-cross-view-retain → 2/2 passed
against the dockerized stack.
In the unread ('unread') and starred ('starred') cross views, marking a mail
read or removing its star flips the very keyword the view filters on
(notKeyword:$seen / hasKeyword:$flagged). The push-triggered refresh re-runs
that filtered query, so the mail was absent from the fresh page and the merge
dropped it - the row vanished from under the cursor on the user's own action.
Keep such rows in place: only their status + the counters update (both already
applied by markAsRead/toggleStar/batchMarkAsRead), they stay visible until the
view is re-opened. A new retainedInViewIds set records the ids the user just
read/unstarred while in these views; refreshCurrentMailbox re-inserts those rows
at their prior position via mergeRetainedRows. The set is scoped to the two
keyword cross views, cleared on navigation (fetchEmails/fetchUnifiedEmails/
fetchCrossView/exitUnifiedView), and dropped per-id when the action is reversed
(mark unread / re-star). totalEmails counts the retained rows so hasMore stays
correct.
Genuinely removed rows (deleted/moved -> absent from the local list) are not
resurrected, and a change the user did NOT make in this view (e.g. read on
another device, retain set empty) still drops as before.
Tests: stores/__tests__/email-store-retained-in-view.test.ts - read keeps the
row (unread view), unstar keeps the row (starred view), reversing/ navigation
clear retention, external read still drops, 'all' view does not retain. The
row-keeping cases fail without the fix.
refreshCurrentMailbox runs on every Email state push (delete, star, and
mark-read all raise one). It only handled real single mailboxes: in a
unified ("All Mail") or cross-account view, selectedMailbox is a virtual
id (e.g. __cross_all__ / __unified_inbox__) that no folder matches, so it
was sent to getEmails as an inMailbox filter, returned an empty page, and
the first-page merge wiped the whole aggregated list. The empty query was
previously masked by the length-based merge cutoff; af2b00dd (#592)
changed the cutoff to the page size and unmasked it.
Make refreshCurrentMailbox view-aware, mirroring loadMoreEmails /
performSearch: fan out page 0 via fetchCrossViewEmails / fetchUnifiedEmails
(search-aware) when in those views, then run the existing merge so loaded
pages are preserved. Surface cross-view fetch errors via unifiedErrors.
Test: email-store-refresh-unified asserts a push refresh in the cross-All
and unified-inbox views keeps the list and never queries the virtual id
(fails without the fix: the list wipes to []).
Linus Rath
Ctaoist
dealerweb
Lucas Gaitzsch
zisisvakras
Stefan Hildebrandt
BGandCopilot Autofix powered by AI
GitHub Copilot
Paulhenry Saux
Linus Rath