Extend the existing keyword write coverage to pin the three call sites
that clear a keyword. markAsRead, batchMarkAsRead and toggleStar must
patch with null and never with false, and must still send true when
setting the keyword.
The three removal cases fail against the previous boolean form, one per
call site. The three set cases pass either way, since that path was
already correct.
markAsRead, batchMarkAsRead and toggleStar cleared a keyword by patching
it to false, sending keywords/$seen: false or keywords/$flagged: false.
RFC 8620 section 5.3 removes a property from a PatchObject by setting it
to null, and RFC 8621 section 4.1.1 defines the keywords map as holding
true for every key, so false is not a valid way to clear one.
A server that enforces this rejects the update with invalidProperties.
The client does not inspect notUpdated, so marking a message unread or
removing a star looked like it had worked, the sidebar counts moved, and
the next refresh restored the old state from the server. Stalwart accepts
both forms, which is why this went unnoticed.
Send null instead, matching removeKeyword and the integration test
helpers, which already patch keywords this way.
The Files store was the only account-scoped store never wired into the
account lifecycle: account-state-manager clears email/contacts/calendar/
filter/identity/vacation on a switch, but not files. Because the Files
store is a global that outlives the FilesApp component, the previous
account's client + resources lingered in memory and resurfaced whenever
the drive was reopened after a switch - most visibly when the switch
happened on the mail view (FilesApp unmounted), the common normal-shell
flow that the FilesApp re-init effect alone does not cover.
Clear the Files store centrally in clearAllStores() alongside the other
account-scoped stores, so the reset happens on every switch path.
Tests:
- lib/__tests__/account-state-manager.test.ts: assert clearAllStores()
resets the Files drive (client, resources, pathStack, selection,
supportsFiles) with no cross-account leak.
- integration/tests/13-files-account-switch.spec.ts (dockerized Stalwart,
JMAP FileNode): seeds a distinct top-level folder in alice's and bob's
drives and asserts the on-screen drive swaps on a switch with no stale
folder - both switching off the Files view (mail -> switch -> open
Files) and on it (rail account switcher). Verified: passes with the fix,
fails (path 1) without it. New FileNode seeding helpers in helpers/jmap.ts;
openFiles/openMailView/driveEntry nav helpers in helpers/app.ts.
Editing an event's start now shifts the end by the same amount, preserving the
current duration (end - start). Moving the start forward never leaves the end
before it, and moving it either way never silently changes the length. Guards
against an already-negative-duration state.
Extracted a pure `shiftedEnd(oldStart, oldEnd, newStart)` for the start
date/time onChange handlers. +7 unit tests. tsc + eslint clean.
The keep-alive test parked one shared Response instance in
mockResolvedValue, but a Response body is single-use - as soon as more
than one request lands in the 30s window the second read dies with
"Body is unusable" and the ping reports a lost connection. Serve a
fresh Response per call via mockImplementation, the pattern the rest
of this file already uses.
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.
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.
Review follow-up to #751.
The offload path could not actually run: onBeforeBlobUpload goes through
HookBus.transform, which capped every handler at the 5s default. A handler
that pushes the file to an external store blows past that, and on timeout
the error is swallowed, the original file id is kept, and the composer
attaches the big file anyway - silently - while three such timeouts in a
minute auto-disable the plugin. Buses can now carry their own per-handler
budget and this one gets 120s. The same ceiling existed inside the sandbox,
where http.post/http.fetch were bounded by the 30s default API timeout;
those two are transfers rather than round-trips and now get 120s as well.
PRIVILEGED_ONLY_METHODS listed 'upfiles.set', which matches no dispatched
method - the dispatcher calls it 'upfiles.save'. So the write was never
tier-gated despite the comment saying it was, and an untrusted plugin with
email:blob-write could replace the bytes of a file the user was about to
send. Fixed the name. Relaxing the read while that stayed open was the
wrong order.
Moving upfiles.get to email:blob-read is kept, but it is a wider grant than
the PR described: onBeforeBlobUpload hands the file id to every registered
handler, so the id is not a secret and any untrusted plugin holding
email:blob-read can read every file the user attaches. The consent copy now
says that; email:blob-write's copy was describing the read and now describes
the replace it actually gates.
Also in the composer offload path:
- sanitize the handler's html before it reaches the document and the sent
message, and insert it through the editor at the caret rather than
concatenating it after the signature and quoted block
- honour an abort that lands while the handler is offloading
- delete the right staged file when the handler re-saved it under a new id
via upfiles.save, which otherwise stranded a copy in IndexedDB forever
upfiles.get is a read, but sat behind the email:blob-write permission and
the privileged tier, which requires crypto:full plus a signed managed
bundle. That put a simple attachment-offload plugin in the same trust
bucket as crypto plugins that read stored message bytes.
The method only returns a file the user just attached in this session,
looked up by the unguessable UUID the composer itself passes to the
onBeforeBlobUpload hook. It cannot reach server-side message blobs; those
stay behind jmap.fetchBlob on the privileged tier. So gate it behind
email:blob-read (consent copy already covers attachment bytes) and drop
the tier requirement.
Note: the privileged set also lists 'upfiles.set', but the dispatched
method is named 'upfiles.save', so that entry never matched; left as-is
here, flagged in the PR.
api.http.post(path, body, options) now accepts a Blob/File body and sends
it as-is to the same-origin route instead of JSON.stringify-ing it into
'{}'. The plugin may only supply Content-Type and X-Plugin-* metadata
headers; anything else is refused, and the host attaches its credential
headers after the plugin's, so they can never be overridden. Non-Blob
bodies keep the stock JSON behaviour. The sandbox facade previously
dropped the third argument, so the options never reached the host.
A handler can now return an ExternalAttachmentResult ({externalAttachment:
true, html, text}) instead of a file id. The composer then deletes the
staged file, removes the attachment and appends the replacement content
to the body, so a plugin can push a large file to an external store and
leave a download link behind. Returning a string (or nothing) keeps the
stock upload path byte-for-byte.
Also adds plainTextMode to the upload callback deps, which the new branch
reads to pick between the html and text replacement.
zisisvakras
Linus Rath
Paulhenry Saux
Stefan Hildebrandt
Ctaoist
dealerweb
Linus Rath
Roman Varlakov