Skip to content

fix: send from the address a message was delivered to (shared/aliased mailboxes) - #918

Open
rotterp wants to merge 3 commits into
bulwarkmail:mainfrom
rotterp:fix/shared-mailbox-send-identity
Open

fix: send from the address a message was delivered to (shared/aliased mailboxes)#918
rotterp wants to merge 3 commits into
bulwarkmail:mainfrom
rotterp:fix/shared-mailbox-send-identity

Conversation

@rotterp

@rotterp rotterp commented Aug 26, 2026

Copy link
Copy Markdown

On a shared, group or aliased mailbox every reply, reply-all, forward and new
message went out as identities[0] — the account owner — no matter which
mailbox was open. Users hit this as "it keeps sending as the wrong address and
I have to fix the From by hand every time".

The cause

Two different behaviours were fused behind autoSelectReplyIdentity, which
ships off:

  1. Own-identity match — pick among the user's configured identities
    (exact address, then +tag-stripped). Never rewrites From:; it only
    chooses which of your own addresses sends. This is the Thunderbird/Outlook
    default.
  2. Domain catch-all rewrite — put an address you have not configured into
    From:, sending through a same-domain identity ([Feature]: Make the from-address field editable #246).

(1) is what makes a shared mailbox reply from its own address. It carries no
impersonation risk, so this makes it unconditional. findReplyIdentityId
already implemented exactly this and was, until now, referenced only from tests.

(2) stays behind the setting, whose description already promises precisely that
behaviour. It has to stay opt-in: ownedDomains treats any address on a
domain you hold an identity on as a catch-all candidate — including a
colleague's, and including one seen only in Bcc: — so on a multi-user domain
it can put someone else's address in your From. Enabling it for everyone would
have been a regression, not a fix.

What else this fixes

  • forward was excluded from the resolution entirely, so forwarding a
    message that arrived at a shared mailbox went out as the owner — even though
    the comment above the gate already promised "Reply/forward fall through to
    the recipient-based resolution below"
    , and the neighbouring inline-image
    effect in the same file groups all three modes together. Forwards take (1)
    but deliberately never (2): a reply continues a thread whose participants
    know the addressing, while a forward introduces a rewritten From to a
    recipient the user just typed, who cannot tell it isn't genuine.
  • handleQuickReply is narrowed to (1). That surface is a bare text box
    with no From row, so a rewritten From would be applied with nothing on screen
    to show it — or correct it.
  • ProComposeTabBody never passed composeFromAccountEmail, so in the
    Pro/embedded shell mode === 'compose' resolved to null and every new
    message defaulted to the account owner. That covered the whole compose
    surface there, since the Pro shell hoists every "show composer" intent —
    mailto: links included — into a tab.
  • handleStateChange gated the email-list refresh on the primary account.
    Stalwart's SSE never pushes StateChange for a delegated/shared owner, so
    those changes arrive only via the secondary poll under the owner's
    accountId. With a shared folder open, a background change refreshed the
    folder counters (anyMailboxChanged already reacts to every account) but
    never refetched the rows — so a server-side keyword patch, or another member
    of the shared mailbox acting on it, stayed invisible until a hard reload.
    Uses the existing resolveViewAccountId() helper.

Scope and limits

The refresh covers the first page of the open list, which is what
refreshCurrentMailbox fetches; rows scrolled past it are still re-appended
from local state. It is not instant either — the shared-account poll runs on
the secondary (20s) interval and is skipped while the tab is hidden.

Testing

16 tests across the two suites. Each fix was verified by reverting it and
confirming the intended cases go red while the controls stay green:

  • restoring the old identity gate → exactly 3 red (reply, forward, self-sent)
  • restoring the primary-only refresh gate → exactly 2 red (owner, aggregate)

Controls included on purpose: the catch-all rewrite still fires on a reply
when the setting is on, so the "no rewrite" tests prove the gate rather than a
broken resolver; and Mailbox-only changes, unrelated accounts and own-account
views must not trigger a refetch.

Note the composer suite's settings mock hard-coded autoSelectReplyIdentity: true while the shipped default is false — so the shipped configuration was
the one nothing exercised. The mock now matches the default.

tsc --noEmit and eslint are clean. 652 tests pass across the affected
suites; the 3 auth-store-logout failures are pre-existing on main.

Patrick Rotter and others added 3 commits August 26, 2026 12:21
On a shared, group or aliased mailbox every reply, reply-all and forward
went out as `identities[0]` - the account owner - no matter which mailbox
was open. Users hit this as "it keeps sending as the wrong address and I
have to fix the From by hand every time".

Two DIFFERENT behaviours were fused behind `autoSelectReplyIdentity`:

  1. pick among the user's OWN configured identities (exact address, then
     `+tag`-stripped). Never rewrites `From:` - it only chooses which of
     your own addresses sends.
  2. the domain catch-all: rewrite `From:` to an address you have NOT
     configured, sending through a same-domain identity.

