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.
Clicking an address in the app's own chrome - the recipient card, the
contact sidebar, the contact details - handed a mailto: URL to the OS
mail handler. In a webmail client that goes nowhere: the browser asks
which application should open the link, and the message is composed
somewhere else, if at all. Only users who registered Bulwark itself as
their system mailto handler got a usable result, and even then via a
detour through the OS into a new tab.
A new MailtoLink component sends the click to the built-in composer
instead. It stays a real anchor with an href, so "Copy email address",
middle click and assistive technology keep working; only the plain
left click is redirected, and only when a listener takes it - otherwise
the browser's own behaviour still applies. Modified clicks are left
alone.
The request reaches the mail page through a window event, the way
`bulwark:rate-limit-blocked` already does, rather than threading a
callback through three component levels and five call sites. It goes to
openMailtoDraft rather than the protocol path: an in-app click carries
its own context - the account being read - so the account picker that an
external mailto: needs would only get in the way here.
Message content is deliberately untouched. Plain text bodies render into
the main document rather than the sandboxed frame and their sanitizer
permits mailto:, so a document-wide interceptor would have swept up
untrusted links too; a component cannot be reached from message HTML at
all.
Opening a message with a calendar invitation printed the whole parsed
event to the console on every render - title, description, attendees and
location included - through a leftover `[PARSE DEBUG]` console.log.
The surrounding file already logs via debug.log('calendar', ...) in 22
places, which honours the debugMode setting and its category filters, so
this one line was the outlier. It also ran JSON.stringify with
indentation unconditionally, even though nothing was listening.
Passing the object to debug.log instead leaves the formatting to the
logger, so nothing is serialised unless the calendar debug category is
actually switched on.
If you have a large number of tags, getTagCounts would not be able to get the
unread counts because it did not respect maxCallsInRequest, even though the
value was actually read out, it was just ignored. There are also other places
where the limits were not respected.
Batching is now generalized in a helper that also takes maxObjectsInSet, which
also was ignored, into account and is applied to all functions.
In addition, the dev mock now also advertises and enforces the limits, so these
issues can get picked up during development.
Possible closes#699
Possibly closes#399
Previously tags where very much focused on color coding email and less about
adding additional information. They were also visualized in different ways in
different locations.
This commit gets rid of all "Color-coding" references, aligns visualization of
the tags across the whole project and tries to improve user experience of using
tags in general.
A search box is shown in the tagging control so the user can quickly search for
a tag if they have a huge (more than 10) amount of tags.
- levels are joined by forward slashes in the keywords
- behaviour is opt-in for now
- long paths are shortened if there is not enough display room
Closes#687.
process.env coerces assigned values to strings, so
process.env.TZ = originalTZ left the literal string "undefined"
behind (instead of clearing TZ) when it was unset before the test
ran. Delete the var in that case instead of assigning undefined.
supportsContacts and supportsCalendars checked the server-wide session
capability. An account with jmap-contact-* / dav-card-* revoked still got
Contacts, and one with jmap-calendar-* / dav-cal-* revoked still got
Calendars, with every action failing on an authorization error.
Setting process.env.TZ at module scope without restoring it could leak
into other test files sharing the same Vitest worker. Match the
beforeAll/afterAll restore pattern already used in
lib/__tests__/calendar-utils.test.ts.
buildForwardAsAttachmentPayload named the synthetic .eml attachment
using the user's own emailDownloadTemplate, which by default embeds
sender/recipient display names. Since this attachment can go to an
external recipient (e.g. an upstream spam gateway, or anyone else),
the filename now always renders as "{date}-{subject}.eml" regardless
of the user's configured template, while still honoring their
space/case/diacritics preferences.
buildForwardAsAttachmentPayload called buildForwardSubject(email.subject,
forwardPrefix) unconditionally, and buildForwardSubject("", prefix)
returns just the bare prefix rather than "". Normal Forward doesn't do
this - EmailComposer's getInitialSubject() returns "" outright when
!replyTo?.subject, only calling buildForwardSubject when there's an
actual subject to prefix. So forwarding a subject-less message as an
attachment produced "Fwd:" as the subject, while normal Forward left
it blank.
Only call buildForwardSubject when email.subject is truthy, matching
getInitialSubject()'s behavior exactly. Add a test.
Caught by GitHub Copilot's automated PR review.
buildForwardAsAttachmentPayload called emailExportFilename(email) with
no options, always using the default naming template regardless of
the user's configured emailDownloadTemplate (and space/case/diacritics
transforms) - the same settings the neighboring "Export as .eml"
action already respects. That could produce inconsistent .eml
filenames between the two actions for the same message.
Accept an optional EmailFilenameOptions parameter and pass it through.
handleForwardAsAttachment now reads the same settings
email-viewer.tsx's emailFilenameOptions useMemo does, via
useSettingsStore.getState() (a one-off read inside an event handler,
matching this file's existing pattern, rather than a new reactive
subscription). Add a unit test covering a custom template.
Caught by GitHub Copilot's automated PR review.
Linus Rath
Linus Rath
Roman Varlakov
Paulhenry Saux
copilot-swe-agent[bot]andrathlinus
dealerweb
Eduard Polonskyi
Paulhenry Saux
Mathy Vanvoorden
Aaron Guise
local