(1) is what makes a shared mailbox reply from its own address, and it is
what people expect. It carries no impersonation risk, so it is now
unconditional - `findReplyIdentityId` already implemented exactly this and
was until now referenced only from tests.

(2) stays behind the setting, whose description already promises exactly
that behaviour ("for domain catch-all deliveries, rewrites the From header
to the alias"). It must stay opt-in: `ownedDomains` in `resolveReplyFrom`
treats ANY address on a domain you hold an identity on as a catch-all
candidate - including a colleague's, and including one that only appeared
in `Bcc:` - so on a multi-user domain it can put someone else's address in
your From. Enabling that for everyone would have been a regression, not a
fix.

`forward` is additionally admitted to (1). It was excluded, so forwarding a
message that arrived at a shared mailbox went out as the owner - even
though the comment above the gate already promised "Reply/forward fall
through to the recipient-based resolution below", and the neighbouring
inline-image effect in the same file groups all three modes together.
Forwards deliberately do NOT take (2) even when it is enabled: a reply
continues a thread whose participants already know the addressing, while a
forward introduces the rewritten From to a recipient the user just typed,
who has no way to tell it is not really from that person.

`handleQuickReply` in `components/mail/mail-app.tsx` is likewise narrowed
to (1). That surface is a bare text box with no From row, so a rewritten
From would be applied with nothing on screen to show it - or to correct
it. It now resolves own identities only; the full composer remains
available when the rewrite is wanted.

Tests cover: reply and forward from the delivered-to address; no rewrite
while the setting is off; no rewrite on a forward even with it on; and, as
a control, that the rewrite still happens on a reply with it on - so the
first two prove the gate rather than a broken resolver. The settings mock
in `reply-addressing.test.tsx` was flipped to the shipped default (it hard
-coded the setting on, which is why this gap had no coverage). Verified
that restoring the old gate turns exactly the three positive cases red.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`ProComposeTabBody` rendered `<EmailComposer>` without
`composeFromAccountEmail`; only the standard shell in
`components/mail/mail-app.tsx` passed it. Without that prop
`mode === 'compose'` resolves `findComposeIdentityId(identities, undefined)`
to null, so in the Pro/embedded shell every new message defaulted to the
account owner even with a shared folder open.

That covered the whole compose surface there, not an edge: the Pro shell
hoists every "show composer" intent into a tab - including `mailto:` links
- and the inline composer never renders while it is active.

Resolved exactly as the standard shell does, so both shells share one
rule.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`handleStateChange` gated the email-list refresh on the PRIMARY account:

    const accountChanges = change.changed[accountId];
    if (accountChanges?.Email) {
      await get().refreshCurrentMailbox(client);

Stalwart's SSE only pushes StateChange for the primary account, never for
a delegated/shared owner, so changes to a shared folder arrive solely via
the client's secondary poll - reported under the OWNER's accountId. With a
shared folder open, a background Email change therefore refreshed the
folder COUNTERS (`anyMailboxChanged` already reacts to every account) but
never refetched the rows, leaving the visible list stale until a manual
reload.

That is most visible when something outside the client mutates a message:
a server-side keyword patch - e.g. an operator job stamping a colour label
- or another member of the shared mailbox acting on it simply did not
appear. The counters moving while the list did not is the tell.

Uses the existing `resolveViewAccountId()` helper, which answers "which
owner account am I looking at" and resolves through `resolveActionMailboxes`
so it stays correct in the multi-account shell. It returns undefined for a
normal own-account view, leaving that path unchanged. The aggregate views
contribute shared accounts too, so they take any contributing account's
Email change; `refreshCurrentMailbox` already coalesces, which bounds the
extra refetches a busy shared mailbox can trigger.

Scope worth stating: this refreshes the first page of the open list, which
is what `refreshCurrentMailbox` fetches - rows scrolled past it are still
re-appended from local state. Nor is it instant: the shared-account poll
runs on the secondary (20s) interval and is skipped while the tab is
hidden.

Tests cover the owner-account case, the aggregate-view case, and four
controls: primary-account (unchanged), an unrelated account, a
Mailbox-only change, and an own-account view. Verified that restoring the
primary-only gate turns exactly the owner and aggregate cases red.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant