From 3f0f26f2ca7c0391b6e955aedffb5d5c354429f7 Mon Sep 17 00:00:00 2001 From: waggins Date: Sun, 4 Oct 2026 09:40:27 -0400 Subject: [PATCH 01/20] =?UTF-8?q?docs:=20add=20the=20webmail=201.10?= =?UTF-8?q?=E2=80=931.12=20parity=20delta=20and=20the=20parity=20roadmap?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 5.5 --- PARITY_CHECKLIST.md | 53 +- docs/parity/01-auth-accounts.md | 41 ++ docs/parity/02-mail-list-folders.md | 83 ++- docs/parity/03-email-viewer.md | 34 + docs/parity/04-composer-send.md | 43 ++ docs/parity/05-calendar.md | 48 ++ docs/parity/06-contacts.md | 20 + docs/parity/07-filters-vacation-files.md | 36 + docs/parity/08-settings-push-i18n-ui.md | 36 + docs/parity/09-jmap-core-sync-security.md | 13 + ...2026-10-04-parity-phase-1-security-send.md | 631 ++++++++++++++++++ .../2026-10-04-webmail-parity-roadmap.md | 139 ++++ 12 files changed, 1164 insertions(+), 13 deletions(-) create mode 100644 docs/superpowers/plans/2026-10-04-parity-phase-1-security-send.md create mode 100644 docs/superpowers/plans/2026-10-04-webmail-parity-roadmap.md diff --git a/PARITY_CHECKLIST.md b/PARITY_CHECKLIST.md index 9166f36e..c26f14bb 100644 --- a/PARITY_CHECKLIST.md +++ b/PARITY_CHECKLIST.md @@ -85,22 +85,46 @@ again), dropped three obsolete items and ticked what it completed: 380 of 415 items are done, 35 open. What still needs a device check or a decision is in the audit's "Fix pass, 2026-09-24" section. +## Webmail 1.10.0 → 1.12.0+ delta, 2026-10-04 + +Webmail moved from 1.9.2 to 1.12.0 (2026-09-30) plus 47 unreleased commits up +to `a4e313f` (2026-10-02). Every user-facing changelog bullet and commit in that +range was checked against native `main` at `76180b3`, skipping what +[docs/audit-2026-09.md](docs/audit-2026-09.md) already lists. The 74 new items +sit in a "Webmail 1.10.0 → 1.12.0+ delta" section in each area file (3 P1, +24 P2, 47 P3); what was already at parity is one "1.10–1.12 delta" line at the +end of each area's Verified list. The three P1s, and the P2s worth doing next: + +- P1: forged `Authentication-Results` can fake a DMARC/DKIM pass (03); refused + recipients in `deliveryStatus` are never reported, so a send that reached no + one shows as sent (04); an escaped quote in a display name splits off an extra + recipient (04). +- P2 security: Sieve values unescaped (07), multi-address `mailto:` unsubscribe + (03), TNEF parser loop (03). +- P2 data: contacts with calendar/scheduling URIs and vCard imports rejected by + Stalwart, address book delete refused (06); cross-account move drops the date + (02); a draft follows an account switch into the wrong account (04); Sieve + `stop` missing after discard/reject and `field: 'all'` rules breaking every + native save (07). +- P2 reliability: push subscriptions lapse after Stalwart's 7-day expiry (08); + list actions fail silently (02); daily events stop at DST (05). + ## Areas | # | Area | File | Items | Done | Open | P1 | P2 | P3 | |---|---|---|---|---|---|---|---|---| -| 01 | Authentication, login, session, multi-account | [docs/parity/01-auth-accounts.md](docs/parity/01-auth-accounts.md) | 30 | 29 | 1 | 4 | 8 | 18 | -| 02 | Mail list, folders, unified views, search, tags | [docs/parity/02-mail-list-folders.md](docs/parity/02-mail-list-folders.md) | 54 | 50 | 4 | 1 | 20 | 33 | -| 03 | Email viewer, thread view, rendering, attachments | [docs/parity/03-email-viewer.md](docs/parity/03-email-viewer.md) | 46 | 45 | 1 | 6 | 13 | 26 | -| 04 | Composer, drafts, sending, identities, templates, scheduled send | [docs/parity/04-composer-send.md](docs/parity/04-composer-send.md) | 51 | 46 | 5 | 5 | 14 | 32 | -| 05 | Calendar and tasks | [docs/parity/05-calendar.md](docs/parity/05-calendar.md) | 50 | 43 | 7 | 5 | 20 | 25 | -| 06 | Contacts and address books | [docs/parity/06-contacts.md](docs/parity/06-contacts.md) | 50 | 46 | 4 | 1 | 18 | 31 | -| 07 | Filters (Sieve), vacation responder, Files | [docs/parity/07-filters-vacation-files.md](docs/parity/07-filters-vacation-files.md) | 32 | 29 | 3 | 3 | 8 | 21 | -| 08 | Settings, sync, push, i18n, themes, updates, misc UI | [docs/parity/08-settings-push-i18n-ui.md](docs/parity/08-settings-push-i18n-ui.md) | 46 | 38 | 8 | 0 | 14 | 32 | -| 09 | JMAP client core, live sync, offline, security, S/MIME | [docs/parity/09-jmap-core-sync-security.md](docs/parity/09-jmap-core-sync-security.md) | 56 | 54 | 2 | 7 | 23 | 26 | -| | **Total** | | **415** | **380** | **35** | **32** | **138** | **244** | - -Counts are of the `- [ ]` and `- [x]` items per file as of 2026-09-24. Done and +| 01 | Authentication, login, session, multi-account | [docs/parity/01-auth-accounts.md](docs/parity/01-auth-accounts.md) | 38 | 29 | 9 | 4 | 12 | 22 | +| 02 | Mail list, folders, unified views, search, tags | [docs/parity/02-mail-list-folders.md](docs/parity/02-mail-list-folders.md) | 76 | 50 | 26 | 1 | 24 | 51 | +| 03 | Email viewer, thread view, rendering, attachments | [docs/parity/03-email-viewer.md](docs/parity/03-email-viewer.md) | 52 | 45 | 7 | 7 | 17 | 27 | +| 04 | Composer, drafts, sending, identities, templates, scheduled send | [docs/parity/04-composer-send.md](docs/parity/04-composer-send.md) | 60 | 46 | 14 | 7 | 16 | 37 | +| 05 | Calendar and tasks | [docs/parity/05-calendar.md](docs/parity/05-calendar.md) | 61 | 43 | 18 | 5 | 23 | 33 | +| 06 | Contacts and address books | [docs/parity/06-contacts.md](docs/parity/06-contacts.md) | 53 | 46 | 7 | 1 | 21 | 31 | +| 07 | Filters (Sieve), vacation responder, Files | [docs/parity/07-filters-vacation-files.md](docs/parity/07-filters-vacation-files.md) | 38 | 29 | 9 | 3 | 11 | 24 | +| 08 | Settings, sync, push, i18n, themes, updates, misc UI | [docs/parity/08-settings-push-i18n-ui.md](docs/parity/08-settings-push-i18n-ui.md) | 54 | 38 | 16 | 0 | 15 | 39 | +| 09 | JMAP client core, live sync, offline, security, S/MIME | [docs/parity/09-jmap-core-sync-security.md](docs/parity/09-jmap-core-sync-security.md) | 57 | 54 | 3 | 7 | 23 | 27 | +| | **Total** | | **489** | **380** | **109** | **35** | **162** | **291** | + +Counts are of the `- [ ]` and `- [x]` items per file as of 2026-10-04. Done and Open split them by tick; the P columns count the priority tags on those items (one item carries none). @@ -141,6 +165,11 @@ Open split them by tick; the P columns count the priority tags on those items - [x] Push effect is keyed on the singleton client, so the SSE stream stays bound to the previous account after `switchAccount`. → [09](docs/parity/09-jmap-core-sync-security.md) *(fixed in edc26ce)* - [x] Webmail password handoff sends the clear-text password in a custom-scheme redirect fragment that any app can register; OAuth `state` uses `Math.random`; `server_url`/`token_endpoint` in the callback are trusted as-is. → [09](docs/parity/09-jmap-core-sync-security.md), [01](docs/parity/01-auth-accounts.md) *(fixed in 2c0dbd1)* +### Webmail 1.10–1.12 delta (2026-10-04) +- [ ] A forged lower `Authentication-Results` header can supply a DKIM/DMARC pass in the security badge. → [03](docs/parity/03-email-viewer.md) +- [ ] Recipients refused at RCPT TO (`deliveryStatus`, #1123) are never read back; a send that reached nobody shows as sent. → [04](docs/parity/04-composer-send.md) +- [ ] `splitRecipients` ignores escaped quotes, so a crafted display name splits off an extra recipient. → [04](docs/parity/04-composer-send.md) + ### Repo health - [x] `npm test` is red on `main` (see Baseline health above). *(fixed in b84d4d8)* diff --git a/docs/parity/01-auth-accounts.md b/docs/parity/01-auth-accounts.md index 050b8dcd..22885da5 100644 --- a/docs/parity/01-auth-accounts.md +++ b/docs/parity/01-auth-accounts.md @@ -163,6 +163,46 @@ RN covers the happy paths (password login, webmail-mediated OAuth handoff, QR pa - What RN does: `randomState()` uses `Math.random` (`src/lib/oauth.ts:41-46`) although a CSPRNG helper with `getRandomValues`/`randomUUID` fallbacks already exists in `src/lib/totp.ts:16-46`. The state is the only guard against a forged `bulwarkmobile://` redirect delivering foreign credentials. - Fix hint: export `randomBytes` from `totp.ts` (or a shared `random.ts`) and use it here. +## Webmail 1.10.0 → 1.12.0+ delta (audited 2026-10-04) + +Webmail changelog 1.10.0, 1.11.0-beta.1 – 1.11.2 and 1.12.0, plus the +unreleased commits up to `a4e313f` (2026-10-02), checked against native `main` +at `76180b3`. Items already listed in [../audit-2026-09.md](../audit-2026-09.md) +are not repeated. "Unverified" means read from the code but not confirmed on a +device. + +- [ ] **TOTP accounts cannot change their password or turn TOTP off** — `P2` — `bugfix-parity` (1.11.0) + - What WEB does: sends the current TOTP code with password and TOTP changes (`stores/account-security-store.ts:263-266,583-603,676-690`). + - What RN does: `src/api/account-security.ts:346,387` send no code; the server refuses the change. + - Fix hint: prompt for the current code in `AccountSecuritySettings.tsx` when TOTP is on, and pass it through. + +- [ ] **Sign-out leaves account data on the device** — `P2` — `bugfix-parity` (1.11.0, WEB `lib/sign-out-cleanup.ts`) + - What RN does: search history is never cleared (`src/stores/search-history-store.ts:41`); the offline body cache keeps full message bodies after sign-out or account removal (`offline-cache-store.ts:292` `clearAll` is never called from `auth-store.ts:575-760`); outbox keys of removed accounts stay. + - Fix hint: one per-account cleanup function called from logout and removeAccount. + +- [ ] **Internationalized domains (IDN) fail at sign-in and in addresses (#1100)** — `P2` — `missing` (1.11.0) + - What WEB does: punycode handling in `lib/idn.ts`, used by `stores/auth-store.ts:29`. + - What RN does: no punycode handling; `isValidEmail` accepts ASCII only (`src/lib/recipients.ts:32`). Unverified whether Hermes' URL covers the host part. + +- [ ] **Filters and the security page can show the previous account's data after a switch** — `P2` — `rn-only-bug` (edge case, partly unverified) + - What RN does: `switchAccount` (`src/stores/auth-store.ts:658-742`) does not reset the filter or vacation stores, and the load effects in `FilterSettings.tsx:160-163` and `AccountSecuritySettings.tsx:870-896` do not depend on the active account. Reachable when a notification tap or deep link switches account while the screen stays mounted; a save would then write the old account's rules into the new one. + - Fix hint: reset the stores in `switchAccount` and key the effects on the active account id. + +- [ ] **No "Sign in with an access token"; session URLs on another origin are rewritten** — `P3` — `missing` (29c74c3) + - What WEB does: Bearer-token login for servers such as Fastmail, and keeps download/upload/eventSource URLs that sit on another HTTPS origin. + - What RN does: `connectWithToken` exists (`src/api/jmap-client.ts:299`) but has no UI; `src/api/jmap-client.ts:587-596` rewrites every session URL onto the server origin, which would break such servers. + +- [ ] **A refused token exchange on a TOTP login gets a generic error** — `P3` — `partial` (post-1.12) + - What RN does: throws `TotpLoginError('token_exchange_failed')` (`src/lib/totp-login.ts:148`), but `src/lib/login-errors.ts:129` only maps `invalid`. + - Fix hint: add a message for `token_exchange_failed`. + +- [ ] **Security page shows an empty name for non-admin users** — `P3` — `bugfix-parity` (1.11.0) + - What WEB does: falls back when `x:Account/get` (admin-only on Stalwart) is refused (`stores/account-security-store.ts:519-529`). + - What RN does: `AccountSecuritySettings.tsx:886-888` reads it from `x:Account/get` only; `fetchAccountDisplayName` (`src/api/account-security.ts:291`) exists but is not used here. + +- [ ] **SSO sign-out does not end the identity provider's session (#905)** — `P3` — `missing` (1.11.0) + - What RN does: `end_session_endpoint` is never called (`src/lib/oauth-native.ts:212`), so the next sign-in in the in-app browser reuses the provider session. + ## Verified at parity (brief list, so the fixer knows NOT to redo) - QR login payloads: WEB emits `bulwarkmail://pair?server=&code=…` (`account-security-settings.tsx:971`); RN parses it plus a `connect` variant and bare URLs (`src/lib/oauth.ts:135-162`) and redeems at `/api/auth/pair/redeem` (`:165-207`) into the same OAuth bundle the browser handoff yields. - Webmail handoff (`mobile_redirect_uri`/`mobile_state`, fragment transport, state check, password/oauth flows) matches `login/page.tsx:115-130, 282, 638-650` and `auth/callback/page.tsx` mobile branch. @@ -175,6 +215,7 @@ RN covers the happy paths (password login, webmail-mediated OAuth handoff, QR pa - Server discovery by email domain (`/.well-known/jmap` probe on bare/`mail.`/`webmail.` hosts, 401 counts as a hit, known servers trusted) is RN's equivalent of WEB's admin server list / auto-pick-by-domain (#799); neither side does `_jmap._tcp` SRV. - Login error → session-expired banner: RN's `ChooseStep` shows the store error ("Session expired") like WEB's `session_expired` banner. - `AuthenticationError` on 401, `RateLimitError` on 429 with Retry-After parsing in `request()`. +- 1.10–1.12 delta: `prompt=select_account` on add account; permanent refresh failures not retried (#972); discovery base path (#971); session refetch after a redirect drops auth (#892); no `max_age=0` (#938); SSO discovery uses the selected server (#952); rate-limited token endpoint doesn't sign out; full sign-out removes an account whose restore failed; webmail "Link Mobile App" password bundles (`src/lib/oauth.ts:193-215,438-451`). ## N/A on mobile - "Remember me" (RN always stores credentials in the device keychain), `rememberMeEnabled`/`SESSION_SECRET` cookie logic, per-slot cookie handling (`cookieSlot`, `oauth_cookie_slot`), orphan-cookie adoption, `serverIdentifiers`/`classifySessionMatch` slot→token desync guard (RN keys credentials per registry id, no shared slot). diff --git a/docs/parity/02-mail-list-folders.md b/docs/parity/02-mail-list-folders.md index ea7a6590..ad62c1c2 100644 --- a/docs/parity/02-mail-list-folders.md +++ b/docs/parity/02-mail-list-folders.md @@ -284,6 +284,86 @@ RN covers the core single-folder loop well (folder tree incl. shared/group accou - What RN does: `runOfflineSync` discovers via `queryEmailsByFilter` which hard-codes `jmapClient.accountId` (`src/lib/offline-sync.ts:55`, `src/api/email.ts:733-747`), so opening a group-folder message offline always fails; `selectMailbox` seeding for a shared folder therefore always yields nothing (`src/stores/email-store.ts:647-666` looks up by raw id, which is correct). - Fix hint: iterate `jmapClient.getSharedMailAccounts()` in the sync with `accountIdOverride`, storing the account id in the cache index (`getFullEmails(ids, accountId)` already exists). +## Webmail 1.10.0 → 1.12.0+ delta (audited 2026-10-04) + +Webmail changelog 1.10.0, 1.11.0-beta.1 – 1.11.2 and 1.12.0, plus the +unreleased commits up to `a4e313f` (2026-10-02), checked against native `main` +at `76180b3`. Items already listed in [../audit-2026-09.md](../audit-2026-09.md) +are not repeated. "Unverified" means read from the code but not confirmed on a +device. + +- [ ] **Search does not leave out Spam and Trash** — `P2` — `bugfix-parity` (1.12.0) + - What WEB does: adds `inMailboxOtherThan: [junk, trash]` unless "All folders" is picked explicitly, and searches the folder itself when the search starts in Spam or Trash. + - What RN does: queries with no mailbox filter (`src/stores/email-store.ts:267-271` `effectiveFolderScope`, `:477-484` `queryScope`). + +- [ ] **List actions fail silently when the server refuses them** — `P2` — `bugfix-parity` (1.11.0) + - What RN does: mark read, star, pin, tag and spam/not-spam from swipes and batch actions are fired with `void` and no catch (`src/screens/EmailListScreen.tsx:633-651,799`; `src/stores/outbox-store.ts:458-461` rethrows). No toast, and the optimistic change is never reverted. The viewer and unified inbox already show a toast. + +- [ ] **A failed search shows the previous folder's rows as results** — `P2` — `bugfix-parity` (1.11.0, WEB `lib/unified-mailbox.ts:597`) + - What RN does: `refreshEmailsImpl` only sets an error when the list is empty (`src/stores/email-store.ts:2842-2875`). + +- [ ] **A cross-account move loses the message date (#1150)** — `P2` — `bugfix-parity` (71a1fad) + - What RN does: `Email/import` sends no `receivedAt` (`src/api/email.ts:963-976` `importEmailBlob`, `src/stores/email-store.ts:~2175` `crossAccountMove`), so moved mail lands as today's. + - Fix hint: pass the original `receivedAt` to `Email/import`. + +- [ ] **No "Copy to folder / account"** — `P3` — `missing` (f02dbf3) + - What WEB does: copies messages to a folder of another connected account. RN cannot copy at all, not even within an account. + +- [ ] **Tag views and tag counts include Trash and Spam (#1156)** — `P3` — `bugfix-parity` (a4e313f) + - What RN does: `src/stores/email-store.ts:444` (`buildJmapFilter`) and `src/api/tag-counts.ts:38-45` do not exclude them. + +- [ ] **"Empty folder" is offered only for Trash and Junk** — `P3` — `partial` (1.12.0) + - What WEB does: offers it on any folder; an ordinary folder is moved to Trash (`stores/email-store.ts:1520` `emptyFolderMovesToTrash`). + - What RN does: `src/components/SidebarDrawer.tsx:510`, `src/api/email.ts:288`. + +- [ ] **Search still appends a wildcard to every term** — `P3` — `bugfix-parity` (1.11.0) + - What WEB does: sends terms as typed (`lib/jmap/search-utils.ts:44-48`). + - What RN does: `toWildcardQuery` (`src/lib/search-utils.ts:1-8`), used at `stores/email-store.ts:442`, `api/unified-inbox.ts:364`, `api/email.ts:1343`. The "Verified at parity" line below that calls the wildcard parity is out of date. + +- [ ] **Search hits are not highlighted (`SearchSnippet/get`)** — `P3` — `missing` (1.10.0, WEB `lib/search-snippet.ts`) + - What RN does: no SearchSnippet use; the row preview is at `src/screens/EmailListScreen.tsx:88`. + +- [ ] **No message-size filter in advanced search** — `P3` — `missing` (1.10.0, WEB `lib/jmap/search-utils.ts`, `components/search/search-chips.tsx`) + - What RN does: no minSize/maxSize in `EmailFilters` / `buildJmapFilter` (`src/stores/email-store.ts:253,435-465`). + +- [ ] **Search folder picker is flat** — `P3` — `partial` (1.12.0) + - What RN does: the first 12 folders as chips, no hierarchy (`src/screens/EmailListScreen.tsx:1567-1577`). + +- [ ] **Tags with no local definition are always grey** — `P3` — `partial` (1.12.0, #1052) + - What WEB does: gives each unknown tag its own colour; unified rows can be tinted with their account colour. + - What RN does: `FALLBACK_KEYWORD_COLOR` (`src/stores/keywords-store.ts:30`); `suggestKeywordColor` (`src/lib/keyword-discovery.ts:68`) could be reused. + +- [ ] **The list's Move sheet is not scoped to the message's account (#1149)** — `P3` — `bugfix-parity` (c317cd9) + - What RN does: in a shared mailbox your own folders come first (`src/screens/EmailListScreen.tsx:1705-1727`); the viewer is already scoped (`EmailThreadScreen.tsx:381`). + +- [ ] **List and notification previews do not skip a leading style sheet** — `P3` — `bugfix-parity` (2d521d1, 6bce332, WEB `lib/utils.ts`) + - What RN does: list row preview, `src/lib/push-background-task.ts` and the widgets show the CSS text. + +- [ ] **Mail folder sharing (`mail:share`) and share-notification toasts** — `P3` — `missing` (1.10.0) + - What WEB does: `components/layout/mailbox-share-dialog.tsx`, `stores/share-notification-store.ts`. + - What RN does: calendars and files can be shared, mailboxes cannot; `components/calendar/CalendarShareSheet.tsx` is the pattern. + +- [ ] **Mail list rows have no screen-reader label (#1008)** — `P3` — `missing` (1.10.0) + - What RN does: the row `Pressable` (`src/screens/EmailListScreen.tsx:158`) has no `accessibilityLabel`, role, or unread/selected state. + +- [ ] **Rows jump while attachment chips load** — `P3` — `partial` (1.11.0, WEB `components/email/attachment-chips.tsx:98-160`) + - What RN does: renders nothing until the chips arrive and keeps no cache (`src/components/email/ListAttachmentChips.tsx:60-88`). + +- [ ] **Mail deleted during a list refresh can reappear (#966)** — `P3` — `bugfix-parity` (1.11.0, unverified) + - What RN does: the full re-query overwrites the list without tracking rows removed while it was in flight (`src/stores/email-store.ts:2607+`). + +- [ ] **The list may not return to the top when another folder opens** — `P3` — `bugfix-parity` (17a42c4, unverified) + - What RN does: the FlatList has no per-folder key and no `scrollToOffset` (`src/screens/EmailListScreen.tsx:1440`). + +- [ ] **An opened message may jump in the "unread first" order** — `P3` — `bugfix-parity` (1.12.0, unverified) + - What RN does: `retainedIds` only applies to the Unread filter view, not the `unread_first` order (`src/stores/email-store.ts:1323`). + +- [ ] **The app restores the last folder on start instead of the inbox** — `P3` — `partial` (64b39c5, decision) + - What RN does: `currentMailboxId` is persisted (`src/stores/email-store.ts:783`). May be the intended mobile behaviour; decide and close. + +- [ ] **Global search across mail, contacts, calendar and files (#641, #847)** — `P3` — `missing` (1.10.0, product decision) + - What WEB does: `lib/global-search/`, `stores/global-search-store.ts`. Earlier audits called it Pro/desktop-only; the changelog describes a cross-surface search. + ## Verified at parity (do not redo) - Folder tree: nesting, per-account grouping of shared/group accounts with unread roll-up, expanded state persisted, role priority sort (`src/lib/mailbox-tree.ts`, `SidebarDrawer.tsx`); create/rename/delete own folders with server error surfaced (`FolderSettings.tsx`); delete-with-emails confirm. - Shared/group folders: queries and every mutation (read/star/pin/tag/move/archive/delete/undo/import) routed to the owner account with the unprefixed id (`refFor`, `src/stores/email-store.ts:64-99`); thread screen fetches by owner (`EmailThreadScreen.tsx:90-96`); state-change handling per account (`email-store.ts:981-1015`). @@ -294,10 +374,11 @@ RN covers the core single-folder loop well (folder tree incl. shared/group accou - Swipe actions: configurable per direction, instant and reveal modes, pure gesture module with tests, stale-props fix (`SwipeableRow.tsx`, `swipe-gesture.ts`); more actions than WEB (pin, move). RTL: ar, he and fa ship since 990cd84, but swipe directions are not mirrored yet (area 08, RTL finding). - Multi-select: long-press, select-all-visible/indeterminate, batch star/read/tag/move/archive/delete (`EmailListScreen.tsx:320-500`). - Pagination via `onEndReached`, pull-to-refresh, incremental `Email/queryChanges` + `Email/changes` refresh with per-account state tokens, base-view restore after search (#10), sort change invalidates snapshots (#5), offline seed and fallback. -- Search: full-text with wildcard suffix (`src/lib/search-utils.ts` = WEB `toWildcardQuery`), from/to/subject/date/attachment/unread/starred tri-state filters, chips row, search inside a shared folder routed to its owner. +- Search: full-text (the wildcard suffix is no longer parity since WEB 1.11.0, see the delta section), from/to/subject/date/attachment/unread/starred tri-state filters, chips row, search inside a shared folder routed to its owner. - Keyword writes send RFC 6901-escaped `keywords/` patch pointers since 5041897 (`src/api/patch-pointer.ts`; the whole-map writes erased other keywords, audit B4), with `null` instead of `false` — not affected by 5c484af1 / be97c8bf; tag ids normalized exactly like WEB `normalizeKeywordLevel` (`KeywordSettings.tsx:118-123`); batch tag toggle via `TagSheet` with "all have it" semantics. - Unread dot (#27), date formats smart/relative/full with 12/24h, preview toggle, density-aware rows, sender favicon avatars with failure cache, `.eml`/`.zip` import into the open folder, Scheduled quick view, "Include group inboxes" setting and shared badge/account dot in the unified list. - Mark-as-read delay (0/3s/5s) in the thread screen; both clients still display `receivedAt` in the list (WEB `lib/email-date.ts` is not wired into the list yet). +- 1.10–1.12 delta: Email/Mailbox `changes` push deltas; attachment chips on rows, loaded lazily (#1089); delete a non-empty folder; "clear search on folder switch"; tag view across accounts (#1038) and search inside it (#1084); unified section only when populated (#843, #959); stale quick-search guard (#872); shared-folder hits keep their account (#923) and "All folders" covers shared accounts (#1082); Email/set failures surfaced (#956); missing archive reported (#578) and shared mail archived in the owner's account (#889); spam from the viewer (#695); new folders subscribed (#951); unparsable dates (#1099); no snap back to unified (#1102); cross-account/tag/All-folders paging; refused empty-folder, cancel-scheduled and mark-folder-read reported; keyword patches write only changed keys; mark-all-read past 500; tag counters without full id lists; unified threads owned per account (#1012); failed cross-account move keeps the original; tapping a folder's unread count. ## N/A on mobile - Category tabs / message-list tabs (`stores/message-list-tabs-store.ts`) — plugin-registered; RN has no plugin runtime. diff --git a/docs/parity/03-email-viewer.md b/docs/parity/03-email-viewer.md index 8e89c446..8c2fd2a8 100644 --- a/docs/parity/03-email-viewer.md +++ b/docs/parity/03-email-viewer.md @@ -239,6 +239,39 @@ RN has a solid single-message reader (WebView body with CSP, shrink-to-fit + pin - What RN does: `SmimeSettings` renders empty mock arrays and buttons with no handlers (`src/components/settings/SmimeSettings.tsx:32-33`, `118-121`, `163-166`); there is no verification/decryption in the viewer. - Fix hint: hide the screen (or show a "not available in the mobile app" note) until a crypto path exists; treat viewer-side S/MIME as N/A. +## Webmail 1.10.0 → 1.12.0+ delta (audited 2026-10-04) + +Webmail changelog 1.10.0, 1.11.0-beta.1 – 1.11.2 and 1.12.0, plus the +unreleased commits up to `a4e313f` (2026-10-02), checked against native `main` +at `76180b3`. Items already listed in [../audit-2026-09.md](../audit-2026-09.md) +are not repeated. "Unverified" means read from the code but not confirmed on a +device. + +- [ ] **A sender can fake a DMARC/DKIM pass in the security badge** — `P1` — `bugfix-parity` (1.11.0, security) + - What WEB does: splits each `Authentication-Results` header into results (skipping quotes and comments) and takes DKIM, DMARC and iprev only from the topmost header; a sender's own header can only downgrade SPF (`lib/email-headers.ts:40-145`, `lib/jmap/client.ts:2444-2448`). + - What RN does: `deriveHeaderInfo` joins every `Authentication-Results` header with `; ` and takes the first regex match for `dkim=` / `dmarc=` (`src/lib/email-headers.ts:107-160,262-264`), so a header the sender added, or text inside a comment, can supply the pass. + - Fix hint: port WEB's parser and the topmost-header rule; add tests with a forged lower header. + +- [ ] **A `mailto:` unsubscribe can go to several addresses without showing them** — `P2` — `bugfix-parity` (1.11.0, security) + - What WEB does: sends to the single address in the link and shows recipient, subject and body before sending (`lib/validation.ts:180`, `components/email/unsubscribe-banner.tsx:44-49,164,198`). + - What RN does: takes every comma-separated address plus `?to=`/`?cc=` and confirms with a generic alert (`src/lib/unsubscribe.ts:79-118`, `src/components/email/UnsubscribeBanner.tsx:99-140`). + +- [ ] **A crafted `winmail.dat` can freeze the app** — `P2` — `bugfix-parity` (1.11.0, security) + - What WEB does: stops when a value fails to parse or makes no progress, and bounds the count with `readValueCount` (`lib/tnef.ts`). + - What RN does: `parseMAPIProps` (`src/lib/tnef.ts:168-220`) can loop up to a sender-chosen 32-bit count on a truncated value, blocking the JS thread. + +- [ ] **No "Rules" entry on a message** — `P2` — `missing` (1.12.0) + - What WEB does: creates a filter rule from the message (move by sender, domain or list, mark read, tag, block), suggests conditions, can apply it to existing mail, with undo (`components/email/rules-menu.tsx`, `lib/filters/quick-rules.ts`, `lib/filters/retroactive.ts`, `stores/quick-rule-store.ts`). + - What RN does: nothing; would hang off `src/components/email/ActionSheet.tsx` and prefill `src/components/filters/FilterRuleModal.tsx`. + +- [ ] **No copy chip for verification codes** — `P2` — `missing` (1.12.0) + - What WEB does: detects one-time codes and shows a copy chip in the message and, for a day, in the list; "Show Verification Codes" setting (`lib/verification-code.ts`, `components/email/verification-code-chip.tsx`). + - What RN does: nothing; candidates are `MessageHeader.tsx`/`MessageContent.tsx`, the list row and `ReadingSettings.tsx`. + +- [ ] **Fixed-width tables shrink instead of wrapping on iOS (#1020)** — `P3` — `bugfix-parity` (1.11.0, unverified on device) + - What WEB does: `releaseFixedWidthTables` (`lib/email-fit-width.ts`). + - What RN does: a `` is scaled down (`src/lib/email-html.ts:452-466`, `src/components/EmailBodyView.tsx:264`). + ## Verified at parity (brief list, so the fixer knows what NOT to redo) - Body isolation: sandboxed WebView with `default-src 'none'` CSP, `originWhitelist` about:blank, links opened externally, no cookies/storage/file access (`src/components/EmailBodyView.tsx:675-724`, `src/lib/email-html.ts:236-256`) — matches WEB's srcDoc iframe + CSP approach (1.6.7). - `hasMeaningfulHtmlBody` text-alternative preference (`src/lib/email-html.ts:353-362`) — same regex as WEB `lib/signature-utils.ts` (only the same-partId guard is missing, see P1 finding). @@ -256,6 +289,7 @@ RN has a solid single-message reader (WebView body with CSP, shrink-to-fit + pin - View source (raw RFC 822, shareable) and Export .eml with the email filename template (`src/screens/EmailSourceScreen.tsx`, `src/lib/email-export.ts:173-206`). - Calendar invitation: detection by MIME/extension, parse via `CalendarEvent/parse`, RSVP via import-then-`rsvpEvent` with `replyTo`/`organizerCalendarAddress`, cancelled notice, "no writable calendar" warning, `calendarInvitationParsingEnabled` setting (`src/components/email/CalendarInvitationBanner.tsx`, `src/lib/calendar-invitation.ts`). - Tag sheet from the viewer with keyword definitions (`src/screens/EmailThreadScreen.tsx:1033-1100`). +- 1.10–1.12 delta: link clicks gated by scheme; `cid:` octet-stream parts hidden (#1005); images/tables keep their max-width (#1034, #790); truncated body refetched (#928); text direction detected (#663); plain-text-only mail as text (#489); SVG/HTML attachments never previewed inline; odd types previewed by file name; remote content blocked by the WebView CSP; spam/not-spam refusals reported; MDN header CR/LF stripped (GHSA-w38p). ## N/A on mobile - Print (WEB `handlePrint`, `components/email/email-viewer.tsx:2621-2670`) — could be done with `expo-print` later, but not a parity requirement. diff --git a/docs/parity/04-composer-send.md b/docs/parity/04-composer-send.md index f21231a4..0a0c9736 100644 --- a/docs/parity/04-composer-send.md +++ b/docs/parity/04-composer-send.md @@ -280,6 +280,48 @@ Legend for refs: WEB paths are relative to `the webmail repo`, RN paths to `the - What RN does: `ComposeScreen.tsx:620-623`, `:696-699`, `:719` hard-code "Photo library permission is required…", "Could not load image"; `identityError` fallback text at `:1052` ("identity unavailable", "Loading..."). - Fix hint: route through `t()` with keys in `locales//common.json`. +## Webmail 1.10.0 → 1.12.0+ delta (audited 2026-10-04) + +Webmail changelog 1.10.0, 1.11.0-beta.1 – 1.11.2 and 1.12.0, plus the +unreleased commits up to `a4e313f` (2026-10-02), checked against native `main` +at `76180b3`. Items already listed in [../audit-2026-09.md](../audit-2026-09.md) +are not repeated. "Unverified" means read from the code but not confirmed on a +device. + +- [ ] **Recipients the server refuses at send are never reported** — `P1` — `bugfix-parity` (1.12.0, #1123) + - What WEB does: Stalwart runs RCPT TO while creating the submission and records refusals in `deliveryStatus` while the create succeeds. WEB reads `deliveryStatus` back (`EmailSubmission/get` on `#creationId`); if every recipient was refused it fails the send, drops the Sent copy and keeps the draft, and if some were it warns and names them (`lib/jmap/client.ts` ~716-790: `deliveryStatusCall`, `rejectedRecipients`, `RecipientsRejectedError`). + - What RN does: never reads `deliveryStatus` (`src/api/email.ts:1586-1680` `sendEmail`), so a send that reached nobody shows as sent. + +- [ ] **A quote in a display name can create an extra recipient** — `P1` — `bugfix-parity` (1.11.0, security; lower exposure than WEB) + - What WEB does: honours escaped quotes when splitting (`lib/email-composer-utils.ts:283-289`). + - What RN does: `splitRecipients` (`src/lib/recipients.ts:87`) toggles on every `"` and ignores `\"`, so `Support\", ceo@corp.example, \"x` splits off an address; `findTopLevelColon` (~:155) has the same gap. Callers: `ComposeScreen.tsx:948,2238`, `IdentitySettings.tsx:84` (pasted recipients and identity settings). + +- [ ] **A send with no `EmailSubmission/set` response counts as a success** — `P2` — `bugfix-parity` (15740fa, 07625bc) + - What WEB does: throws `SendUnconfirmedError`, keeps the draft and shows "check Sent before sending again". + - What RN does: when the response has no submission entry, `sendEmail` returns success with an undefined `emailSubmissionId` (`src/api/email.ts:1644-1680`) and the composer closes as sent. + +- [ ] **An open draft does not stay on its own account across an account switch** — `P2` — `bugfix-parity` (1.11.0) + - What RN does: `createDraft`/`sendEmail` resolve the account when they run (`src/api/email.ts:1545,1695`); the composer stays mounted when a notification tap or deep link switches account (`App.tsx:111-115`, `navigation/linking.ts:247`). Autosave and send then go to the new account with the old identity, and the old draft is destroyed in the wrong account. + - Fix hint: capture the account id when the composer opens and pass it through (`ComposeScreen.tsx:744-757`), or close the composer on switch. + +- [ ] **No delivery status notification (DSN) / REQUIRETLS option** — `P3` — `missing` (1.10.0) + - What WEB does: `components/email/email-composer.tsx:615,2565,3410-3420`; reads back `deliveryStatus` (`lib/jmap/client.ts:717-760`). + - What RN does: none in the `sendEmail` envelope (`src/api/email.ts:~1537-1556`); `src/api/jmap-client.ts:1043-1079` already parses `submissionExtensions`. + +- [ ] **A failed filing of the Sent copy is not shown** — `P3` — `bugfix-parity` (1.11.0) + - What RN does: only logs `filingWarning` (`ComposeScreen.tsx:2117-2118`); WEB warns so the mail isn't sent twice. + +- [ ] **No Return-Path note for a From override (#1009)** — `P3` — `missing` (1.11.0) + - What WEB does: says the identity's address shows in the Return-Path, and tries the override as envelope sender with a fallback (`email-composer.tsx:809-817,2383-2392`, `lib/jmap/client.ts:4116-4140`). + - What RN does: `ComposeScreen.tsx:1200`, `src/api/email.ts:1562-1568`. + +- [ ] **Pasted plain-text lists are not turned into real lists** — `P3` — `missing` (1.12.0) + - What WEB does: lines starting with `- `, `* `, `• `, `1. `, `1) ` become lists on paste. + - What RN does: no paste handler in the contenteditable editor (`src/components/RichTextEditor.tsx`, `src/lib/editor-html.ts`). + +- [ ] **No @-mention of a recipient in the body** — `P3` — `missing` (2b5110e, 26cbca7) + - What WEB does: `@` + first name inserts a recipient mention, with a setting to turn it off and screen-reader announcements. + ## Verified at parity (brief list, so the fixer knows what NOT to redo) - Scheduled send via `EmailSubmission` envelope `HOLDFOR` with explicit `rcptTo` (bare addresses, names stripped) and `maxDelayedSend`/FUTURERELEASE capability checks: `src/api/email.ts:828-844`, `src/api/jmap-client.ts:513-531`, `ComposeScreen.tsx:796-821` — matches `lib/jmap/client.ts:592-612`, `:3245-3258`. The capability check read only the session-level object, which Stalwart leaves empty, so scheduling and the undo delay were off on Stalwart until e28e8b4 (#57, audit B8); holds are capped at Stalwart's 7-day limit since 4e7ae2f. - Undo-send delay setting (0/5/10/20/30 s) applied only when the server supports delayed send: `ComposeScreen.tsx:825-828`, `ComposingSettings.tsx:42-48`. @@ -299,6 +341,7 @@ Legend for refs: WEB paths are relative to `the webmail repo`, RN paths to `the - Blob upload tolerant of both Stalwart upload-response shapes: `src/api/blob.ts:38-61`. - Scheduled list filters `undoStatus==='pending'` and future `sendAt`, cancel via `undoStatus:'canceled'` keeping the Sent copy: `src/api/email.ts:902-988`. - Contact prefill from contact/group detail: `ContactDetailScreen.tsx:159`, `GroupDetailScreen.tsx:83`. +- 1.10–1.12 delta: per-message HTML/plain toggle (#1022); exact vs same-domain reply identity (#1000) and delivered-to reply (#991); `$answered` with a send delay (#985); background colour; "Email sent" toast; refused send reported, and the server's real reason shown instead of a dangling reference (cbde644); no replay of submit/import/upload; reply draft keeps its thread; identity Bcc; attachment size limits; scheduled send capped at 7 days; unified-inbox reply from the receiving account (#1104); forward parts copied into the sending account (cd9536d, c0db46d). ## N/A on mobile - Ctrl/Cmd+Enter send, Ctrl+Shift+Enter schedule, `t` template shortcut, Escape close (`email-composer.tsx:2428-2468`). diff --git a/docs/parity/05-calendar.md b/docs/parity/05-calendar.md index 84ab8b21..d67e092c 100644 --- a/docs/parity/05-calendar.md +++ b/docs/parity/05-calendar.md @@ -272,6 +272,53 @@ RN has a solid read path (month/week/agenda, shared-calendar namespacing, per-vi - What RN does: device zone only (`src/api/calendar.ts:15-21`); birthday colour fixed (`src/lib/birthday-calendar.ts:7`). - Fix hint: add `timeZone` to the settings store (synced with WEB's key so it round-trips through settings sync), pass it as the JMAP `timeZone` arg and into saved events; convert display via `Intl` like WEB's `getWallClock`. +## Webmail 1.10.0 → 1.12.0+ delta (audited 2026-10-04) + +Webmail changelog 1.10.0, 1.11.0-beta.1 – 1.11.2 and 1.12.0, plus the +unreleased commits up to `a4e313f` (2026-10-02), checked against native `main` +at `76180b3`. Items already listed in [../audit-2026-09.md](../audit-2026-09.md) +are not repeated. "Unverified" means read from the code but not confirmed on a +device. + +- [ ] **A daily recurring event stops at a DST change** — `P2` — `bugfix-parity` (1.11.0) + - What WEB does: builds each occurrence from the event's local start time (`lib/recurrence-expansion.ts:420-444`). + - What RN does: builds each day from the period start (`src/lib/recurrence-expansion.ts:271-273,401-408,584`). + +- [ ] **Cannot save an event without invitations when the server refuses to send them** — `P2` — `bugfix-parity` (1.11.0) + - What WEB does: detects the refusal (`SchedulingDeniedError`, `lib/jmap/scheduling-error.ts`) and offers to save without sending (`components/calendar/calendar-app.tsx:915-925`). + - What RN does: on Stalwart 0.16.21+ the whole save fails (`src/api/calendar.ts:587-691`, `src/screens/CalendarScreen.tsx:738+`). + +- [ ] **iCal subscriptions are not tied to the login that created them** — `P2` — `bugfix-parity` (1.11.0) + - What WEB does: keys on server URL plus username (`subscriptionOwner`) and clears them on sign-out (`forgetICalSubscriptions`, `stores/calendar-store.ts:494-521,1719-1741`). + - What RN does: keyed on the JMAP account id only, which is unique only per server; old entries without an account id show for every account; nothing is cleared on sign-out (`src/stores/calendar-subscriptions-store.ts:61-70`). + +- [ ] **Server-side invitations, updates and cancellations (`CalendarEventNotification`) are not shown** — `P3` — `missing` (1.10.0; P2 for anyone relying on Stalwart scheduling) + - What WEB does: `stores/calendar-event-notification-store.ts`, `components/layout/calendar-event-notification-toaster.tsx`. + - What RN does: only names the type in `src/api/first-touch-gate.ts:28`. + +- [ ] **No attendee free/busy (`Principal/getAvailability`)** — `P3` — `missing` (1.10.0, WEB `components/calendar/participant-availability.tsx`) + - What RN does: nothing in `ParticipantInput.tsx` / `EventModal.tsx`. + +- [ ] **No default `ParticipantIdentity` for organizing** — `P3` — `missing` (1.10.0, WEB `stores/calendar-store.ts:577-660`, `components/settings/calendar-settings.tsx:23-104`) + - What RN does: the organizer is always `currentUserEmails[0]` (`src/components/calendar/EventModal.tsx:330`). + +- [ ] **Tasks with a due date are not shown in the month view (#1107)** — `P3` — `missing` (1.11.0, WEB `calendar-month-view.tsx`, `task-chip.tsx`, `lib/calendar-tasks.ts`) + - What RN does: tasks only appear in the tasks sheet (`MonthView.tsx`, `MonthScrollView.tsx`). + +- [ ] **Join links in location/description are not detected; location doesn't open maps (#1095)** — `P3` — `missing` (1.12.0) + - What WEB does: detects Teams, Zoom, Meet, Webex, Jitsi, Whereby and GoTo links; location opens in maps with a copy button. + - What RN does: only `virtualLocations` get "Open link" (`src/components/calendar/EventDetailSheet.tsx:187,265-279`); the maps pattern exists in `ContactDetailScreen.tsx:237`. + +- [ ] **Day and week views cannot be limited to working hours and days (#1164)** — `P3` — `missing` (0b4d0b0) + - What RN does: `TimeGridScrollView.tsx`, `WeekView.tsx`, no setting. + +- [ ] **The week view's all-day strip grows without limit; no tasks in it (#1122)** — `P3` — `partial` (1.12.0) + - What WEB does: collapses to 3 rows with a toggle and shows tasks. + - What RN does: `src/components/calendar/WeekView.tsx:96-104`. + +- [ ] **Invitations can carry blank participant names (#748)** — `P3` — `bugfix-parity` (1.12.0) + - What RN does: writes `name: ''` (`src/lib/calendar-participants.ts:284,309`). + ## Verified at parity (brief list, so the fixer knows what NOT to redo) - Client-side recurrence expansion: `src/lib/recurrence-expansion.ts` is a faithful port of `lib/recurrence-expansion.ts` (byX filtering, bySetPosition, RDATE overrides, fast-forward, per-occurrence `utcStart`/`utcEnd` #116); only the server-instance guard differs. - Stalwart singular `recurrenceRule`/`excludedRecurrenceRule` normalisation on read and write (#13, JSCalendar 2.0) — `src/api/calendar.ts:64-126`. @@ -290,6 +337,7 @@ RN has a solid read path (month/week/agenda, shared-calendar namespacing, per-vi - Push StateChange refresh for `Calendar`/`CalendarEvent` on primary + shared accounts — `App.tsx:390-397`, `src/stores/calendar-store.ts:261-284`. - Calendar tab hidden when the account lacks the JMAP calendars capability — `App.tsx:95, 160`, `src/lib/capabilities.ts:25`. - Week start setting, week numbers in month view, 12h/24h time format threaded through views. +- 1.10–1.12 delta: free scrolling (#759); infinite agenda; synthetic occurrence ids (#140); participants deduped and named (#986); linkified descriptions (#968); RSVP incl. a single occurrence (#967, #1086); task progress (#994, #958) and alarms (#504); clear failures reported (#434); UID dedupe on import (#113); refused principal read (#1036); single-occurrence edits; time-zone-correct ranges past 1000 events; declined events struck through (#1110) and outlined; failed fetch keeps calendars; Save shows progress; empty descriptions not sent; UTC-start recurring events probably don't shift (#1119, unverified). ## N/A on mobile - Mouse drag to move/resize, click-drag create, 15-minute snapping, top-edge resize, hover preview (`hooks/use-time-grid-interactions.ts`) — long-press create is the mobile equivalent. diff --git a/docs/parity/06-contacts.md b/docs/parity/06-contacts.md index 21a57355..e34b7874 100644 --- a/docs/parity/06-contacts.md +++ b/docs/parity/06-contacts.md @@ -291,6 +291,25 @@ RN covers the visible surface reasonably well (list with alphabetical index, det - What RN does: filters `useCalendarStore().events` (`src/components/contacts/ContactActivity.tsx:90-91, 127-142`), i.e. only the range the calendar tab happened to load; on a fresh start it is empty. - Fix hint: call the RN calendar API for `[now, now+365d]` with a participant filter, falling back to the cache. +## Webmail 1.10.0 → 1.12.0+ delta (audited 2026-10-04) + +Webmail changelog 1.10.0, 1.11.0-beta.1 – 1.11.2 and 1.12.0, plus the +unreleased commits up to `a4e313f` (2026-10-02), checked against native `main` +at `76180b3`. Items already listed in [../audit-2026-09.md](../audit-2026-09.md) +are not repeated. "Unverified" means read from the code but not confirmed on a +device. + +- [ ] **A contact with calendar, scheduling or free/busy links fails to save** — `P2` — `bugfix-parity` (1.11.0) + - What WEB does: maps them to `calendars` / `schedulingAddresses` / `directories` (`lib/jmap/contact-wire.ts`). + - What RN does: sends flat `calendarUri` / `schedulingUri` / `freeBusyUri`, which Stalwart rejects as "Invalid property" (`src/screens/ContactFormScreen.tsx:458-460`, `src/api/contacts.ts:83-89`); the detail screen reads `contact.calendarUri`, which the server never returns (`ContactDetailScreen.tsx:602+`). + +- [ ] **vCard import sends fields Stalwart rejects** — `P2` — `bugfix-parity` (1.11.0) + - What WEB does: `addressToWire` and the same URI mapping in `contact-wire.ts`. + - What RN does: flat address fields, the URI fields and `source` (`src/lib/vcard.ts:651-669,976-989`, `src/stores/contacts-store.ts:471-479`), so those cards fail to import. + +- [ ] **Deleting an address book that still has contacts fails** — `P2` — `bugfix-parity` (1.11.0, WEB `lib/jmap/client.ts:5822-5835`) + - What RN does: `AddressBook/set` destroy omits `onDestroyRemoveContents` (`src/api/contacts.ts:362-371`), so the server answers `addressBookHasContents`; the comment at `contacts-store.ts:631` wrongly assumes Stalwart deletes the cards. + ## Verified at parity (brief list, so the fixer knows what NOT to redo) - Address book list/create/rename/delete with `myRights` checks (`src/components/settings/ContactsSettings.tsx`, `AddressBookPickerSheet`); inline "New address book…" in the move sheet (WEB #415). - Move contacts between books (single from detail, bulk from list) via `addressBookIds` patch (`contacts-store.ts:237-241`) — RN equivalent of WEB drag-and-drop. @@ -307,6 +326,7 @@ RN covers the visible surface reasonably well (list with alphabetical index, det - Contacts tab hidden when the session lacks `urn:ietf:params:jmap:contacts` (`src/lib/capabilities.ts:29-31`, `App.tsx:170-179`) — WEB 1.8.x "Hide Contacts and Calendars when the account lacks the JMAP capability". - Push `StateChange` for `AddressBook`/`ContactCard` triggers refetch (`contacts-store.ts:152-168`, `App.tsx:390-396`); contacts cache persisted for instant render; selected category persisted. - Contact store reset on logout/account switch (`src/stores/auth-store.ts:58,76,113,179,379`) — the RN counterpart of the 583e20d9 namespacing fix (single-account store, so no id-form flip). +- 1.10–1.12 delta: sort by last name (#963); default address book (#924); `name.full` on every write (#430); new contacts go to the selected book (#940); cleared fields cleared on the server. ## N/A on mobile - Pro multi-account aggregation (`hooks/use-pro-multi-account-contacts.ts`, `fetchAllAccountsContacts`, `::` id namespacing) — Pro shell only; RN keeps one account active and resets the store on switch. diff --git a/docs/parity/07-filters-vacation-files.md b/docs/parity/07-filters-vacation-files.md index 22de4820..a2f88c55 100644 --- a/docs/parity/07-filters-vacation-files.md +++ b/docs/parity/07-filters-vacation-files.md @@ -164,6 +164,41 @@ Filters: RN carries a byte-for-byte port of WEB's Sieve parser/generator/tests a - [x] **README still says "file storage - UI stubs only"** — fixed in c0e1d6b — `P3` — `rn-only-bug` (docs) - `repos/react-native/README.md:30` lists filters, S/MIME, plugins, themes and file storage as stubs; filters, vacation and files are real implementations (`src/api/files.ts`, `src/screens/FilesScreen.tsx`, `src/components/files/ShareSheet.tsx`, `src/api/__tests__/files.test.ts`). Update the README when the items above land. +## Webmail 1.10.0 → 1.12.0+ delta (audited 2026-10-04) + +Webmail changelog 1.10.0, 1.11.0-beta.1 – 1.11.2 and 1.12.0, plus the +unreleased commits up to `a4e313f` (2026-10-02), checked against native `main` +at `76180b3`. Items already listed in [../audit-2026-09.md](../audit-2026-09.md) +are not repeated. "Unverified" means read from the code but not confirmed on a +device. + +- [ ] **Sieve values are not escaped (injection through rule names, header names, sizes)** — `P2` — `bugfix-parity` (1.11.0–1.11.1, security) + - What WEB does: escapes the header name, checks sizes against `^\d+[KMG]?$` and collapses whitespace in rule names (`lib/sieve/generator.ts:47,79,343`). + - What RN does: writes them raw (`src/lib/sieve/generator.ts:46-50,80-108`; `# Rule: ${rule.name}` at `:339`). A newline in a rule name can inject commands such as `redirect`; a name with doubled or trailing spaces is duplicated on every save, because the parser compares the trimmed name (`src/lib/sieve/parser.ts:838-841`). + +- [ ] **"Stop processing" writes no `stop` after "Delete silently" or "Reject"** — `P2` — `bugfix-parity` (cbdc4de) + - What RN does: skips `stop;` when the last action is `discard` or `reject` (`src/lib/sieve/generator.ts:361-364`), so a later rule still files the message. + - Fix hint: only skip when the last action is `stop`. + +- [ ] **Rules from the current webmail break or loosen on a native save** — `P2` — `bugfix-parity` (c55b9f4) + - What WEB does: has a `field: 'all'` condition ("matches every message") and `address_is` / `domain_is` comparators. + - What RN does: the generator throws "Unsupported filter condition field" for `all` (`src/lib/sieve/generator.ts:86-88`), so once such a rule exists every native filter save fails; `address_is` / `domain_is` fall through to `header :contains` (`:107`), silently loosening the rule. Neither is in the UI (`src/lib/sieve/types.ts:23-38`, `FilterRuleModal.tsx:42`). + +- [ ] **Forward actions ignore the server's redirect limit** — `P3` — `missing` (1.11.0, 6da2bf9) + - What WEB does: counts forwards per message against `maxNumberRedirects` (1 on Stalwart) and warns (`lib/filters/forward-limit.ts`, `components/settings/filter-settings.tsx:259-261,540`). + - What RN does: never reads `maxNumberRedirects` (`src/lib/sieve/types.ts:20`); extra forwards are dropped without notice. + +- [ ] **No warning before saving an auto-reply Stalwart would refuse as too long** — `P3` — `missing` (1.11.0) + - What WEB does: subject 511 bytes, body 2047 (`components/settings/vacation-settings.tsx:16,90-98`). + - What RN does: `src/components/settings/VacationSettings.tsx` saves and fails. + +- [ ] **Files: smaller 1.11 fixes not ported** — `P3` — `bugfix-parity` (1.11.0, WEB `lib/jmap/client.ts`) + - A new folder whose name is taken fails instead of becoming "name (2)" (`src/api/files.ts:248-262`, no `onExists: "rename"`; WEB `createFileNodeIn` ~`:8072-8105`). + - Folders cannot be copied (`FilesScreen.tsx:521`, `src/api/files.ts:378-384`). + - Sharing fails on Stalwart before 0.16.6: no mapping to the older rights names (`src/api/files.ts:400-420`; WEB `:40-66,8039`). + - File names Stalwart refuses are not caught before sending. + - `safeMimeType` turns any type over 30 characters into `application/octet-stream` even on 0.16.6+, so office files lose their type (`src/api/files.ts:327-330`; WEB `:8036-8047,8082`). + ## Verified at parity (brief list, so the fixer knows what NOT to redo) - Sieve parser/generator/types are a faithful port up to 1.7.2: `diff -w --strip-trailing-cr lib/sieve/parser.ts repos/react-native/src/lib/sieve/parser.ts` shows only the multi-value/attachment hunks (and `debug.warn` → `console.warn`). Included and identical: `@metadata` JSON round-trip, external-rule parsing with origin labels (Roundcube/Nextcloud/etc., changelog 1.4.14 #201), Nextcloud marker regions preserved verbatim, `# Rule:` dedupe of Bulwark blocks that failed to re-parse (1.7.0 "literal braces" fix, `parser.ts` `findBodyOpenBrace` + `filteredExternal`), vacation-only script detection, `externalRequires` merge, `INBOX` canonical path (1.7.1 #313: `FilterRuleModal.tsx:57`), all 10 action types, `computeRequires` (fileinto/copy/imap4flags/reject/body/vacation), `stop` folding into `stopProcessing`. Since then the webmail writes "Keep" as `fileinto "INBOX"` (62465e1e, #1027) and added the vacation `include`, `:mailboxid` targets, `redirect :copy` and the spam guard; ported in f63d739, d53bfec, 3f88868 and 6c31c8d. - Filter store semantics identical: skip server-managed `vacation` script (1.4.10), keep activation via `onSuccessActivateScript` on create/update (1.4.10), external/opaque rules read-only, bulwark rules kept contiguous before external ones, `addRule/updateRule/deleteRule/reorderRules/toggleRule`, rollback on failed save, opaque banner with "Open raw editor" / two-step "Reset to visual builder", `SieveScript/validate` via uploaded blob with error-method handling, `createSieveScript('filters')`. @@ -172,6 +207,7 @@ Filters: RN carries a byte-for-byte port of WEB's Sieve parser/generator/tests a - Vacation: `VacationResponse/get|set` on singleton with `core+mail+vacationresponse` using, default object when the list is empty, capability gating (`isVacationSupported`), enable toggle + status pill, subject, plain-text body, preview, end-before-start warning, empty-body warning, error surfacing. - Files API: folder detection `blobId == null` (`src/api/files.ts:12-14`), `FileNode/get ids:null`, which Stalwart caps at `maxObjectsInGet` (500), continued since 4eb8c47 by paging `FileNode/query` (which lists folders too since Stalwart 0.16.6) and batched gets (#1069), `shareWith/myRights` requested explicitly, `principals:owner` in `using` only when advertised (sharing is gated on `principals` since caea3d0), real-hierarchy folder create without blob/type/size, cascade delete with `onDestroyRemoveChildren`, MIME-type >30 chars → `application/octet-stream` (1.4.12), cross-account "shared with me" aggregation with `accountId:nodeId` namespacing and owner-routed download URLs, shared subtrees browsable and writes hidden inside them, `Share2`/`Users` badges, ShareSheet with the same read/readWrite/manager presets as WEB `FILE_PRESETS`, custom-rights detection, principal search excluding self, revoke, refresh after change; unit tests in `src/api/__tests__/files.test.ts`. - Files UI: list/grid toggle persisted to settings, folder breadcrumb, pull-to-refresh, hidden-file filter, colored/plain icons, long-press multi-select (own nodes only), rename/new-folder prompts rejecting `/`, delete confirmation naming folder cascade, path stack pruned when a folder disappears, Files tab disabled when the capability is missing. +- 1.10–1.12 delta: "Keep" as `fileinto "INBOX"` (#1027); "keep a copy" on forward; per-rule "also move spam"; filters keep running with the auto-reply on; mark read/star/label on moved mail; rules keep their folder after a rename; every file listed past `maxObjectsInGet` (#1069); numbered names for taken upload names. ## N/A on mobile - Drag-and-drop upload / whole-folder upload (`uploadFolder`, `getDroppedFilesAndFolders`), drag-out of files, marquee selection, keyboard shortcuts (Ctrl+A/C/X/V, F2, Delete, Backspace), right-click context menus, resizable folder-tree sidebar, breadcrumb right-click dropdown, `?preview=` window title updates, `window.open` blob preview safety list (`isMimeTypeSafeForInlinePreview`). diff --git a/docs/parity/08-settings-push-i18n-ui.md b/docs/parity/08-settings-push-i18n-ui.md index f3a62ea1..8a7764a8 100644 --- a/docs/parity/08-settings-push-i18n-ui.md +++ b/docs/parity/08-settings-push-i18n-ui.md @@ -328,6 +328,41 @@ RN toggles that are stored but never read (fix: either wire them or remove the c - What RN does: `AboutDataSettings.tsx:114-134` shows version+commit; update info lives only in the (Android-only) Updates tab; on iOS nothing indicates a newer build exists. - Fix hint: reuse `useUpdatesStore.hasUpdate()` for a pill; on iOS link to TestFlight/App Store instead of Install. +## Webmail 1.10.0 → 1.12.0+ delta (audited 2026-10-04) + +Webmail changelog 1.10.0, 1.11.0-beta.1 – 1.11.2 and 1.12.0, plus the +unreleased commits up to `a4e313f` (2026-10-02), checked against native `main` +at `76180b3`. Items already listed in [../audit-2026-09.md](../audit-2026-09.md) +are not repeated. "Unverified" means read from the code but not confirmed on a +device. + +- [ ] **Push subscriptions lapse after Stalwart's 7-day expiry** — `P2` — `bugfix-parity` (1.11.0) + - What WEB does: renews when the tab becomes visible (`components/push-notification-prompt.tsx:117-135`, `lib/web-push.ts:410-440`). + - What RN does: the renewal check (`src/lib/push-notifications.ts:268`) runs only on sign-in, account or setting change, or token rotation, not on return to the foreground (`App.tsx:641-670,764`), and never for accounts other than the active one. A phone left in the background for a week, or any secondary account, silently stops getting notifications. + +- [ ] **No inbox-only notifications option (#983)** — `P3` — `missing` (1.11.0, WEB `lib/web-push.ts:90-135`, `pushNotifyInboxOnly`) + - What RN does: every non-junk folder notifies (`src/lib/push-notifications.ts:300-330`). + +- [ ] **One message reaching several accounts rings once per account** — `P3` — `missing` (cd39805, 868805e) + - What WEB does: rings once for a burst across accounts, and again for a second mail in the same account. + - What RN does: each child notification alerts (`GROUP_ALERT_CHILDREN`, `android/.../BulwarkFcmModule.kt:142,191`; `src/lib/push-background-task.ts`). + +- [ ] **A failed preview lookup drops the notification** — `P3` — `bugfix-parity` (4bc5d48) + - What WEB does: shows a generic "New mail". + - What RN does: `src/lib/push-background-task.ts:340-378` returns `[]` on any method error. + +- [ ] **Folder deep links ignore the folder they name** — `P3` — `partial` + - What RN does: only opens the Mail tab (`src/navigation/linking.ts:285`), so WEB's folder-link fixes (1d82c59, 1c5dbbc) have nothing to land on. + +- [ ] **"Free scrolling" and the automatic time-zone setting are missing from settings search** — `P3` — `partial` (1.10.0) + - What RN does: `calendar.settings.calendar_free_scroll` and `time_zone_auto_zone` are not in `src/lib/settings-search.ts:102-113`. + +- [ ] **About card links the repo, not the running build's commit** — `P3` — `missing` (0e39b19) + - What RN does: `src/components/settings/AboutDataSettings.tsx:211`. + +- [ ] **RTL: the drawer may slide in from the wrong side (#944)** — `P3` — `bugfix-parity` (1.10.0, unverified on device) + - What RN does: fixed negative `translateX` with no RTL check (`src/components/SidebarDrawer.tsx:683,705`). + ## Verified at parity (do not redo) - Per-account push registry keys, legacy key migration, in-flight coalescing, relay register/verify/active/unregister endpoint usage, 90-day expiry with 7-day refresh, relay-confirmed-dead reaping: `RN: src/lib/push-notifications.ts:16-95,271-298,342-383,411-439` match `lib/web-push.ts:16-25,334-389,448-489` and `repos/relay/README.md` endpoints. @@ -341,6 +376,7 @@ RN toggles that are stored but never read (fix: either wire them or remove the c - Confirm dialog styling mirrors WEB confirm-dialog (`Dialog.tsx:19-27`); undo snackbar timer is `createdAt`-based so re-renders do not reset it. - Offline detection via NetInfo with reachability fallback; outbox flush on reconnect (`App.tsx:289-295`). - Hardware back closes a settings pane (`SettingsScreen.tsx:192-199`); settings groups/tabs mirror WEB's six groups. +- 1.10–1.12 delta: shared/group push previews (#839); update status after an upgrade; push opens in its own account and previews the named message, never the newest unread (39efd8a); notification tap waits for the account switch; quick switches don't mix identities. ## N/A on mobile diff --git a/docs/parity/09-jmap-core-sync-security.md b/docs/parity/09-jmap-core-sync-security.md index 0adea2cf..2158ce98 100644 --- a/docs/parity/09-jmap-core-sync-security.md +++ b/docs/parity/09-jmap-core-sync-security.md @@ -288,6 +288,18 @@ RN's JMAP client (`src/api/jmap-client.ts`, 615 lines) is a thin transport: sess - What RN does: `jmapPost`/`fetchInboxForAccount` do a bare `secureFetch` per account and per shared account in parallel (`src/api/unified-inbox.ts:116-131`, `215-233`), swallowing shared-account errors and only mapping 401 to "Session expired". - Fix hint: route through the same `request()` helper once it has deadlines/back-off (pass explicit credentials instead of the singleton). +## Webmail 1.10.0 → 1.12.0+ delta (audited 2026-10-04) + +Webmail changelog 1.10.0, 1.11.0-beta.1 – 1.11.2 and 1.12.0, plus the +unreleased commits up to `a4e313f` (2026-10-02), checked against native `main` +at `76180b3`. Items already listed in [../audit-2026-09.md](../audit-2026-09.md) +are not repeated. "Unverified" means read from the code but not confirmed on a +device. + +- [ ] **An `Email/set` create can be replayed after a dropped connection** — `P3` — `partial` (1.11.0) + - What WEB does: never replays a request that creates mail. + - What RN does: submit, import and upload are never replayed (`src/api/jmap-client.ts:101-108,884`), but an `Email/set` create still can be, which can leave a duplicate draft or Sent copy. + ## Verified at parity (brief list, so the fixer knows what NOT to redo) - Session discovery, `primaryAccounts` selection (mail -> core -> first account), shared/group account detection (`src/api/jmap-client.ts:388-401`, `480-493` vs `lib/jmap/client.ts:988-995`, `4421-4463`). - Per-capability account ids for Sieve and Files (`src/api/sieve.ts:20-23`, `src/api/files.ts:27-31`); calendar/contacts use the primary account on both sides in practice. @@ -309,6 +321,7 @@ RN's JMAP client (`src/api/jmap-client.ts`, 615 lines) is a thin transport: sess - No secrets in `console.*` output; `usesNonExemptEncryption: false` declared (`app.config.js:370`). - Stalwart self-service via `urn:stalwart:jmap`: password change, display name, TOTP enable/disable, app passwords and API keys with IP allow-list, principal read (`src/api/account-security.ts`), with proper `error`/`notUpdated` checks. - SHA-256 implementation correct (FIPS 180-4) and APK size + optional checksum verification (`src/lib/sha256.ts`, `src/lib/install-update.ts:100-127`). +- 1.10–1.12 delta: assertSetResult on mutations (#956); fallback poll stays within the per-request call limit (`src/api/push-stream.ts:115-121`, but it doesn't cover shared accounts). ## N/A on mobile - Server-side SSRF/DNS-rebinding guard, endpoint allow-list, `OAUTH_ALLOW_PRIVATE_ENDPOINTS`, IPv6 transition-address checks (`lib/security/url-guard.ts`, `lib/stalwart/server-fetch.ts`; GHSA-24w9): RN talks to the server directly from the device; the only analogue is scheme validation (covered above). diff --git a/docs/superpowers/plans/2026-10-04-parity-phase-1-security-send.md b/docs/superpowers/plans/2026-10-04-parity-phase-1-security-send.md new file mode 100644 index 00000000..c71acb1f --- /dev/null +++ b/docs/superpowers/plans/2026-10-04-parity-phase-1-security-send.md @@ -0,0 +1,631 @@ +# Parity Phase 1: Security and Send Correctness Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Close the three P1s and five security/send P2s from the webmail 1.10–1.12 delta. When this is done, no header, display name, unsubscribe link, Sieve value or `winmail.dat` a sender controls can fool the user or redirect mail, and the app never reports "sent" when nothing went out. + +**Architecture:** Every task ports a fix the webmail already shipped, into the matching native module: `src/lib/*` for pure parsing, `src/api/email.ts` for the send request, `src/lib/sieve/*` for the filter script, and the screens/components that surface the results. The pure functions get unit tests. The UI changes stay thin and reuse one shared send-error helper. + +**Tech Stack:** React Native / Expo, TypeScript, Zustand stores, vitest (`npm test`), JMAP (RFC 8620/8621) against Stalwart. + +**Spec:** the Phase 1 rows of [2026-10-04-webmail-parity-roadmap.md](2026-10-04-webmail-parity-roadmap.md). Each row's finding, with WEB and RN pointers, is in the "Webmail 1.10.0 → 1.12.0+ delta" section of [docs/parity/03-email-viewer.md](../../parity/03-email-viewer.md), [04-composer-send.md](../../parity/04-composer-send.md) and [07-filters-vacation-files.md](../../parity/07-filters-vacation-files.md). + +## Global Constraints + +- **Webmail reference checkout:** `git clone https://github.com/bulwarkmail/webmail /tmp/webmail && git -C /tmp/webmail checkout a4e313f`. Below, "WEB `path`" means a path in that checkout. +- **Gate on every commit:** `npm run typecheck && npm test && npm run i18n:check` all pass. +- **User-visible strings:** use `t('key', 'English fallback')`, with the webmail key when the plan names one. Run `npm run i18n:harvest` when a key is new to both catalogs. +- **One commit per task:** `fix: …` or `feat: …` in the style of `git log`, with the trailer `Co-Authored-By: Claude Opus 5.5 `. +- **Tick the finding in the same commit:** change its `- [ ]` to `- [x]` in its `docs/parity/*.md` delta section and append ` — fixed in `. Do this by amending the task's commit after it exists (`git commit --amend --no-edit`), or put the hash in a follow-up `docs:` commit. Also tick the matching line in the `PARITY_CHECKLIST.md` P1 list for Tasks 1–3. +- **Keep scripts compatible with webmail:** a Sieve script saved by either client must read back on the other. Match WEB's generator output byte for byte where WEB's tests pin it. + +## Review Focus + +1. **A message with one `Authentication-Results` header and no forged one** must show exactly what it shows today. Covered by the existing `email-headers.test.ts` cases, which must stay green unchanged (Task 1). +2. **A send whose read-back (`EmailSubmission/get`) errors out or is missing** (older Stalwart, or a server without `deliveryStatus`) must count as a plain success, not a failure (Task 3, test `treats a missing or failed read-back as a plain success`). +3. **A held (undo-send) send with all recipients refused** must fail like an immediate one, and must not leave an undo bar for a message that never went out (Task 3, delayed-send test; Task 4 relies on the throw happening before `recordHeldSend`). +4. **An existing filter script saved by native before this change** (with `stop;` written after a move, and rule names that are already single-spaced) must regenerate byte-identically (Task 5, test `leaves a plain script byte-identical`). +5. **A plain `mailto:` link clicked in a message body** (not the unsubscribe banner) must still open the composer with all its addresses: only the banner gets the strict parser (Task 8, test `parseMailtoUrl still accepts several addresses`). + +--- + +### Task 1: Read DKIM, DMARC and iprev from the topmost Authentication-Results header only + +**Files:** +- Modify: `src/lib/email-headers.ts:106-168` (`parseAuthenticationResults`), `:259-264` (`deriveHeaderInfo`) +- Test: `src/lib/__tests__/email-headers.test.ts` + +**Interfaces:** +- Produces: `parseAuthenticationResults(headers: string | readonly string[]): AuthenticationResults`, the same result type as today. A single string keeps working as a one-header list. + +- [ ] **Step 1: Write the failing tests** (add to the `parseAuthenticationResults` describe) + +```ts +it('takes DKIM and DMARC only from the topmost header', () => { + const r = parseAuthenticationResults([ + 'mx.example; spf=fail smtp.mailfrom=evil.example; dmarc=fail header.from=bank.example', + 'evil.example; dkim=pass header.d=bank.example; dmarc=pass header.from=bank.example', + ]); + expect(r.dmarc?.result).toBe('fail'); + expect(r.dkim).toBeUndefined(); +}); + +it('ignores results inside comments and property values', () => { + const r = parseAuthenticationResults( + 'mx.example; spf=pass smtp.mailfrom="dmarc=pass"@x.example (dkim=pass header.d=bank.example); dmarc=fail', + ); + expect(r.dmarc?.result).toBe('fail'); + expect(r.dkim).toBeUndefined(); + expect(r.spf?.result).toBe('pass'); +}); + +it('lets a lower header escalate SPF to a failure but never supply a pass', () => { + expect(parseAuthenticationResults(['mx; spf=none smtp.mailfrom=a.example', 'x; spf=fail smtp.mailfrom=a.example']).spf?.result).toBe('fail'); + expect(parseAuthenticationResults(['mx; spf=fail smtp.mailfrom=a.example', 'x; spf=pass smtp.mailfrom=a.example']).spf?.result).toBe('fail'); + expect(parseAuthenticationResults(['mx; dkim=none', 'x; spf=pass smtp.mailfrom=a.example']).spf).toBeUndefined(); +}); +``` + +Also add one `deriveHeaderInfo` test. The input is `headers` with two `Authentication-Results` entries: the first `mx; spf=pass smtp.mailfrom=a.example`, the second `x; dmarc=pass`. Assert `info.auth?.dmarc` is `undefined`. + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `npx vitest run src/lib/__tests__/email-headers.test.ts` +Expected: the four new tests FAIL (dmarc reads `pass`, or dkim is defined). The existing tests pass. + +- [ ] **Step 3: Implement** + +Port WEB `lib/email-headers.ts` `splitResinfo`, `parseResinfo`, `parseResinfos`, `METHOD_RE`, `PROP_RE`, `DMARC_SEVERITY` and the new `parseAuthenticationResults` body unchanged. The rule: DKIM, DMARC and iprev come from `perHeader[0]` only. SPF entries from the lower headers can only raise the severity to a failure (`severity >= SPF_SEVERITY.temperror`), and are never taken when they would be a pass. Keep the native `SpfEntry`/`AuthenticationResults` types and the `all` array. + +In `deriveHeaderInfo`, pass `authHeaders` (the array, already in message order) instead of `authHeaders.join('; ')`, and fix the comment there. + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `npx vitest run src/lib/__tests__/email-headers.test.ts` +Expected: PASS, including every pre-existing case. + +- [ ] **Step 5: Commit** + +```bash +git add src/lib/email-headers.ts src/lib/__tests__/email-headers.test.ts docs/parity/03-email-viewer.md PARITY_CHECKLIST.md +git commit -m "fix: take DKIM and DMARC results only from the receiving server's own header" +``` + +--- + +### Task 2: Treat an escaped quote in a display name as part of the name + +**Files:** +- Modify: `src/lib/recipients.ts` (`angleRunCloses` ~:58, `splitRecipients` :80, `findTopLevelColon` ~:155) +- Test: `src/lib/__tests__/recipients.test.ts` + +**Interfaces:** +- Produces: no signature changes. + +- [ ] **Step 1: Write the failing tests** + +```ts +it('keeps an escaped quote inside the display name (no extra recipient)', () => { + const input = '"Support\\", ceo@corp.example, \\"x" '; + expect(splitRecipients(input)).toEqual([input]); +}); + +it('round-trips a name containing quotes and commas', () => { + const name = 'Support", ceo@corp.example, "x'; + const formatted = formatRecipient(name, 'support@shop.example'); + const parts = splitRecipients(`${formatted}, other@example.com`); + expect(parts).toHaveLength(2); + expect(parseRecipient(parts[0])).toEqual({ name, email: 'support@shop.example' }); +}); + +it('does not open a group on a colon after an escaped quote', () => { + const r = parseRecipient('"a\\": b" '); + expect(r.group).toBeUndefined(); + expect(r.email).toBe('a@example.com'); +}); +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `npx vitest run src/lib/__tests__/recipients.test.ts` +Expected: the new tests FAIL. The first one splits into 3 entries. + +- [ ] **Step 3: Implement** + +The rule is the same in all three scanners: when the scanner is inside quotes and sees `\`, it consumes that character and the next one as a single quoted-pair (WEB `lib/email-composer-utils.ts:283-289`), so an escaped `"` never toggles `inQuotes`. In `splitRecipients`, append both characters to `current`. + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `npx vitest run src/lib/__tests__/recipients.test.ts` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add src/lib/recipients.ts src/lib/__tests__/recipients.test.ts docs/parity/04-composer-send.md PARITY_CHECKLIST.md +git commit -m "fix: keep an escaped quote inside a recipient's display name" +``` + +--- + +### Task 3: Read the submission's deliveryStatus back, and refuse to call an unconfirmed send a success + +**Files:** +- Modify: `src/api/jmap-result.ts` (next to `ScheduleTooLateError`, ~:84) +- Modify: `src/api/email.ts:1410-1419` (`SendEmailResult`), `:1535-1680` (`sendEmail`) +- Test: `src/api/__tests__/email-send-confirmation.test.ts` (new). Use the `vi.mock('../jmap-client', …)` setup from `src/api/__tests__/email-submission.test.ts:3-14`. +- Modify: existing tests that pin the exact method-call list of a send, so they expect the third call. Find them with `grep -rn "EmailSubmission/set" src/api/__tests__`. + +**Interfaces:** +- Produces, in `src/api/jmap-result.ts`: + - `interface RejectedRecipient { email: string; smtpReply: string }` + - `function rejectedRecipients(deliveryStatus: Record | null | undefined): { rejected: RejectedRecipient[]; all: boolean }` + - `function formatRejectedRecipients(recipients: RejectedRecipient[]): string` produces `"a@x (550 5.1.2 …), b@y"` + - `class RecipientsRejectedError extends Error { readonly recipients: RejectedRecipient[] }` with `name = 'RecipientsRejectedError'` + - `class SendUnconfirmedError extends Error` with `name = 'SendUnconfirmedError'` +- Produces: `SendEmailResult.rejectedRecipients?: RejectedRecipient[]` (set only when some, not all, were refused). + +- [ ] **Step 1: Write the failing tests** + +Helper: `respond(extra)` resolves `mockRequest` with an `Email/set` created `{ draft: { id: 'email-9' } }` (call id `'0'`), an `EmailSubmission/set` created `{ 'sub-1': { id: 'sub-9' } }` (`'1'`), and then `extra` entries. Destroy calls go through `mockRequest` as well; assert on them through `mockRequest.mock.calls`. + +```ts +it('asks for the new submission deliveryStatus in the send request', async () => { + respond([['EmailSubmission/get', { list: [{ id: 'sub-9', deliveryStatus: {} }] }, 'deliveryStatus']]); + await sendEmail(OUTGOING, 'id-1', 'sent-1'); + expect(mockRequest.mock.calls[0][0][2]).toEqual( + ['EmailSubmission/get', { accountId: 'acc-1', ids: ['#sub-1'], properties: ['deliveryStatus'] }, 'deliveryStatus'], + ); +}); + +it('returns the refused recipients when the others were accepted', async () => { + respond([['EmailSubmission/get', { list: [{ deliveryStatus: { + 'ok@example.com': { delivered: 'queued', smtpReply: '250 2.1.5 OK' }, + 'gone@example.com': { delivered: 'no', smtpReply: '550 5.1.1 No such user' }, + } }] }, 'deliveryStatus']]); + const result = await sendEmail(OUTGOING, 'id-1', 'sent-1'); + expect(result.rejectedRecipients).toEqual([{ email: 'gone@example.com', smtpReply: '550 5.1.1 No such user' }]); +}); + +it('fails the send, removes the filed copy and keeps the old draft when every recipient was refused', async () => { + respond([['EmailSubmission/get', { list: [{ deliveryStatus: { + 'gone@example.com': { delivered: 'no', smtpReply: '550 5.1.1 No such user' }, + } }] }, 'deliveryStatus']]); + mockRequest.mockResolvedValueOnce({ methodResponses: [['Email/set', { destroyed: ['email-9'] }, '0']] }); + const err = await sendEmail(OUTGOING, 'id-1', 'sent-1', undefined, { draftId: 'draft-1' }).catch((e) => e); + expect(err).toBeInstanceOf(RecipientsRejectedError); + expect(destroyedIds()).toEqual(['email-9']); // never 'draft-1' +}); + +it('also fails a held send whose recipients were all refused', async () => { /* holdForSeconds = 30, same expectation */ }); + +it('throws SendUnconfirmedError when the response has no EmailSubmission/set', async () => { + mockRequest.mockResolvedValueOnce({ methodResponses: [['Email/set', { created: { draft: { id: 'email-9' } } }, '0']] }); + await expect(sendEmail(OUTGOING, 'id-1', 'sent-1')).rejects.toBeInstanceOf(SendUnconfirmedError); + expect(destroyedIds()).toEqual([]); // the copy may be the only record that it went out +}); + +it('treats a missing or failed read-back as a plain success', async () => { + respond([['error', { type: 'unknownMethod' }, 'deliveryStatus']]); + await expect(sendEmail(OUTGOING, 'id-1', 'sent-1')).resolves.toMatchObject({ emailSubmissionId: 'sub-9' }); + respond([]); + await expect(sendEmail(OUTGOING, 'id-1', 'sent-1')).resolves.toMatchObject({ emailSubmissionId: 'sub-9' }); +}); + +it('reports a refused submission by its own error, not the dangling read-back', async () => { + mockRequest.mockResolvedValueOnce({ methodResponses: [ + ['Email/set', { created: { draft: { id: 'email-9' } } }, '0'], + ['EmailSubmission/set', { notCreated: { 'sub-1': { type: 'forbiddenFrom', description: 'Not allowed' } } }, '1'], + ['error', { type: 'invalidResultReference' }, 'deliveryStatus'], + ] }); + await expect(sendEmail(OUTGOING, 'id-1', 'sent-1')).rejects.toThrow('Not allowed'); +}); +``` + +Add unit tests for `rejectedRecipients` (an empty map gives `{ rejected: [], all: false }`; all refused gives `all: true`) and for `formatRejectedRecipients`. + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `npx vitest run src/api/__tests__/email-send-confirmation.test.ts` +Expected: FAIL. The imports don't exist yet. + +- [ ] **Step 3: Implement** + +- In `src/api/jmap-result.ts`, add the interfaces, helpers and errors listed above. Copy the text from WEB `lib/jmap/client.ts:756-796`, including the `SendUnconfirmedError` message. +- In `sendEmail`, append the read-back as the third method call: `['EmailSubmission/get', { accountId, ids: ['#sub-1'], properties: ['deliveryStatus'] }, 'deliveryStatus']`. This is a creation-id reference, not a `#ids` result reference; WEB `client.ts:719-731` explains why. +- In the response loop, set aside every entry whose call id is `'deliveryStatus'` before the existing handling, so an `error` from the read-back is never taken for a failed send or a filing problem. Keep its `list[0].deliveryStatus` only when the method name is `EmailSubmission/get`. +- After the loop, in this order: + 1. An existing `failure` keeps today's path. + 2. If there is no `emailSubmissionId`, throw `new SendUnconfirmedError()` without destroying anything. + 3. If `rejectedRecipients(...).all`, run the same cleanup as the failure path (destroy `emailId`, leave `opts.draftId` alone) and throw `RecipientsRejectedError`. + 4. Otherwise, return `rejectedRecipients` on the result when the list is non-empty. +- Step 3 must come before the old-draft cleanup at `:1664`. + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `npx vitest run src/api` +Expected: PASS, including the updated call-list assertions in the existing suites. + +- [ ] **Step 5: Commit** + +```bash +git add src/api/jmap-result.ts src/api/email.ts src/api/__tests__/ docs/parity/04-composer-send.md PARITY_CHECKLIST.md +git commit -m "fix: fail a send the server refused for every recipient, and never call an unconfirmed send sent" +``` + +--- + +### Task 4: Tell the user about refused recipients and unconfirmed sends + +**Files:** +- Create: `src/lib/send-errors.ts` +- Modify: `src/screens/ComposeScreen.tsx:2105-2187` (send success branch and `catch`) +- Modify: `src/components/email/QuickReplyBox.tsx:102-142` +- Test: `src/lib/__tests__/send-errors.test.ts` + +**Interfaces:** +- Consumes: `RecipientsRejectedError`, `SendUnconfirmedError`, `formatRejectedRecipients`, `SendEmailResult.rejectedRecipients` (Task 3); `RequestTimeoutError` (existing, `src/api/jmap-client.ts:1278`) and `ScheduleTooLateError` (existing, `src/api/jmap-result.ts:84`). +- Produces: `sendErrorAlert(e: unknown, t: (key: string, fallback?: string) => string): { title: string; message: string }`. + +- [ ] **Step 1: Write the failing tests** (`t` is `(k, f) => f ?? k`) + +```ts +it('lists the refused recipients with their SMTP replies', () => { + const e = new RecipientsRejectedError([{ email: 'gone@example.com', smtpReply: '550 5.1.1 No such user' }]); + expect(sendErrorAlert(e, t)).toEqual({ + title: 'Not sent - the server rejected every recipient.', + message: 'gone@example.com (550 5.1.1 No such user)', + }); +}); +it('points at Sent for a timeout and for an unconfirmed send', () => { + const expected = { + title: 'No answer from the server', + message: 'The message may already have gone out. Check your Sent folder before sending it again.', + }; + expect(sendErrorAlert(new RequestTimeoutError(), t)).toEqual(expected); + expect(sendErrorAlert(new SendUnconfirmedError(), t)).toEqual(expected); +}); +it('keeps the schedule-too-late copy and falls back to the error message', () => { /* … */ }); +``` + +Construct `RequestTimeoutError` the way `src/api/__tests__/jmap-client-hardening.test.ts` does. + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `npx vitest run src/lib/__tests__/send-errors.test.ts` +Expected: FAIL (module missing). + +- [ ] **Step 3: Implement** + +- `sendErrorAlert` maps each error to an alert title and message: + - `RecipientsRejectedError` → key `email_composer.send_recipients_rejected` (the webmail key), with the formatted list as the message. + - `RequestTimeoutError` and `SendUnconfirmedError` → the existing `email_composer.send_timeout_title` / `send_timeout_body` copy. + - `ScheduleTooLateError` → the existing `schedule_too_late_*` copy. + - Anything else → `email_composer.send_failed` plus `e.message`. +- `ComposeScreen`: replace the three `catch` branches with `const { title, message } = sendErrorAlert(e, t); Alert.alert(title, message)`. A `RecipientsRejectedError` keeps the composer open, as every failure already does. +- `ComposeScreen` success path: after `sendEmail` returns and before the existing toast/undo branches, add one call when `result.rejectedRecipients?.length` is non-zero: `toast.warning(t('email_composer.send_some_recipients_rejected', 'Sent, but not to these recipients - the server rejected them.'), formatRejectedRecipients(result.rejectedRecipients))`. +- `QuickReplyBox`: use `sendErrorAlert` in its `catch`, and show the same warning toast on success. + +- [ ] **Step 4: Run the tests and the i18n check** + +Run: `npx vitest run src/lib/__tests__/send-errors.test.ts src/components/__tests__ && npm run i18n:check` +Expected: PASS. If `i18n:check` lists `send_recipients_rejected` / `send_some_recipients_rejected`, run `npm run i18n:harvest` and include `locales/rn/en.json`. They become webmail keys again at the next `sync-locales`. + +- [ ] **Step 5: Commit** + +```bash +git add src/lib/send-errors.ts src/lib/__tests__/send-errors.test.ts src/screens/ComposeScreen.tsx src/components/email/QuickReplyBox.tsx locales/rn/en.json docs/parity/04-composer-send.md +git commit -m "fix: say which recipients the server refused, and point at Sent when a send is unconfirmed" +``` + +Device check: on Stalwart, send to `nobody@` alone. You should see the alert, the composer should stay open, and nothing should appear in Sent. Then send to it plus a real address: you should see the warning toast and the message in Sent. + +--- + +### Task 5: Escape every value written into the Sieve script, and keep spaced rule names single + +**Files:** +- Modify: `src/lib/sieve/generator.ts:44-50` (size), `:80-82` (header name), `:339` (`# Rule:` line) +- Modify: `src/lib/sieve/parser.ts:836-841` (matching a block to its rule by name) +- Test: `src/lib/sieve/__tests__/generator.test.ts`, `src/lib/sieve/__tests__/parser.test.ts` + +**Interfaces:** +- Produces: no signature changes. + +- [ ] **Step 1: Write the failing tests** + +```ts +// generator.test.ts +it('writes a rule name on one line with its whitespace collapsed', () => { + const script = generateScript([makeRule({ name: 'Foo Bar\nredirect "x@evil.example";' })]); + expect(script).toContain('# Rule: Foo Bar redirect "x@evil.example";\n'); + expect(script).not.toMatch(/^redirect/m); +}); +it('escapes a custom header name', () => { + const script = generateScript([makeRule({ conditions: [{ field: 'header', headerName: 'X-A" :contains "B', comparator: 'contains', value: 'v' }] })]); + expect(script).toContain('header :contains "X-A\\" :contains \\"B" "v"'); +}); +it('writes only a number with an optional K/M/G as a size, else 0', () => { + const size = (value: string) => generateScript([makeRule({ conditions: [{ field: 'size', comparator: 'greater_than', value }] })]); + expect(size('10M')).toContain('size :over 10M'); + expect(size('1; redirect "x@evil.example"')).toContain('size :over 0'); +}); +it('leaves a plain script byte-identical', () => { + // Generate a fixture from today's main (a move + stop rule with a single-spaced name) and compare. +}); + +// parser.test.ts +it('reads a rule whose name has doubled or trailing spaces back as the same rule', () => { + const rules = [makeRule({ name: 'Foo Bar ' })]; + const parsed = parseScript(generateScript(rules)); + expect(parsed.rules.filter((r) => r.origin !== 'bulwark')).toEqual([]); + expect(parsed.rules).toHaveLength(1); +}); +``` + +The byte-identical fixture comes from WEB `lib/sieve/__tests__/generator.test.ts` (the `cbdc4de` hunk). Copy WEB's `generator-injection.test.ts` cases that apply to these three fields as well. + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `npx vitest run src/lib/sieve` +Expected: the new tests FAIL. + +- [ ] **Step 3: Implement** + +- Size: copy WEB `generator.ts` exactly. Take the trimmed raw value, test it against `/^\d+[KMG]?$/i`, and use `'0'` when it doesn't match. +- Header name: wrap it in `escapeString(...)`. +- Rule line: `# Rule: ${rule.name.replace(/\s+/g, ' ')}`. +- Parser: match `/#\s*Rule:[ \t]*(.*?)[ \t]*$/m`, and compare both sides through `oneLine = (s) => s.replace(/\s+/g, ' ').trim()` (WEB `cbdc4de`, `parser.ts:847-858`). + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `npx vitest run src/lib/sieve` +Expected: PASS, including `webmail-fixtures.test.ts`. + +- [ ] **Step 5: Commit** + +```bash +git add src/lib/sieve docs/parity/07-filters-vacation-files.md +git commit -m "fix: escape header names, sizes and rule names in the filter script" +``` + +--- + +### Task 6: Stop after discard/reject, and understand the webmail's newer conditions + +**Files:** +- Modify: `src/lib/sieve/types.ts:23-38` +- Modify: `src/lib/sieve/generator.ts` (`generateCondition` :43-108, stop logic :361-364) +- Modify: `src/lib/sieve/parser.ts` (`parseAtom` ~:335) +- Modify: `src/lib/sieve/condition-value.ts:26-48` +- Test: `src/lib/sieve/__tests__/generator.test.ts`, `parser.test.ts`, `condition-value.test.ts` + +**Interfaces:** +- Produces: + - `FilterConditionField` gains `'all'`. + - `FilterComparator` gains `'address_is' | 'domain_is' | 'any'`. An `'all'` condition is always `{ field: 'all', comparator: 'any', value: '' }`. + - `isValueLessCondition(cond: FilterCondition): boolean` in `condition-value.ts` is true for `has_any` and for `field === 'all'`. It replaces the `isHasAnyCondition` uses in `formatConditionValue`. Keep `isHasAnyCondition` exported for the modal until Task 7. + +- [ ] **Step 1: Write the failing tests** + +Copy WEB's `describe('all messages')` block from `lib/sieve/__tests__/generator.test.ts` (commit `c55b9f4`) unchanged. It pins `if true {`, `allof(true, )`, the `anyof`/`allof` mixes, `require ["imap4flags"];` and the read-back. Then add: + +```ts +it('writes a stop after discard and reject when the rule says stop', () => { + for (const type of ['discard', 'reject'] as const) { + const script = generateScript([makeRule({ stopProcessing: true, actions: [{ type, value: 'no' }] })]); + expect(script).toMatch(new RegExp(`${type}[^\\n]*;\\n stop;\\n}`)); + } +}); +it('writes no second stop when the last action is already stop', () => { + const script = generateScript([makeRule({ stopProcessing: true, actions: [{ type: 'stop' }] })]); + expect(script.match(/stop;/g)).toHaveLength(1); +}); +it('writes address_is and domain_is as address tests', () => { + const s = generateScript([makeRule({ conditions: [ + { field: 'from', comparator: 'address_is', value: 'anna@acme.com' }, + { field: 'to', comparator: 'domain_is', value: 'acme.com' }, + ] })]); + expect(s).toContain('address :is "From" "anna@acme.com"'); + expect(s).toContain('address :domain :is "To" "acme.com"'); +}); +``` + +Parser: copy WEB `lib/sieve/__tests__/address-comparators.test.ts`. It covers metadata-less scripts and the reversed tag order `:is :domain`. In `condition-value.test.ts`, assert `describeCondition({ field: 'all', comparator: 'any', value: '' }, t)` is `'All messages'`, with a `t` that returns the fallback. + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `npx vitest run src/lib/sieve` +Expected: FAIL. `'all'` currently throws "Unsupported filter condition field". + +- [ ] **Step 3: Implement** + +- `generateCondition`: start with `if (field === 'all') return 'true';`. Before the `switch`, when the comparator is `address_is`/`domain_is` and the field is in `ADDRESS_FIELDS = new Set(['from', 'to', 'cc'])`, return `address ${part}:is "${headerName}" ${formatStringArg(values)}`. Use the same text as WEB `generator.ts:84-92`. +- Stop: `if (rule.stopProcessing && !actionLines.includes('stop;')) actionLines.push('stop;')`. +- Parser: port the `address` atom from WEB `parser.ts:456-470`. Also map a bare `true` atom to the `'all'` condition, so a script without metadata reads back too. +- `describeCondition`: for `field === 'all'`, return just the field label, using `settings.filters.condition_fields.all` ("All messages"). + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `npx vitest run src/lib/sieve && npm run typecheck` +Expected: PASS. Typecheck may flag exhaustive switches over `FilterComparator`/`FilterConditionField` in the UI. Give `'all'` and the new comparators sensible labels there; Task 7 adds the real UI. + +- [ ] **Step 5: Commit** + +```bash +git add src/lib/sieve docs/parity/07-filters-vacation-files.md +git commit -m "fix: stop after a silent delete or reject, and read the webmail's all-messages and address rules" +``` + +--- + +### Task 7: Offer "All messages", "is the address" and "has the domain" in the rule editor + +**Files:** +- Modify: `src/components/filters/FilterRuleModal.tsx:42-50` (field and comparator lists), plus its condition row and save validation +- Test: `src/components/__tests__/filter-rule-modal.test.tsx` (new, or extend an existing component test if one covers the modal; check with `grep -rln FilterRuleModal src/components/__tests__`) + +**Interfaces:** +- Consumes: `isValueLessCondition` (Task 6). +- Produces: none. + +- [ ] **Step 1: Write the failing tests** + +The test file should cover: +- Choosing the field "All messages" hides the comparator and value inputs and saves `{ field: 'all', comparator: 'any', value: '' }`. +- A rule with only that condition is saveable: the empty value is not flagged. +- For From/To/Cc the comparator list ends with `address_is` and `domain_is`. For Subject it doesn't include them. +- Opening a rule that already has an `'all'` condition shows "All messages" and doesn't crash. + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `npx vitest run src/components/__tests__/filter-rule-modal.test.tsx` +Expected: FAIL. + +- [ ] **Step 3: Implement** + +- Add `'all'` at the end of `ALL_FIELDS`, as webmail does. +- `comparatorsFor('all')` returns `['any']`. For `from`/`to`/`cc`, return `[...TEXT_COMPARATORS, 'address_is', 'domain_is']`. +- Switching a row to `'all'` resets it to `{ field: 'all', comparator: 'any', value: '' }`. +- Use `isValueLessCondition` wherever the modal hides the value input or skips the empty-value check. +- Labels: `settings.filters.condition_fields.all`, `settings.filters.comparators.address_is`, `settings.filters.comparators.domain_is` (webmail keys). + +- [ ] **Step 4: Run the tests and the i18n check** + +Run: `npx vitest run src/components && npm run i18n:check` +Expected: PASS. Harvest any key the vendored catalog doesn't have yet. + +- [ ] **Step 5: Commit** + +```bash +git add src/components/filters/FilterRuleModal.tsx src/components/__tests__ locales/rn/en.json docs/parity/07-filters-vacation-files.md +git commit -m "feat: add the all-messages condition and address/domain matching to the rule editor" +``` + +Device check: on the phone, create an "All messages → Mark as read" rule. Confirm webmail shows the same rule, then save once from each side; the script must not grow. + +--- + +### Task 8: Send a mailto unsubscribe to one address only, and show what will be sent + +**Files:** +- Modify: `src/lib/unsubscribe.ts` (add after `parseMailtoUrl`, ~:118) +- Modify: `src/components/email/UnsubscribeBanner.tsx:99-140` +- Test: `src/lib/__tests__/unsubscribe.test.ts` + +**Interfaces:** +- Produces: `parseUnsubscribeMailto(url: string): { to: [string]; subject?: string; body?: string } | null`, plus `UNSUBSCRIBE_SUBJECT_MAX = 200` and `UNSUBSCRIBE_BODY_MAX = 500`. + +- [ ] **Step 1: Write the failing tests** + +```ts +it('takes exactly one recipient from the address part', () => { + expect(parseUnsubscribeMailto('mailto:leave@list.example?subject=unsubscribe')).toEqual({ to: ['leave@list.example'], subject: 'unsubscribe' }); +}); +it('refuses a list of addresses', () => { + expect(parseUnsubscribeMailto('mailto:a@x.example,b@y.example')).toBeNull(); +}); +it('ignores to= and cc= query fields', () => { + expect(parseUnsubscribeMailto('mailto:leave@list.example?to=ceo@corp.example&cc=boss@corp.example')) + .toEqual({ to: ['leave@list.example'] }); +}); +it('keeps the subject on one line and caps subject and body', () => { + const r = parseUnsubscribeMailto(`mailto:l@x.example?subject=a%0D%0Ab&body=${'x'.repeat(600)}`)!; + expect(r.subject).toBe('a b'); + expect(r.body).toHaveLength(500); +}); +it('parseMailtoUrl still accepts several addresses', () => { + expect(parseMailtoUrl('mailto:a@x.example,b@y.example')?.to).toEqual(['a@x.example', 'b@y.example']); +}); +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `npx vitest run src/lib/__tests__/unsubscribe.test.ts` +Expected: the `parseUnsubscribeMailto` tests FAIL. + +- [ ] **Step 3: Implement** + +- Port WEB `lib/validation.ts:173-205` as `parseUnsubscribeMailto`, built on the native `parseMailtoUrl`. +- In the banner: + - Parse with `parseUnsubscribeMailto`, once when rendering, so the confirmation can show the result. + - Send only `to`/`subject`/`body`, with no `cc`. + - When the link doesn't parse, hide the mailto option, just as an invalid http URL is hidden today. +- The `confirm()` alert message for mailto becomes `confirm_message_mailto` + `\n\n` + `to[0]`, then the subject and body on their own lines when present (WEB `unsubscribe-banner.tsx:45-51`). +- Leave `EmailBodyView`'s use of `parseMailtoUrl` alone (Review Focus 5). + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `npx vitest run src/lib/__tests__/unsubscribe.test.ts src/components` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add src/lib/unsubscribe.ts src/lib/__tests__/unsubscribe.test.ts src/components/email/UnsubscribeBanner.tsx docs/parity/03-email-viewer.md +git commit -m "fix: send a mailto unsubscribe to the one listed address and show it before sending" +``` + +--- + +### Task 9: Bound the winmail.dat value loops + +**Files:** +- Modify: `src/lib/tnef.ts:168-220` (`parseMAPIProps`) +- Test: `src/lib/__tests__/tnef.test.ts` + +**Interfaces:** +- Produces: no signature changes. + +- [ ] **Step 1: Write the failing tests** + +`parseMAPIProps` is private, so these tests go through `parseTnef`. Build the file with the `u32`/`attr`/`bytes` helpers already at the top of `tnef.test.ts`, putting the MAPI block in an attachment-level attribute (`attAttachment`, the one the existing attachment test uses). + +```ts +it('stops on a truncated variable-length value instead of spinning on the count', () => { + // MAPI block: 1 prop, PT_BINARY (0x0102) id 0x3701, valueCount 0xFFFFFFFF, then a length (1000) larger than what is left. + const mapi = [...u32(1), 0x02, 0x01, 0x01, 0x37, ...u32(0xffffffff), ...u32(1000)]; + const started = Date.now(); + expect(() => parseTnef(tnefWithAttachmentProps(mapi))).not.toThrow(); + expect(Date.now() - started).toBeLessThan(100); +}); +it('stops a multi-value fixed run that consumes nothing', () => { + // PT_MV_LONG (0x1003) with valueCount 0xFFFFFFFF and 2 bytes left. + const mapi = [...u32(1), 0x03, 0x10, 0x00, 0x30, ...u32(0xffffffff), 0, 0]; + const started = Date.now(); + expect(() => parseTnef(tnefWithAttachmentProps(mapi))).not.toThrow(); + expect(Date.now() - started).toBeLessThan(100); +}); +``` + +`tnefWithAttachmentProps(mapi: number[]): Uint8Array` is a new helper in the test file: the TNEF signature and key, then `attr(2, , mapi)`, as the existing attachment tests assemble theirs. Also copy the matching cases from WEB `lib/__tests__/tnef.test.ts`. + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `npx vitest run src/lib/__tests__/tnef.test.ts` +Expected: the new tests FAIL (time out, or exceed the bound). + +- [ ] **Step 3: Implement** + +Port WEB `lib/tnef.ts:173-231`: +- Add `readValueCount(r) = Math.min(r.readUint32LE(), Math.floor(r.remaining / 4))`. +- Use it for both value counts. +- `break` when `readMAPIVarValue` returns `null`. +- `break` when `readMAPIFixedValue` leaves `r.remaining` unchanged. + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `npx vitest run src/lib/__tests__/tnef.test.ts` +Expected: PASS. + +- [ ] **Step 5: Commit and close the phase** + +Update the counts table in `PARITY_CHECKLIST.md` (re-run the per-file count from the 2026-10-04 status note), then: + +```bash +git add src/lib/tnef.ts src/lib/__tests__/tnef.test.ts docs/parity/03-email-viewer.md PARITY_CHECKLIST.md +git commit -m "fix: stop reading a winmail.dat value list that makes no progress" +npm run typecheck && npm test && npm run i18n:check +``` + +Expected: everything passes, and 8 more items are ticked. Phase 1's rows in the roadmap are done. diff --git a/docs/superpowers/plans/2026-10-04-webmail-parity-roadmap.md b/docs/superpowers/plans/2026-10-04-webmail-parity-roadmap.md new file mode 100644 index 00000000..c2dbec79 --- /dev/null +++ b/docs/superpowers/plans/2026-10-04-webmail-parity-roadmap.md @@ -0,0 +1,139 @@ +# Webmail parity roadmap (October 2026) + +How to work down the 109 open items in [PARITY_CHECKLIST.md](../../../PARITY_CHECKLIST.md) +and [docs/parity/](../../parity/): 74 come from the webmail 1.10.0 → 1.12.0+ +delta (audited 2026-10-04), 35 are left over from the 1.9.2 audit. The six +open webmail items in [docs/audit-2026-09.md](../../audit-2026-09.md) ("Missing +vs webmail") are folded into Phase 6. + +Each phase is one branch and one implementation plan. Only Phase 1 has a +detailed plan yet ([2026-10-04-parity-phase-1-security-send.md](2026-10-04-parity-phase-1-security-send.md)). +Write the next phase's plan when the previous one merges, because each phase +changes files the next one touches. Re-run the delta audit before writing a +plan, since webmail ships every few days. + +## Ground rules for every phase + +- **Source of truth.** For each item, the area file in `docs/parity/` gives + the WEB and RN file pointers. Port webmail's logic and tests rather than + re-deriving them. Reference checkout: + `git clone https://github.com/bulwarkmail/webmail` at `a4e313f` or later. +- **One finding, one commit** (`fix:` / `feat:`, as in the git log). Tick the + item in its area file in the same commit and add the commit hash, as the + existing ticks do. +- **Gate:** `npm run typecheck && npm test && npm run i18n:check` passes on + every commit. New user-visible strings use `t('key', 'English')`. Reuse the + webmail key when one exists (it arrives with the next `sync-locales`); + otherwise `npm run i18n:harvest` adds it to `locales/rn/en.json`. +- **Device check.** Anything that touches push, the WebView, the editor or + native modules gets a line in the PR saying what was checked on which device + and server (Stalwart version). +- **Close the loop.** At the end of a phase, update the counts table in + `PARITY_CHECKLIST.md`. + +## Phase 1: security and send correctness (detailed plan written) + +Goal: nothing a sender writes can fool the user or redirect mail, and the app +never says "sent" when nothing went out. Size: about 3–4 days. + +| Item | Area | Pri | +|---|---|---| +| Forged `Authentication-Results` can supply a DKIM/DMARC pass | 03 | P1 | +| Refused recipients in `deliveryStatus` never reported (#1123) | 04 | P1 | +| Escaped quote in a display name splits off a recipient | 04 | P1 | +| Send with no `EmailSubmission/set` response counts as success | 04 | P2 | +| Sieve values unescaped; rule names with spaces duplicate | 07 | P2 | +| No `stop` after discard/reject; `field: 'all'` and `address_is`/`domain_is` break native saves | 07 | P2 | +| `mailto:` unsubscribe to several addresses, not shown | 03 | P2 | +| Crafted `winmail.dat` freezes the app | 03 | P2 | + +## Phase 2: data correctness + +Goal: no write the app makes is silently refused by the server, put in the +wrong account, or rewritten. Size: about 4–5 days. + +| Item | Area | Pri | Note | +|---|---|---|---| +| Contacts with calendar/scheduling/free-busy URIs fail to save | 06 | P2 | Port webmail `lib/jmap/contact-wire.ts` as `src/lib/contact-wire.ts`. One mapping layer feeds the next item too. | +| vCard import sends fields Stalwart rejects | 06 | P2 | Same wire layer: `addressToWire`. | +| Address book delete refused while it has contacts | 06 | P2 | `onDestroyRemoveContents: true` behind the existing confirm. | +| Cross-account move loses the message date (#1150) | 02 | P2 | Pass `receivedAt` to `Email/import`. | +| Open draft follows an account switch into the wrong account | 04 | P2 | Capture `accountId` when the composer mounts. Thread it through `createDraft`/`sendEmail` (`SendEmailOptions.accountId` exists). | +| Filters / security screens keep the previous account's data after a switch | 01 | P2 | Reset the stores in `switchAccount`; key the effects on the account id. | +| iCal subscriptions not tied to the login | 05 | P2 | Key on server URL + username; forget on sign-out. Needs a store migration. | +| Sign-out leaves search history, offline bodies and outbox keys behind | 01 | P2 | One `forgetAccountData(accountKey)` called from logout and removeAccount. Pairs with the iCal item. | +| Daily recurrence stops at a DST change | 05 | P2 | Port webmail `recurrence-expansion.ts:420-444`. Tests in Europe/Berlin and America/New_York. | +| Save without invitations when the server refuses scheduling | 05 | P2 | `SchedulingDeniedError` + "Save without sending" alert. | +| Blank participant names in invitations (#748) | 05 | P3 | Same files as above. | +| Files: smaller 1.11 fixes (rename-on-exists, copy folders, pre-0.16.6 rights, MIME type) | 07 | P3 | | + +## Phase 3: reliability and honest feedback + +Goal: when something fails, the user finds out, and background features keep +working past a week. Size: about 4 days. + +| Item | Area | Pri | Note | +|---|---|---|---| +| Push subscriptions lapse after Stalwart's 7-day expiry | 08 | P2 | Renew on foreground and for every signed-in account. Needs a device check over more than 7 days, or a server with a short expiry. | +| List actions (swipe/batch) fail silently | 02 | P2 | Catch, toast and revert the optimistic change, the same way the viewer already does. | +| A failed search shows the previous folder's rows | 02 | P2 | | +| Search doesn't leave out Spam and Trash | 02 | P2 | | +| TOTP accounts cannot change password or turn TOTP off | 01 | P2 | Prompt for the current code. | +| Internationalized domains (#1100) | 01 | P2 | Check what Hermes `URL` does first; add a punycode dependency only if needed. | +| Tag views/counts include Trash and Spam (#1156); list Move sheet not account-scoped (#1149) | 02 | P3 | | +| Failed preview lookup drops the push; one message rings once per account | 08 | P3 | Android module + background task. | +| Sent-copy filing warning not shown; `Email/set` create can be replayed | 04, 09 | P3 | | +| Mail deleted during a refresh reappears (#966); scroll position and unread-first jumps (unverified) | 02 | P3 | Confirm on a device before fixing. | +| Refused TOTP token exchange gets a generic error; empty name for non-admins | 01 | P3 | | + +## Phase 4: new webmail features that matter on a phone + +Goal: the features users of the current webmail will expect in the app. +Each item is a small sub-project with its own plan. Do them in this order. + +| Item | Area | Pri | Size | +|---|---|---|---| +| Verification-code copy chip (viewer + list, setting) | 03 | P2 | S–M | +| "Rules" from a message, with retroactive apply and undo | 03, 07 | P2 | L. Builds on the Phase 1 Sieve work. | +| Offline send queue (outbox op carrying the Email/set + submission) | 04, 09 | P2 | L | +| Contact autocomplete: groups, recent recipients, server and directory search | 06 | P2 | M | +| Server-side invitations (`CalendarEventNotification`) inbox | 05 | P3→P2 | M | +| Join links and maps in events; working-hours day/week views; tasks in the month view; collapsible all-day strip | 05 | P3 | M | +| Copy messages to a folder / another account | 02 | P3 | M | +| Sign in with an access token; keep cross-origin session URLs | 01 | P3 | S | +| Inbox-only notifications option (#983) | 08 | P3 | S | + +## Phase 5: platform gaps (need work outside this repo) + +These have been deferred because each needs something outside this repo. Plan each +one with the owner of that other piece. + +| Item | Area | Pri | Dependency | +|---|---|---|---| +| iOS push | 08 | P2 | APNs transport in the push relay + an iOS token module | +| Cross-device settings sync (native #1); unblocks template and tag sync | 08, 04, 02 | P2 | A server-side settings store both clients can use | +| RTL completion (swipe directions, drawer side #944) | 08 | P2 | Device check in ar/he | +| Remaining calendar i18n; Jalali grid | 05 | P2/P3 | Port `jalali-utils.ts` | +| S/MIME sign/encrypt on send | 04 | P3 | Raw-MIME send path | +| Sending from shared/group accounts | 04 | P3 | Envelope/identity routing | + +## Phase 6: P3 backlog + +Pick these up when you are working on the same files anyway. Group them so +each sweep stays in one area: + +- **Mail list and search (02):** + - search snippets highlighted, size filter, nested folder picker, colours for unknown tags, row screen-reader labels, attachment-chip placeholders; + - no wildcard suffix in search, empty any folder, folder sharing, folder reorder, folder icons, tag nesting; + - the date-locale setting; + - two decisions: last folder vs inbox on start, and global search. +- **Composer (04):** DSN/REQUIRETLS, Return-Path note, pasting a list, @-mentions, font size (audit-2026-09), identity refresh. +- **Viewer (03):** wrapping fixed-width tables on iOS, and what remains of the invitation banner. +- **Calendar (05):** free/busy, default ParticipantIdentity, duplicate/copy title/add note, `supported-calendar-component-set`, deep links to dates, the push types, birthday colour. +- **Contacts (06):** sharing an address book, list filters, deep links. +- **Filters and files (07):** redirect-limit warning, auto-reply length warning, legacy flat-name migration, Files deep links. +- **Settings and UI (08):** + - settings-search entries, About build link, font size everywhere, status/navigation bar theming, sidebar apps, the relay list; + - from audit-2026-09: icon badge, themes, Tabler icons, the favicon source. +- **Security (09):** a screenshot / recent-apps protection option. +- **Accounts (01):** ending the SSO session on sign-out, and the settings scope of shared accounts. From f66084fc9599315f68712981edb84bf55459294e Mon Sep 17 00:00:00 2001 From: waggins Date: Sun, 4 Oct 2026 09:42:40 -0400 Subject: [PATCH 02/20] fix: take DKIM and DMARC results only from the receiving server's own header Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AdqB9PokeySGSsJ92sqngP --- src/lib/__tests__/email-headers.test.ts | 35 +++++ src/lib/email-headers.ts | 185 +++++++++++++++++++----- 2 files changed, 183 insertions(+), 37 deletions(-) diff --git a/src/lib/__tests__/email-headers.test.ts b/src/lib/__tests__/email-headers.test.ts index c166bb10..002516ea 100644 --- a/src/lib/__tests__/email-headers.test.ts +++ b/src/lib/__tests__/email-headers.test.ts @@ -23,6 +23,30 @@ describe('parseAuthenticationResults', () => { const none = parseAuthenticationResults('spf=pass smtp.mailfrom=a.example; spf=none smtp.helo=b.example'); expect(none.spf?.result).toBe('pass'); }); + + it('takes DKIM and DMARC only from the topmost header', () => { + const r = parseAuthenticationResults([ + 'mx.example; spf=fail smtp.mailfrom=evil.example; dmarc=fail header.from=bank.example', + 'evil.example; dkim=pass header.d=bank.example; dmarc=pass header.from=bank.example', + ]); + expect(r.dmarc?.result).toBe('fail'); + expect(r.dkim).toBeUndefined(); + }); + + it('ignores results inside comments and property values', () => { + const r = parseAuthenticationResults( + 'mx.example; spf=pass smtp.mailfrom="dmarc=pass"@x.example (dkim=pass header.d=bank.example); dmarc=fail', + ); + expect(r.dmarc?.result).toBe('fail'); + expect(r.dkim).toBeUndefined(); + expect(r.spf?.result).toBe('pass'); + }); + + it('lets a lower header escalate SPF to a failure but never supply a pass', () => { + expect(parseAuthenticationResults(['mx; spf=none smtp.mailfrom=a.example', 'x; spf=fail smtp.mailfrom=a.example']).spf?.result).toBe('fail'); + expect(parseAuthenticationResults(['mx; spf=fail smtp.mailfrom=a.example', 'x; spf=pass smtp.mailfrom=a.example']).spf?.result).toBe('fail'); + expect(parseAuthenticationResults(['mx; dkim=none', 'x; spf=pass smtp.mailfrom=a.example']).spf).toBeUndefined(); + }); }); describe('isAuthenticationSpoofed', () => { @@ -78,6 +102,17 @@ describe('deriveHeaderInfo', () => { expect(isAuthenticationSpoofed(info.auth)).toBe(true); }); + it('does not let a lower Authentication-Results header supply a DMARC pass', () => { + const info = deriveHeaderInfo({ + headers: [ + { name: 'Authentication-Results', value: 'mx; spf=pass smtp.mailfrom=a.example' }, + { name: 'Authentication-Results', value: 'x; dmarc=pass' }, + ], + messageId: null, + }); + expect(info.auth?.dmarc).toBeUndefined(); + }); + it('copes with no headers', () => { const info = deriveHeaderInfo({ headers: undefined, messageId: null }); expect(info.readReceiptRequestedBy).toBeNull(); diff --git a/src/lib/email-headers.ts b/src/lib/email-headers.ts index 3b61f202..c0aae538 100644 --- a/src/lib/email-headers.ts +++ b/src/lib/email-headers.ts @@ -103,31 +103,135 @@ export function isAuthenticationSpoofed(auth?: AuthenticationResults): boolean { return false; } -/** Parse an Authentication-Results header into SPF, DKIM, DMARC, iprev results. */ -export function parseAuthenticationResults(header: string): AuthenticationResults { - const results: AuthenticationResults = {}; +interface ResInfo { + method: string; + result: string; + props: Record; +} - // A single header can carry more than one SPF result when the server - // evaluates multiple identities (HELO and MAIL FROM). Collect them all so a - // hard fail on any identity isn't softened to an ambiguous state recorded - // for another one. - const spfRegex = /spf=(\w+)(?:\s+\([^)]*\))?(?:\s+smtp\.(mailfrom|helo)=([^\s;]+))?/g; - const spfResults: SpfEntry[] = []; - let spfM: RegExpExecArray | null; - while ((spfM = spfRegex.exec(header)) !== null) { - spfResults.push({ - result: spfM[1] as SpfResult, - identity: spfM[2] as SpfEntry['identity'], - domain: spfM[3], - }); +/** + * Split one Authentication-Results header into its `;`-separated parts + * (RFC 8601), dropping comments. A `;` inside a quoted string or a comment + * does not split: both can carry sender-chosen text such as the envelope + * address. + */ +function splitResinfo(header: string): string[] { + const parts: string[] = []; + let current = ''; + let depth = 0; + let quoted = false; + for (let i = 0; i < header.length; i++) { + const c = header[i]; + if (c === '\\' && (quoted || depth > 0)) { + if (depth === 0) current += c + (header[i + 1] ?? ''); + i++; + continue; + } + if (quoted) { + current += c; + if (c === '"') quoted = false; + continue; + } + if (c === '(') { + depth++; + continue; + } + if (depth > 0) { + if (c === ')' && --depth === 0) current += ' '; + continue; + } + if (c === '"') quoted = true; + if (c === ';') { + parts.push(current); + current = ''; + continue; + } + current += c; + } + parts.push(current); + return parts.map((part) => part.trim()).filter(Boolean); +} + +const METHOD_RE = /^([a-z0-9][a-z0-9_-]*)(?:\/\d+)?\s*=\s*([a-z]+)(?=\s|$)/i; +const PROP_RE = /\s*([^\s=]+)\s*=\s*("(?:[^"\\]|\\.)*"|\S*)/y; + +/** + * Read one resinfo: the method must open the part, so a `dmarc=pass` that + * appears inside a property value (say an envelope local part in + * `smtp.mailfrom=`) is never taken for a result. + */ +function parseResinfo(part: string): ResInfo | null { + const match = METHOD_RE.exec(part); + if (!match) return null; + const props: Record = {}; + const rest = part.slice(match[0].length); + PROP_RE.lastIndex = 0; + let prop: RegExpExecArray | null; + while ((prop = PROP_RE.exec(rest)) !== null && prop[0].length > 0) { + const key = prop[1].toLowerCase(); + let value = prop[2]; + if (value.startsWith('"')) value = value.slice(1, -1).replace(/\\(.)/g, '$1'); + if (!(key in props)) props[key] = value; } + return { method: match[1].toLowerCase(), result: match[2].toLowerCase(), props }; +} + +function parseResinfos(header: string): ResInfo[] { + return splitResinfo(header) + .map(parseResinfo) + .filter((info): info is ResInfo => info !== null); +} + +const DMARC_SEVERITY: Record = { + fail: 3, + permerror: 2, + temperror: 2, + none: 1, + pass: 0, +}; + +/** + * Parse Authentication-Results headers into SPF, DKIM, DMARC results. + * + * Pass the headers in message order. The topmost one is the receiving + * server's own; anything below it may have been written by the sender, so + * DKIM, DMARC and iprev come from the topmost header only, and the others + * can only escalate SPF to a failure, never supply a pass. + */ +export function parseAuthenticationResults(headers: string | readonly string[]): AuthenticationResults { + const results: AuthenticationResults = {}; + const list = typeof headers === 'string' ? [headers] : headers; + const perHeader = list.map(parseResinfos); + const own = perHeader[0] ?? []; + const foreign = perHeader.slice(1).flat(); + + // Parse SPF. A single Authentication-Results header can carry more than one + // SPF result when the server evaluates multiple identities (HELO and MAIL + // FROM). Collect them all so a hard fail on any identity isn't softened to + // an ambiguous state recorded for another one. + const severity = (r: string) => SPF_SEVERITY[r as SpfResult] ?? -1; + const isFailure = (r: string) => severity(r) >= SPF_SEVERITY.temperror; + const toSpfEntry = (info: ResInfo): SpfEntry => { + const identity = info.props['smtp.mailfrom'] !== undefined + ? 'mailfrom' + : info.props['smtp.helo'] !== undefined ? 'helo' : undefined; + return { + result: info.result as SpfResult, + identity: identity as SpfEntry['identity'], + domain: identity ? info.props[`smtp.${identity}`] || undefined : undefined, + }; + }; + const spfResults: SpfEntry[] = [ + ...own.filter((info) => info.method === 'spf').map(toSpfEntry), + ...foreign.filter((info) => info.method === 'spf').map(toSpfEntry).filter((e) => isFailure(e.result)), + ]; if (spfResults.length > 0) { - const severity = (r: string) => SPF_SEVERITY[r as SpfResult] ?? -1; // MAIL FROM is the primary SPF identity. Another identity (HELO) may only - // escalate the headline to a genuine failure state - a HELO `none` or - // `neutral` must not downgrade a MAIL FROM `pass`. - const isFailure = (r: string) => severity(r) >= SPF_SEVERITY.temperror; - let primary = spfResults.find((e) => e.identity === 'mailfrom') ?? spfResults[0]; + // escalate the headline to a genuine failure state — a HELO `none` or + // `neutral` must not downgrade a MAIL FROM `pass`, since most senders + // publish no SPF record for their EHLO hostname. + let primary = + spfResults.find((e) => e.identity === 'mailfrom') ?? spfResults[0]; for (const cur of spfResults) { if (isFailure(cur.result) && severity(cur.result) > severity(primary.result)) { primary = cur; @@ -140,29 +244,36 @@ export function parseAuthenticationResults(header: string): AuthenticationResult }; } - const dkimMatch = header.match(/dkim=(\w+)(?:\s+header\.d=([^\s;]+))?(?:\s+header\.s=([^\s;]+))?/); - if (dkimMatch) { + const dkim = own.find((info) => info.method === 'dkim'); + if (dkim) { results.dkim = { - result: dkimMatch[1] as DkimResult, - domain: dkimMatch[2], - selector: dkimMatch[3], + result: dkim.result as DkimResult, + domain: dkim.props['header.d'], + selector: dkim.props['header.s'], }; } - const dmarcMatch = header.match(/dmarc=(\w+)(?:\s+header\.from=([^\s;]+))?(?:\s+policy\.dmarc=(\w+))?/); - if (dmarcMatch) { + // One DMARC verdict per message; should a header carry several, the most + // severe stands. + const dmarc = own + .filter((info) => info.method === 'dmarc') + .reduce( + (worst, info) => (!worst || (DMARC_SEVERITY[info.result] ?? -1) > (DMARC_SEVERITY[worst.result] ?? -1) ? info : worst), + undefined, + ); + if (dmarc) { results.dmarc = { - result: dmarcMatch[1] as DmarcResult, - domain: dmarcMatch[2], - policy: dmarcMatch[3] as DmarcPolicy | undefined, + result: dmarc.result as DmarcResult, + domain: dmarc.props['header.from'], + policy: dmarc.props['policy.dmarc'] as DmarcPolicy | undefined, }; } - const iprevMatch = header.match(/iprev=(\w+)(?:\s+policy\.iprev=([\d.]+))?/); - if (iprevMatch) { + const iprev = own.find((info) => info.method === 'iprev'); + if (iprev) { results.iprev = { - result: iprevMatch[1] as 'pass' | 'fail', - ip: iprevMatch[2], + result: iprev.result as 'pass' | 'fail', + ip: iprev.props['policy.iprev'], }; } @@ -261,8 +372,8 @@ export function deriveHeaderInfo(email: Pick): E const headers = email.headers; const authHeaders = headerValues(headers, 'Authentication-Results'); // The last hop's results are prepended, so the first header is the - // receiving server's own verdict. - const auth = authHeaders.length ? parseAuthenticationResults(authHeaders.join('; ')) : undefined; + // receiving server's own verdict; the parser trusts that one for DKIM/DMARC. + const auth = authHeaders.length ? parseAuthenticationResults(authHeaders) : undefined; const spamRaw = headerValue(headers, 'X-Spam-Status') ?? headerValue(headers, 'X-Spam-Result') From 80ef7ce65a0bfe819d4c3645201ec8536e919222 Mon Sep 17 00:00:00 2001 From: waggins Date: Sun, 4 Oct 2026 09:44:18 -0400 Subject: [PATCH 03/20] docs: add the calendar-invitation trust fix to the phase 1 plan Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AdqB9PokeySGSsJ92sqngP --- ...2026-10-04-parity-phase-1-security-send.md | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/docs/superpowers/plans/2026-10-04-parity-phase-1-security-send.md b/docs/superpowers/plans/2026-10-04-parity-phase-1-security-send.md index c71acb1f..3eb16e78 100644 --- a/docs/superpowers/plans/2026-10-04-parity-phase-1-security-send.md +++ b/docs/superpowers/plans/2026-10-04-parity-phase-1-security-send.md @@ -629,3 +629,46 @@ npm run typecheck && npm test && npm run i18n:check ``` Expected: everything passes, and 8 more items are ticked. Phase 1's rows in the roadmap are done. + +--- + +### Task 10: Use the same topmost-header rule for calendar-invitation trust + +Added during execution: the Task 1 review found a second, independent Authentication-Results parser in `src/lib/calendar-invitation.ts` with the old regexes, whose `getEmailAuthenticationResults` lets lower (sender-written) headers "fill gaps", so a forged `dkim=pass`/`dmarc=pass` still makes `hasVerifiedAuthentication` true for an invitation. Webmail derives invitation trust from the same `authenticationResults` its mail viewer uses (WEB `lib/calendar-invitation.ts:203-209`). + +**Files:** +- Modify: `src/lib/calendar-invitation.ts:262-323` (the local `AuthenticationResults`, `parseAuthenticationResults`, `getEmailAuthenticationResults`) +- Test: `src/lib/__tests__/calendar-invitation.test.ts` + +**Interfaces:** +- Consumes: `parseAuthenticationResults(headers: string | readonly string[])` and `headerValues(headers, name)` from `src/lib/email-headers.ts` (Task 1). +- Produces: `getEmailAuthenticationResults(email)` keeps its name and `| null` return; its result type becomes the `AuthenticationResults` exported by `email-headers.ts`. The local parser is deleted; check every importer of the deleted names with `grep -rn "calendar-invitation'" src` and point them at `email-headers.ts`. + +- [ ] **Step 1: Write the failing tests** + +```ts +it('does not trust an invitation on a pass from a lower, sender-written header', () => { + const email = { headers: [ + { name: 'Authentication-Results', value: 'mx.example; spf=fail smtp.mailfrom=evil.example; dkim=none; dmarc=fail header.from=bank.example' }, + { name: 'Authentication-Results', value: 'evil.example; dkim=pass header.d=bank.example; dmarc=pass header.from=bank.example' }, + ] }; + const auth = getEmailAuthenticationResults(email); + expect(auth?.dmarc?.result).toBe('fail'); + expect(auth?.dkim?.result).not.toBe('pass'); +}); +``` + +Add a trust-assessment test through whatever exported function uses `hasVerifiedAuthentication` (find it in the file): the same forged email must not be assessed as verified. A single honest header with `dkim=pass` still is. + +- [ ] **Step 2: Run to verify it fails** — `npx vitest run src/lib/__tests__/calendar-invitation.test.ts`; expected: the new tests FAIL. + +- [ ] **Step 3: Implement** — `getEmailAuthenticationResults` returns `parseAuthenticationResults(headerValues(email.headers, 'Authentication-Results'))` (null when there are none); delete the local parser and type. + +- [ ] **Step 4: Run to verify they pass** — same command plus `npm run typecheck`; expected PASS. + +- [ ] **Step 5: Commit** + +```bash +git add src/lib/calendar-invitation.ts src/lib/__tests__/calendar-invitation.test.ts +git commit -m "fix: judge an invitation's sender by the receiving server's own authentication results" +``` From 5f9812aa013ce83ae8dba51a62773154f8e945c8 Mon Sep 17 00:00:00 2001 From: waggins Date: Sun, 4 Oct 2026 09:45:10 -0400 Subject: [PATCH 04/20] fix: keep an escaped quote inside a recipient's display name Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01AdqB9PokeySGSsJ92sqngP --- src/lib/__tests__/recipients.test.ts | 19 +++++++++++++++++++ src/lib/recipients.ts | 14 +++++++++++--- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/src/lib/__tests__/recipients.test.ts b/src/lib/__tests__/recipients.test.ts index 168efc25..9b7fea8d 100644 --- a/src/lib/__tests__/recipients.test.ts +++ b/src/lib/__tests__/recipients.test.ts @@ -38,6 +38,19 @@ describe('splitRecipients', () => { it('keeps RFC 5322 groups as one entry', () => { expect(splitRecipients('Team: a@x.com, b@y.com;, c@z.com')).toEqual(['Team: a@x.com, b@y.com;', 'c@z.com']); }); + + it('keeps an escaped quote inside the display name (no extra recipient)', () => { + const input = '"Support\\", ceo@corp.example, \\"x" '; + expect(splitRecipients(input)).toEqual([input]); + }); + + it('round-trips a name containing quotes and commas', () => { + const name = 'Support", ceo@corp.example, "x'; + const formatted = formatRecipient(name, 'support@shop.example'); + const parts = splitRecipients(`${formatted}, other@example.com`); + expect(parts).toHaveLength(2); + expect(parseRecipient(parts[0])).toEqual({ name, email: 'support@shop.example' }); + }); }); describe('parseRecipient', () => { @@ -61,6 +74,12 @@ describe('parseRecipient', () => { const list = parseRecipientList([formatRecipient('Doe, John', 'j@x.com'), formatRecipient(undefined, 'b@y.com')].join(', ')); expect(list).toEqual([{ name: 'Doe, John', email: 'j@x.com' }, { email: 'b@y.com' }]); }); + + it('does not open a group on a colon after an escaped quote', () => { + const r = parseRecipient('"a\\": b" '); + expect(r.group).toBeUndefined(); + expect(r.email).toBe('a@example.com'); + }); }); describe('expandRecipients', () => { diff --git a/src/lib/recipients.ts b/src/lib/recipients.ts index 21848ab2..44fd17e8 100644 --- a/src/lib/recipients.ts +++ b/src/lib/recipients.ts @@ -58,7 +58,8 @@ function angleRunCloses(value: string, from: number): boolean { let inQuotes = false; for (let i = from + 1; i < value.length; i++) { const ch = value[i]; - if (ch === '"') inQuotes = !inQuotes; + if (inQuotes && ch === '\\') i++; + else if (ch === '"') inQuotes = !inQuotes; else if (inQuotes) continue; else if (ch === '>') return true; else if (ch === '<') return false; @@ -84,7 +85,13 @@ export function splitRecipients(value: string, separators = ','): string[] { let inGroup = false; for (let i = 0; i < value.length; i++) { const ch = value[i]; - if (ch === '"') { + if (inQuotes && ch === '\\' && i + 1 < value.length) { + // A quoted-pair: formatRecipient writes a `"` in a display name as + // `\"`. Taking it as the closing quote let a sender's name like + // `Support", ceo@corp.example, "x` split into an extra recipient. + current += ch + value[i + 1]; + i++; + } else if (ch === '"') { inQuotes = !inQuotes; current += ch; } else if (ch === '<' && !inQuotes) { @@ -157,7 +164,8 @@ function findTopLevelColon(value: string): number { let inAngle = false; for (let i = 0; i < value.length; i++) { const ch = value[i]; - if (ch === '"') inQuotes = !inQuotes; + if (inQuotes && ch === '\\') i++; + else if (ch === '"') inQuotes = !inQuotes; else if (ch === '<' && !inQuotes) inAngle = true; else if (ch === '>' && !inQuotes) inAngle = false; else if (ch === ':' && !inQuotes && !inAngle) return i; From 600f3551c3768fc8187343d11a1c373cc0eea2ee Mon Sep 17 00:00:00 2001 From: waggins Date: Sun, 4 Oct 2026 09:47:29 -0400 Subject: [PATCH 05/20] fix: fail a send the server refused for every recipient, and never call an unconfirmed send sent Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01AdqB9PokeySGSsJ92sqngP --- .../__tests__/email-send-confirmation.test.ts | 147 ++++++++++++++++++ src/api/__tests__/email.test.ts | 3 +- src/api/email.ts | 69 ++++++-- src/api/jmap-result.ts | 49 ++++++ 4 files changed, 254 insertions(+), 14 deletions(-) create mode 100644 src/api/__tests__/email-send-confirmation.test.ts diff --git a/src/api/__tests__/email-send-confirmation.test.ts b/src/api/__tests__/email-send-confirmation.test.ts new file mode 100644 index 00000000..94516420 --- /dev/null +++ b/src/api/__tests__/email-send-confirmation.test.ts @@ -0,0 +1,147 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +vi.mock('../jmap-client', () => ({ + jmapClient: { + accountId: 'acc-1', + request: vi.fn(), + learnHoldLimit: vi.fn(), + getSubmissionAccountIds: vi.fn(() => ['acc-1']), + hasDelayedSend: vi.fn(() => true), + getMaxCallsInRequest: vi.fn(() => 16), + getMaxObjectsInGet: vi.fn(() => 500), + getMaxObjectsInSet: vi.fn(() => 500), + }, +})); + +import { jmapClient } from '../jmap-client'; +import { sendEmail } from '../email'; +import { + formatRejectedRecipients, + RecipientsRejectedError, + rejectedRecipients, + SendUnconfirmedError, +} from '../jmap-result'; + +const mockRequest = jmapClient.request as ReturnType; + +beforeEach(() => { + vi.clearAllMocks(); +}); + +const OUTGOING = { + from: [{ email: 'me@example.com' }], + to: [{ email: 'you@example.com' }], + subject: 'Hello', + textBody: 'Hi there', +}; + +/** Resolves the send request: the created message and submission, then `extra`. */ +function respond(extra: unknown[]) { + mockRequest.mockResolvedValueOnce({ + methodResponses: [ + ['Email/set', { created: { draft: { id: 'email-9' } } }, '0'], + ['EmailSubmission/set', { created: { 'sub-1': { id: 'sub-9' } } }, '1'], + ...extra, + ], + }); +} + +/** Ids of every Email/set destroy the code under test issued. */ +function destroyedIds(): string[] { + return mockRequest.mock.calls.flatMap(([calls]) => + (calls as [string, { destroy?: string[] }][]) + .filter(([method]) => method === 'Email/set') + .flatMap(([, args]) => args.destroy ?? []), + ); +} + +const REFUSED = [['EmailSubmission/get', { list: [{ deliveryStatus: { + 'gone@example.com': { delivered: 'no', smtpReply: '550 5.1.1 No such user' }, +} }] }, 'deliveryStatus']]; +const DESTROYED = { methodResponses: [['Email/set', { destroyed: ['email-9'] }, '0']] }; + +describe('sendEmail delivery confirmation', () => { + it('asks for the new submission deliveryStatus in the send request', async () => { + respond([['EmailSubmission/get', { list: [{ id: 'sub-9', deliveryStatus: {} }] }, 'deliveryStatus']]); + await sendEmail(OUTGOING, 'id-1', 'sent-1'); + expect(mockRequest.mock.calls[0][0][2]).toEqual( + ['EmailSubmission/get', { accountId: 'acc-1', ids: ['#sub-1'], properties: ['deliveryStatus'] }, 'deliveryStatus'], + ); + }); + + it('returns the refused recipients when the others were accepted', async () => { + respond([['EmailSubmission/get', { list: [{ deliveryStatus: { + 'ok@example.com': { delivered: 'queued', smtpReply: '250 2.1.5 OK' }, + 'gone@example.com': { delivered: 'no', smtpReply: '550 5.1.1 No such user' }, + } }] }, 'deliveryStatus']]); + const result = await sendEmail(OUTGOING, 'id-1', 'sent-1'); + expect(result.rejectedRecipients).toEqual([{ email: 'gone@example.com', smtpReply: '550 5.1.1 No such user' }]); + }); + + it('fails the send, removes the filed copy and keeps the old draft when every recipient was refused', async () => { + respond(REFUSED); + mockRequest.mockResolvedValueOnce(DESTROYED); + const err = await sendEmail(OUTGOING, 'id-1', 'sent-1', undefined, { draftId: 'draft-1' }).catch((e) => e); + expect(err).toBeInstanceOf(RecipientsRejectedError); + expect(destroyedIds()).toEqual(['email-9']); // never 'draft-1' + }); + + it('also fails a held send whose recipients were all refused', async () => { + respond(REFUSED); + mockRequest.mockResolvedValueOnce(DESTROYED); + const err = await sendEmail(OUTGOING, 'id-1', 'sent-1', 30, { draftId: 'draft-1' }).catch((e) => e); + expect(err).toBeInstanceOf(RecipientsRejectedError); + expect(destroyedIds()).toEqual(['email-9']); + }); + + it('throws SendUnconfirmedError when the response has no EmailSubmission/set', async () => { + mockRequest.mockResolvedValueOnce({ methodResponses: [['Email/set', { created: { draft: { id: 'email-9' } } }, '0']] }); + await expect(sendEmail(OUTGOING, 'id-1', 'sent-1')).rejects.toBeInstanceOf(SendUnconfirmedError); + expect(destroyedIds()).toEqual([]); // the copy may be the only record that it went out + }); + + it('treats a missing or failed read-back as a plain success', async () => { + respond([['error', { type: 'unknownMethod' }, 'deliveryStatus']]); + await expect(sendEmail(OUTGOING, 'id-1', 'sent-1')).resolves.toMatchObject({ emailSubmissionId: 'sub-9' }); + respond([]); + await expect(sendEmail(OUTGOING, 'id-1', 'sent-1')).resolves.toMatchObject({ emailSubmissionId: 'sub-9' }); + }); + + // Regression guard: native already reported notCreated before the read-back existed. + it('reports a refused submission by its own error, not the dangling read-back', async () => { + mockRequest.mockResolvedValueOnce({ methodResponses: [ + ['Email/set', { created: { draft: { id: 'email-9' } } }, '0'], + ['EmailSubmission/set', { notCreated: { 'sub-1': { type: 'forbiddenFrom', description: 'Not allowed' } } }, '1'], + ['error', { type: 'invalidResultReference' }, 'deliveryStatus'], + ] }); + mockRequest.mockResolvedValueOnce(DESTROYED); + await expect(sendEmail(OUTGOING, 'id-1', 'sent-1')).rejects.toThrow('Not allowed'); + }); +}); + +describe('rejectedRecipients', () => { + it('reports nothing refused for an empty or missing map', () => { + expect(rejectedRecipients({})).toEqual({ rejected: [], all: false }); + expect(rejectedRecipients(undefined)).toEqual({ rejected: [], all: false }); + }); + + it('flags all when every recipient was refused', () => { + const r = rejectedRecipients({ 'a@x': { delivered: 'no', smtpReply: ' 550 gone ' } }); + expect(r).toEqual({ rejected: [{ email: 'a@x', smtpReply: '550 gone' }], all: true }); + }); + + it('does not flag all when one was accepted', () => { + const r = rejectedRecipients({ 'a@x': { delivered: 'no' }, 'b@y': { delivered: 'queued' } }); + expect(r.all).toBe(false); + expect(r.rejected).toHaveLength(1); + }); +}); + +describe('formatRejectedRecipients', () => { + it('joins recipients with their replies', () => { + expect(formatRejectedRecipients([ + { email: 'a@x', smtpReply: '550 5.1.2 No' }, + { email: 'b@y', smtpReply: '' }, + ])).toBe('a@x (550 5.1.2 No), b@y'); + }); +}); diff --git a/src/api/__tests__/email.test.ts b/src/api/__tests__/email.test.ts index 101f27ad..cb396a06 100644 --- a/src/api/__tests__/email.test.ts +++ b/src/api/__tests__/email.test.ts @@ -530,9 +530,10 @@ describe('email operations', () => { expect(mockRequest).toHaveBeenCalledTimes(1); const calls = mockRequest.mock.calls[0][0]; - expect(calls).toHaveLength(2); + expect(calls).toHaveLength(3); expect(calls[0][0]).toBe('Email/set'); expect(calls[1][0]).toBe('EmailSubmission/set'); + expect(calls[2][0]).toBe('EmailSubmission/get'); expect(calls[1][1].create['sub-1'].emailId).toBe('#draft'); }); diff --git a/src/api/email.ts b/src/api/email.ts index a02943da..2e4507ea 100644 --- a/src/api/email.ts +++ b/src/api/email.ts @@ -1,5 +1,15 @@ import { jmapClient } from './jmap-client'; -import { assertSetResult, batched, parseHoldLimit, requireMethodResult, ScheduleTooLateError } from './jmap-result'; +import { + assertSetResult, + batched, + parseHoldLimit, + RecipientsRejectedError, + rejectedRecipients, + requireMethodResult, + ScheduleTooLateError, + SendUnconfirmedError, + type RejectedRecipient, +} from './jmap-result'; import { keywordPointer, mailboxPointer } from './patch-pointer'; import { CAPABILITIES } from './types'; import type { Attachment, Email, EmailAddress, JMAPMethodCall, Mailbox, Thread } from './types'; @@ -24,6 +34,9 @@ export const EMAIL_FULL_PROPERTIES = [ 'messageId', 'inReplyTo', 'references', 'headers', ]; +/** Call id of the deliveryStatus read-back that rides along with a send. */ +const DELIVERY_STATUS_CALL_ID = 'deliveryStatus'; + const SUBMISSION_USING = [CAPABILITIES.CORE, CAPABILITIES.MAIL, CAPABILITIES.SUBMISSION]; // Prefix a shared account's folder ids so they can't collide with the user's @@ -1416,6 +1429,8 @@ export interface SendEmailResult { emailSubmissionId?: string; /** Post-send filing/cleanup problem (message did go out). */ filingWarning?: string; + /** Recipients the server refused while the others were accepted. */ + rejectedRecipients?: RejectedRecipient[]; } function toMessageIdList(value: string[] | string | undefined): string[] | undefined { @@ -1588,6 +1603,11 @@ export async function sendEmail( [ ['Email/set', { accountId, create: { draft: emailCreate } }, '0'], ['EmailSubmission/set', submissionArgs, '1'], + // Stalwart runs RCPT TO while creating the submission and records a + // refused recipient as delivered "no" instead of failing the create, so + // the set response alone reads as a success. A creation-id reference: + // Stalwart does not evaluate `#ids` result references into /set responses. + ['EmailSubmission/get', { accountId, ids: ['#sub-1'], properties: ['deliveryStatus'] }, DELIVERY_STATUS_CALL_ID], ], SUBMISSION_USING, ); @@ -1597,7 +1617,16 @@ export async function sendEmail( let sendAt: string | undefined; let filingWarning: string | undefined; let failure: Error | undefined; - for (const [methodName, result] of res.methodResponses) { + let deliveryStatus: Record | undefined; + for (const [methodName, result, callId] of res.methodResponses) { + // Only reports: its error (a failed set leaves the reference dangling) + // must not be taken for a failed send or a filing problem. + if (callId === DELIVERY_STATUS_CALL_ID) { + if (methodName === 'EmailSubmission/get') { + deliveryStatus = (result as { list?: { deliveryStatus?: typeof deliveryStatus }[] }).list?.[0]?.deliveryStatus ?? undefined; + } + continue; + } if (methodName === 'error' || methodName.endsWith('/error')) { // Once the submission exists the message has left (or is held), so a // later error is the implicit `onSuccessUpdateEmail` Email/set that @@ -1644,21 +1673,34 @@ export async function sendEmail( } } - if (failure) { - // Nothing went out. A message created before the submission was refused - // must not stay behind: nobody tracked that copy, so a retry left a - // second one next to it. The caller still holds the message, and a - // previous draft version is untouched. - if (emailId) { - try { - await destroyEmails([emailId], accountId); - } catch (err) { - console.warn('[email] failed to remove the unsent copy:', err); - } + // Nothing went out. A message created before the submission was refused + // must not stay behind: nobody tracked that copy, so a retry left a + // second one next to it. The caller still holds the message, and a + // previous draft version is untouched. + const removeUnsentCopy = async () => { + if (!emailId) return; + try { + await destroyEmails([emailId], accountId); + } catch (err) { + console.warn('[email] failed to remove the unsent copy:', err); } + }; + + if (failure) { + await removeUnsentCopy(); throw failure; } + // No submission in the response: the message may still have left, so keep + // the filed copy and the old draft - they may be the only record of it. + if (!emailSubmissionId) throw new SendUnconfirmedError(); + + const refused = rejectedRecipients(deliveryStatus); + if (refused.all) { + await removeUnsentCopy(); + throw new RecipientsRejectedError(refused.rejected); + } + // The message is out (or scheduled) - now it is safe to drop the old draft. // A failure here leaves an orphan in Drafts, which is a filing warning // rather than a failed send (#849). @@ -1676,6 +1718,7 @@ export async function sendEmail( emailId, emailSubmissionId, filingWarning, + rejectedRecipients: refused.rejected.length ? refused.rejected : undefined, }; } diff --git a/src/api/jmap-result.ts b/src/api/jmap-result.ts index 9398583c..b456ce67 100644 --- a/src/api/jmap-result.ts +++ b/src/api/jmap-result.ts @@ -90,6 +90,55 @@ export class ScheduleTooLateError extends Error { } } +interface DeliveryStatus { + delivered?: string; + smtpReply?: string; +} + +export interface RejectedRecipient { + email: string; + smtpReply: string; +} + +/** + * The recipients a submission's deliveryStatus (RFC 8621 §7) marks as not + * delivered, and whether that is all of them - then the message went nowhere. + */ +export function rejectedRecipients(deliveryStatus: Record | null | undefined): { + rejected: RejectedRecipient[]; + all: boolean; +} { + const entries = Object.entries(deliveryStatus ?? {}); + const rejected = entries + .filter(([, status]) => status?.delivered === 'no') + .map(([email, status]) => ({ email, smtpReply: status.smtpReply?.trim() ?? '' })); + return { rejected, all: rejected.length > 0 && rejected.length === entries.length }; +} + +/** "a@example.com (550 5.1.2 Mailbox does not exist.), b@example.com" */ +export function formatRejectedRecipients(recipients: RejectedRecipient[]): string { + return recipients.map(({ email, smtpReply }) => (smtpReply ? `${email} (${smtpReply})` : email)).join(', '); +} + +/** The server refused every recipient of a send, so nothing went out. */ +export class RecipientsRejectedError extends Error { + constructor(readonly recipients: RejectedRecipient[]) { + super(`The server rejected every recipient: ${formatRejectedRecipients(recipients)}`); + this.name = 'RecipientsRejectedError'; + } +} + +/** + * The send request came back without an EmailSubmission: nothing confirms the + * message left, and it may still have. The draft is kept. + */ +export class SendUnconfirmedError extends Error { + constructor() { + super('Send confirmation was not received. Check Sent before sending again. Your draft has been kept.'); + this.name = 'SendUnconfirmedError'; + } +} + /** * The hold limit named in a rejected submission, if that was the reason: * Stalwart's MTA refuses a HOLDFOR beyond its `futureRelease` limit with From e11311809e145ee29f974057b778dfdf0bab50eb Mon Sep 17 00:00:00 2001 From: waggins Date: Sun, 4 Oct 2026 09:50:20 -0400 Subject: [PATCH 06/20] fix: say which recipients the server refused, and point at Sent when a send is unconfirmed Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01AdqB9PokeySGSsJ92sqngP --- locales/rn/en.json | 2 ++ src/components/email/QuickReplyBox.tsx | 11 ++++++- src/lib/__tests__/send-errors.test.ts | 34 ++++++++++++++++++++ src/lib/send-errors.ts | 43 ++++++++++++++++++++++++++ src/screens/ComposeScreen.tsx | 38 +++++++---------------- 5 files changed, 101 insertions(+), 27 deletions(-) create mode 100644 src/lib/__tests__/send-errors.test.ts create mode 100644 src/lib/send-errors.ts diff --git a/locales/rn/en.json b/locales/rn/en.json index eedb5336..922e6c02 100644 --- a/locales/rn/en.json +++ b/locales/rn/en.json @@ -305,6 +305,8 @@ "scheduled_body": "Your message will be sent at {time}.", "scheduled_title": "Scheduled", "send_now": "Send now", + "send_recipients_rejected": "Not sent - the server rejected every recipient.", + "send_some_recipients_rejected": "Sent, but not to these recipients - the server rejected them.", "send_timeout_body": "The message may already have gone out. Check your Sent folder before sending it again.", "send_timeout_title": "No answer from the server", "toolbar": { diff --git a/src/components/email/QuickReplyBox.tsx b/src/components/email/QuickReplyBox.tsx index 920a2a99..2473f176 100644 --- a/src/components/email/QuickReplyBox.tsx +++ b/src/components/email/QuickReplyBox.tsx @@ -20,6 +20,8 @@ import { pickEmailBody, plainTextBody } from '../../lib/email-body'; import { htmlToPlainText } from '../../lib/compose-html'; import { mailboxesOfAccount } from '../../lib/mailbox-tree'; import { emailDisplayDate } from '../../lib/email-date'; +import { sendErrorAlert } from '../../lib/send-errors'; +import { formatRejectedRecipients } from '../../api/jmap-result'; import { buildQuoteHeader, quoteHeaderLabels } from '../../lib/quote-header'; interface Props { @@ -131,12 +133,19 @@ export function QuickReplyBox({ email, jmapAccountId, onMoreOptions, onSent }: P } catch { /* the reply is out; the flag is cosmetic */ } onSent?.({ ...email, keywords: { ...email.keywords, $answered: true } }); setText(''); + if (result.rejectedRecipients?.length) { + toast.warning( + t('email_composer.send_some_recipients_rejected', 'Sent, but not to these recipients - the server rejected them.'), + formatRejectedRecipients(result.rejectedRecipients), + ); + } // The keyboard would cover the undo bar or the toast. Keyboard.dismiss(); // A reply held for the undo-send delay has not gone out yet (webmail b03a0c1d). if (!result.scheduled) toast.success(t('notifications.email_sent', 'Email sent successfully')); } catch (err) { - Alert.alert(t('email_composer.send_failed', 'Failed to send'), err instanceof Error ? err.message : String(err)); + const { title, message } = sendErrorAlert(err, t); + Alert.alert(title, message); } finally { setSending(false); } diff --git a/src/lib/__tests__/send-errors.test.ts b/src/lib/__tests__/send-errors.test.ts new file mode 100644 index 00000000..ae977677 --- /dev/null +++ b/src/lib/__tests__/send-errors.test.ts @@ -0,0 +1,34 @@ +import { describe, it, expect } from 'vitest'; +import { sendErrorAlert } from '../send-errors'; +import { RequestTimeoutError } from '../../api/jmap-client'; +import { RecipientsRejectedError, SendUnconfirmedError, ScheduleTooLateError } from '../../api/jmap-result'; + +const t = (k: string, f?: string) => f ?? k; + +describe('sendErrorAlert', () => { + it('lists the refused recipients with their SMTP replies', () => { + const e = new RecipientsRejectedError([{ email: 'gone@example.com', smtpReply: '550 5.1.1 No such user' }]); + expect(sendErrorAlert(e, t)).toEqual({ + title: 'Not sent - the server rejected every recipient.', + message: 'gone@example.com (550 5.1.1 No such user)', + }); + }); + + it('points at Sent for a timeout and for an unconfirmed send', () => { + const expected = { + title: 'No answer from the server', + message: 'The message may already have gone out. Check your Sent folder before sending it again.', + }; + expect(sendErrorAlert(new RequestTimeoutError(30_000), t)).toEqual(expected); + expect(sendErrorAlert(new SendUnconfirmedError(), t)).toEqual(expected); + }); + + it('keeps the schedule-too-late copy and falls back to the error message', () => { + expect(sendErrorAlert(new ScheduleTooLateError(), t)).toEqual({ + title: 'Too far ahead', + message: 'That is later than this server allows. Pick an earlier time.', + }); + expect(sendErrorAlert(new Error('boom'), t)).toEqual({ title: 'Send failed', message: 'boom' }); + expect(sendErrorAlert('x', t)).toEqual({ title: 'Send failed', message: 'Failed to send email' }); + }); +}); diff --git a/src/lib/send-errors.ts b/src/lib/send-errors.ts new file mode 100644 index 00000000..b90c2e09 --- /dev/null +++ b/src/lib/send-errors.ts @@ -0,0 +1,43 @@ +import { RequestTimeoutError } from '../api/jmap-client'; +import { + RecipientsRejectedError, + SendUnconfirmedError, + ScheduleTooLateError, + formatRejectedRecipients, +} from '../api/jmap-result'; + +/** The alert a failed send shows, shared by the composer and quick reply. */ +export function sendErrorAlert( + e: unknown, + t: (key: string, fallback?: string) => string, +): { title: string; message: string } { + if (e instanceof RecipientsRejectedError) { + return { + title: t('email_composer.send_recipients_rejected', 'Not sent - the server rejected every recipient.'), + message: formatRejectedRecipients(e.recipients), + }; + } + if (e instanceof RequestTimeoutError || e instanceof SendUnconfirmedError) { + // The request may have reached the server; a blind retry would send the + // message twice (#702). + return { + title: t('email_composer.send_timeout_title', 'No answer from the server'), + message: t( + 'email_composer.send_timeout_body', + 'The message may already have gone out. Check your Sent folder before sending it again.', + ), + }; + } + if (e instanceof ScheduleTooLateError) { + // The server refused the hold; the pickers now only offer times within + // the limit it named. + return { + title: t('email_composer.schedule_too_late_title', 'Too far ahead'), + message: t('email_composer.schedule_too_late_body', 'That is later than this server allows. Pick an earlier time.'), + }; + } + return { + title: t('email_composer.send_failed', 'Send failed'), + message: e instanceof Error ? e.message : t('notifications.error_sending', 'Failed to send email'), + }; +} diff --git a/src/screens/ComposeScreen.tsx b/src/screens/ComposeScreen.tsx index 3ec09ce2..0ce83e39 100644 --- a/src/screens/ComposeScreen.tsx +++ b/src/screens/ComposeScreen.tsx @@ -43,7 +43,9 @@ import { getIdentities } from '../api/identity'; import { sendEmail, createDraft, destroyEmails, patchKeywordsForEmails, type OutgoingAttachment, type OutgoingEmail, } from '../api/email'; -import { jmapClient, RequestTimeoutError, ScheduleTooLateError } from '../api/jmap-client'; +import { jmapClient } from '../api/jmap-client'; +import { formatRejectedRecipients } from '../api/jmap-result'; +import { sendErrorAlert } from '../lib/send-errors'; import { uploadBlob, uploadBytes } from '../api/blob'; import { buildReplyRecipients, type ReplySource } from '../lib/reply-recipients'; import { buildReplySubject, buildForwardSubject } from '../lib/subject-prefix'; @@ -2138,6 +2140,13 @@ export default function ComposeScreen({ route, navigation }: Props) { } } } + // Some recipients were refused though the message went to the rest. + if (result.rejectedRecipients?.length) { + toast.warning( + t('email_composer.send_some_recipients_rejected', 'Sent, but not to these recipients - the server rejected them.'), + formatRejectedRecipients(result.rejectedRecipients), + ); + } if (scheduledAt && result.scheduled) { // Confirm an explicit "send later" so the user knows it didn't go out now. const when = result.sendAt ? new Date(result.sendAt) : scheduledAt; @@ -2159,31 +2168,8 @@ export default function ComposeScreen({ route, navigation }: Props) { allowLeaveRef.current = true; navigation.goBack(); } catch (e) { - if (e instanceof RequestTimeoutError) { - // The request may have reached the server; a blind retry would send - // the message twice (#702). - Alert.alert( - t('email_composer.send_timeout_title', 'No answer from the server'), - t( - 'email_composer.send_timeout_body', - 'The message may already have gone out. Check your Sent folder before sending it again.', - ), - ); - return; - } - if (e instanceof ScheduleTooLateError) { - // The server refused the hold; the pickers now only offer times - // within the limit it named. - Alert.alert( - t('email_composer.schedule_too_late_title', 'Too far ahead'), - t('email_composer.schedule_too_late_body', 'That is later than this server allows. Pick an earlier time.'), - ); - return; - } - Alert.alert( - t('email_composer.send_failed', 'Send failed'), - e instanceof Error ? e.message : t('notifications.error_sending', 'Failed to send email'), - ); + const { title, message } = sendErrorAlert(e, t); + Alert.alert(title, message); } finally { setSending(false); } From 2996c023e4dc3282231f86540befd60461884ceb Mon Sep 17 00:00:00 2001 From: waggins Date: Sun, 4 Oct 2026 09:52:38 -0400 Subject: [PATCH 07/20] fix: escape header names, sizes and rule names in the filter script Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AdqB9PokeySGSsJ92sqngP --- src/lib/sieve/__tests__/generator.test.ts | 53 +++++++++++++++++++++++ src/lib/sieve/__tests__/parser.test.ts | 10 +++++ src/lib/sieve/generator.ts | 11 +++-- src/lib/sieve/parser.ts | 9 ++-- 4 files changed, 76 insertions(+), 7 deletions(-) diff --git a/src/lib/sieve/__tests__/generator.test.ts b/src/lib/sieve/__tests__/generator.test.ts index da69f7f3..4a5d58f8 100644 --- a/src/lib/sieve/__tests__/generator.test.ts +++ b/src/lib/sieve/__tests__/generator.test.ts @@ -574,3 +574,56 @@ describe('generateScript', () => { }); }); }); + +describe('generateScript with hostile rule data', () => { + /** The script with every comment removed: what Sieve actually executes. */ + function commands(script: string): string { + return script.replace(/\/\*[\s\S]*?\*\//g, '').replace(/^\s*#.*$/gm, ''); + } + + it('writes a rule name on one line with its whitespace collapsed', () => { + const script = generateScript([makeRule({ name: 'Foo Bar\nredirect "x@evil.example";' })]); + expect(script).toContain('# Rule: Foo Bar redirect "x@evil.example";\n'); + expect(script).not.toMatch(/^redirect/m); + expect(commands(script)).not.toContain('x@evil.example'); + }); + + it('escapes a custom header name', () => { + const script = generateScript([makeRule({ + conditions: [{ field: 'header', headerName: 'X-A" :contains "B', comparator: 'contains', value: 'v' }], + })]); + expect(script).toContain('header :contains "X-A\\" :contains \\"B" "v"'); + }); + + it('writes only a number with an optional K/M/G as a size, else 0', () => { + const size = (value: string) => generateScript([makeRule({ + conditions: [{ field: 'size', comparator: 'greater_than', value }], + })]); + expect(size('10M')).toContain('size :over 10M'); + expect(size('1; redirect "x@evil.example"')).toContain('size :over 0'); + expect(commands(size('1 { redirect "x@evil.example"; } if true'))).not.toContain('x@evil.example'); + }); + + // Generated from the code before the escaping change: green on both sides by design. + it('leaves a plain script byte-identical', () => { + const script = generateScript([ + makeRule({ + id: 'r1', + name: 'Newsletter Filter', + conditions: [{ field: 'from', comparator: 'contains', value: 'news@example.com' }], + actions: [{ type: 'move', value: 'Newsletters' }], + stopProcessing: true, + }), + makeRule({ + id: 'r2', + name: 'Spam And Big', + conditions: [ + { field: 'header', headerName: 'X-Spam-Flag', comparator: 'contains', value: 'YES' }, + { field: 'size', comparator: 'greater_than', value: '10M' }, + ], + actions: [{ type: 'mark_read' }], + }), + ]); + expect(script).toBe("/* @metadata:begin\n{\"version\":1,\"rules\":[{\"id\":\"r1\",\"name\":\"Newsletter Filter\",\"enabled\":true,\"matchType\":\"all\",\"conditions\":[{\"field\":\"from\",\"comparator\":\"contains\",\"value\":\"news@example.com\"}],\"actions\":[{\"type\":\"move\",\"value\":\"Newsletters\"}],\"stopProcessing\":true},{\"id\":\"r2\",\"name\":\"Spam And Big\",\"enabled\":true,\"matchType\":\"all\",\"conditions\":[{\"field\":\"header\",\"headerName\":\"X-Spam-Flag\",\"comparator\":\"contains\",\"value\":\"YES\"},{\"field\":\"size\",\"comparator\":\"greater_than\",\"value\":\"10M\"}],\"actions\":[{\"type\":\"mark_read\"}],\"stopProcessing\":false}]}\n@metadata:end */\n\nrequire [\"fileinto\", \"imap4flags\"];\n\n# Rule: Newsletter Filter\nif header :contains \"From\" \"news@example.com\" {\n fileinto \"Newsletters\";\n stop;\n}\n\n# Rule: Spam And Big\nif allof(header :contains \"X-Spam-Flag\" \"YES\", size :over 10M) {\n addflag \"\\\\Seen\";\n}\n"); + }); +}); diff --git a/src/lib/sieve/__tests__/parser.test.ts b/src/lib/sieve/__tests__/parser.test.ts index c4090e30..2f2343fd 100644 --- a/src/lib/sieve/__tests__/parser.test.ts +++ b/src/lib/sieve/__tests__/parser.test.ts @@ -248,3 +248,13 @@ describe('parseScript', () => { }); }); }); + +describe('parseScript rule names', () => { + it('reads a rule whose name has doubled or trailing spaces back as the same rule', () => { + const rules = [makeRule({ name: 'Foo Bar ' })]; + const parsed = parseScript(generateScript(rules)); + // Bulwark's own rules carry no origin; anything with one is a leftover copy. + expect(parsed.rules.filter((r) => r.origin)).toEqual([]); + expect(parsed.rules).toHaveLength(1); + }); +}); diff --git a/src/lib/sieve/generator.ts b/src/lib/sieve/generator.ts index 0d97c19a..79e615e9 100644 --- a/src/lib/sieve/generator.ts +++ b/src/lib/sieve/generator.ts @@ -44,8 +44,11 @@ function generateCondition(condition: FilterCondition): string { const { field, comparator, value } = condition; if (field === 'size') { - // Size is numeric, single value only. - const sizeValue = Array.isArray(value) ? value[0] : value; + // Size is numeric, single value only. It is written unquoted, so + // anything but a number (with an optional K/M/G quantifier) would be + // Sieve source; fall back to 0. + const raw = String((Array.isArray(value) ? value[0] : value) ?? '').trim(); + const sizeValue = /^\d+[KMG]?$/i.test(raw) ? raw : '0'; const op = comparator === 'greater_than' ? ':over' : ':under'; return `size ${op} ${sizeValue}`; } @@ -78,7 +81,7 @@ function generateCondition(condition: FilterCondition): string { } const headerName = field === 'header' - ? (condition.headerName || 'X-Unknown') + ? escapeString(condition.headerName || 'X-Unknown') : HEADER_MAP[field]; // A field this client does not know (a rule authored by a newer webmail) @@ -336,7 +339,7 @@ export function generateScript( } lines.push(''); - lines.push(`# Rule: ${rule.name}`); + lines.push(`# Rule: ${rule.name.replace(/\s+/g, ' ')}`); const conditions = rule.conditions.map(generateCondition); let conditionStr: string; diff --git a/src/lib/sieve/parser.ts b/src/lib/sieve/parser.ts index 57445a13..cb063e06 100644 --- a/src/lib/sieve/parser.ts +++ b/src/lib/sieve/parser.ts @@ -835,10 +835,13 @@ export function parseScript(content: string): ParseResult { // identifies it as ours. const filteredExternal = external.rules.filter(r => { const raw = r.rawBlock || ''; - const match = raw.match(/#\s*Rule:\s*(.+?)\s*$/m); + const match = raw.match(/#\s*Rule:[ \t]*(.*?)[ \t]*$/m); if (match) { - const name = match[1].trim(); - if (bulwarkRules.some(b => b.name === name)) return false; + // The generator writes the name on one line with its whitespace + // collapsed, so compare both sides the same way. + const oneLine = (s: string) => s.replace(/\s+/g, ' ').trim(); + const name = oneLine(match[1]); + if (bulwarkRules.some(b => oneLine(b.name) === name)) return false; } if (/#\s*Vacation auto-reply/i.test(raw)) return false; return true; From f11bfbd982087b70b1110d9ec2beff128f75a58c Mon Sep 17 00:00:00 2001 From: waggins Date: Sun, 4 Oct 2026 09:53:35 -0400 Subject: [PATCH 08/20] fix: keep a "*/" in a rule from ending the filter script's metadata comment Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AdqB9PokeySGSsJ92sqngP --- src/lib/sieve/__tests__/generator.test.ts | 15 +++++++++++++++ src/lib/sieve/generator.ts | 5 ++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/lib/sieve/__tests__/generator.test.ts b/src/lib/sieve/__tests__/generator.test.ts index 4a5d58f8..ce96214d 100644 --- a/src/lib/sieve/__tests__/generator.test.ts +++ b/src/lib/sieve/__tests__/generator.test.ts @@ -588,6 +588,21 @@ describe('generateScript with hostile rule data', () => { expect(commands(script)).not.toContain('x@evil.example'); }); + it('keeps a "*/" in a rule from ending the metadata comment', () => { + const name = 'a */ redirect "x@evil.example"; /* b'; + const script = generateScript([makeRule({ + name, + conditions: [{ field: 'subject', comparator: 'contains', value: 'x */ redirect "y@evil.example"; /*' }], + })]); + expect(commands(script)).not.toContain('evil.example'); + expect(script).not.toMatch(/^\s*redirect/m); + const parsed = parseScript(script); + expect(parsed.isOpaque).toBe(false); + expect(parsed.rules).toHaveLength(1); + expect(parsed.rules[0].name).toBe(name); + expect(parsed.rules[0].conditions[0].value).toBe('x */ redirect "y@evil.example"; /*'); + }); + it('escapes a custom header name', () => { const script = generateScript([makeRule({ conditions: [{ field: 'header', headerName: 'X-A" :contains "B', comparator: 'contains', value: 'v' }], diff --git a/src/lib/sieve/generator.ts b/src/lib/sieve/generator.ts index 79e615e9..4ba9717f 100644 --- a/src/lib/sieve/generator.ts +++ b/src/lib/sieve/generator.ts @@ -293,7 +293,10 @@ export function generateScript( if (options.includeVacation) { metadata.includeVacation = true; } - const metadataJson = JSON.stringify(metadata); + // The JSON sits inside a /* ... */ comment: a "*/" in any string (a rule + // name, a condition value) would end the comment and turn the rest into + // live Sieve. JSON reads "\/" back as "/", so the metadata is unchanged. + const metadataJson = JSON.stringify(metadata).replace(/\*\//g, '*\\/'); const lines: string[] = []; lines.push('/* @metadata:begin'); From f15be2c9b8cf5fef5f2a4925b0195155a987266b Mon Sep 17 00:00:00 2001 From: waggins Date: Sun, 4 Oct 2026 09:56:39 -0400 Subject: [PATCH 09/20] fix: stop after a silent delete or reject, and read the webmail's all-messages and address rules Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AdqB9PokeySGSsJ92sqngP --- .../__tests__/address-comparators.test.ts | 194 ++++++++++++++++++ .../sieve/__tests__/condition-value.test.ts | 11 + src/lib/sieve/__tests__/generator.test.ts | 72 +++++-- src/lib/sieve/__tests__/parser.test.ts | 14 ++ src/lib/sieve/condition-value.ts | 24 ++- src/lib/sieve/generator.ts | 24 ++- src/lib/sieve/parser.ts | 21 ++ src/lib/sieve/types.ts | 9 +- 8 files changed, 345 insertions(+), 24 deletions(-) create mode 100644 src/lib/sieve/__tests__/address-comparators.test.ts diff --git a/src/lib/sieve/__tests__/address-comparators.test.ts b/src/lib/sieve/__tests__/address-comparators.test.ts new file mode 100644 index 00000000..110ac231 --- /dev/null +++ b/src/lib/sieve/__tests__/address-comparators.test.ts @@ -0,0 +1,194 @@ +import { describe, it, expect } from 'vitest'; +import { generateScript } from '../generator'; +import { parseScript } from '../parser'; +import type { FilterCondition, FilterRule } from '../types'; + +function makeRule(conditions: FilterCondition[], overrides: Partial = {}): FilterRule { + return { + id: 'rule-1', + name: 'Quick', + enabled: true, + matchType: 'all', + conditions, + actions: [{ type: 'move', value: 'News', mailboxId: 'mb-news' }], + stopProcessing: true, + ...overrides, + }; +} + +/** The if-block the generator writes for the rule. */ +function ruleBlock(script: string): string { + const start = script.indexOf('# Rule:'); + return script.slice(start).trim(); +} + +describe('address comparators in the generator', () => { + it('writes address_is as an exact address test on the parsed From', () => { + const script = generateScript([makeRule([{ field: 'from', comparator: 'address_is', value: 'anna@acme.com' }])]); + expect(script).toContain('if address :is "From" "anna@acme.com" {'); + expect(script).not.toContain('header :contains "From"'); + }); + + it('writes several senders as one address test with a string list', () => { + const script = generateScript([ + makeRule([{ field: 'from', comparator: 'address_is', value: ['anna@acme.com', 'bob@example.org'] }]), + ]); + expect(script).toContain('if address :is "From" ["anna@acme.com", "bob@example.org"] {'); + }); + + it('writes domain_is as an address :domain test', () => { + const script = generateScript([makeRule([{ field: 'from', comparator: 'domain_is', value: 'acme.com' }])]); + expect(script).toContain('if address :domain :is "From" "acme.com" {'); + }); + + it('writes several domains as a string list', () => { + const script = generateScript([ + makeRule([{ field: 'from', comparator: 'domain_is', value: ['acme.com', 'example.org'] }]), + ]); + expect(script).toContain('address :domain :is "From" ["acme.com", "example.org"]'); + }); + + it('uses the right header for To and Cc', () => { + const script = generateScript([ + makeRule([ + { field: 'to', comparator: 'address_is', value: 'me@acme.com' }, + { field: 'cc', comparator: 'domain_is', value: 'acme.com' }, + ]), + ]); + expect(script).toContain('allof(address :is "To" "me@acme.com", address :domain :is "Cc" "acme.com")'); + }); + + it('escapes quotes and backslashes in addresses', () => { + const script = generateScript([makeRule([{ field: 'from', comparator: 'address_is', value: 'a"b\\c@acme.com' }])]); + expect(script).toContain('address :is "From" "a\\"b\\\\c@acme.com"'); + }); + + it('keeps an address comparator on a non-address field readable, as header :contains', () => { + const script = generateScript([makeRule([{ field: 'subject', comparator: 'address_is', value: 'x' }])]); + expect(script).toContain('if header :contains "Subject" "x" {'); + expect(script).not.toContain('address '); + }); + + it('needs no extra require for the address test', () => { + const script = generateScript([ + makeRule([{ field: 'from', comparator: 'address_is', value: 'anna@acme.com' }], { + actions: [{ type: 'mark_read' }], + }), + ]); + expect(script).toContain('require ["imap4flags"];'); + }); + + it('writes the whole quick rule: mailbox id, stop and the spam guard', () => { + const script = generateScript( + [makeRule([{ field: 'from', comparator: 'address_is', value: 'anna@acme.com' }])], + undefined, + { extensions: ['fileinto', 'mailbox', 'mailboxid', 'spamtestplus', 'relational', 'imap4flags'] }, + ); + expect(ruleBlock(script)).toBe([ + '# Rule: Quick', + 'if allof(address :is "From" "anna@acme.com", not spamtest :percent :value "ge" :comparator "i;ascii-numeric" "50") {', + ' fileinto :mailboxid "mb-news" "News";', + ' stop;', + '}', + ].join('\n')); + }); + + it('leaves the spam guard off a block rule (includeSpam)', () => { + const script = generateScript( + [makeRule([{ field: 'from', comparator: 'address_is', value: 'spam@acme.com' }], { + actions: [{ type: 'move', value: 'Junk', mailboxId: 'mb-junk' }], + includeSpam: true, + })], + undefined, + { extensions: ['fileinto', 'mailbox', 'mailboxid', 'spamtestplus', 'relational'] }, + ); + expect(ruleBlock(script)).toBe([ + '# Rule: Quick', + 'if address :is "From" "spam@acme.com" {', + ' fileinto :mailboxid "mb-junk" "Junk";', + ' stop;', + '}', + ].join('\n')); + }); +}); + +describe('List-Id condition', () => { + it('uses the existing header vocabulary', () => { + const script = generateScript([ + makeRule([{ field: 'header', headerName: 'List-Id', comparator: 'contains', value: 'news.acme.com' }]), + ]); + expect(script).toContain('if header :contains "List-Id" "news.acme.com" {'); + }); +}); + +describe('existing comparators are unchanged', () => { + const cases: Array<[FilterCondition, string]> = [ + [{ field: 'from', comparator: 'contains', value: 'anna@acme.com' }, 'header :contains "From" "anna@acme.com"'], + [{ field: 'from', comparator: 'is', value: 'Anna ' }, 'header :is "From" "Anna "'], + [{ field: 'from', comparator: 'ends_with', value: '@acme.com' }, 'header :matches "From" "*@acme.com"'], + [{ field: 'from', comparator: 'starts_with', value: 'anna' }, 'header :matches "From" "anna*"'], + [{ field: 'to', comparator: 'not_contains', value: 'x' }, 'not header :contains "To" "x"'], + [{ field: 'cc', comparator: 'not_is', value: 'x' }, 'not header :is "Cc" "x"'], + [{ field: 'subject', comparator: 'matches', value: 'a*b' }, 'header :matches "Subject" "a*b"'], + [{ field: 'from', comparator: 'contains', value: ['a', 'b'] }, 'header :contains "From" ["a", "b"]'], + ]; + it.each(cases)('%j', (condition, expected) => { + const script = generateScript([makeRule([condition], { actions: [{ type: 'mark_read' }], stopProcessing: false })]); + expect(script).toContain(`if ${expected} {`); + }); +}); + +describe('address comparators in the parser', () => { + it('round-trips through the metadata block', () => { + const rules = [ + makeRule([{ field: 'from', comparator: 'address_is', value: ['anna@acme.com', 'bob@example.org'] }]), + makeRule([{ field: 'from', comparator: 'domain_is', value: 'acme.com' }], { id: 'rule-2', name: 'Domain' }), + ]; + const parsed = parseScript(generateScript(rules)); + expect(parsed.isOpaque).toBe(false); + expect(parsed.rules).toEqual(rules); + }); + + it('keeps a rule with an unknown comparator, which the generator writes as header :contains', () => { + const script = generateScript([]).replace( + '"rules":[]', + JSON.stringify({ rules: [makeRule([{ field: 'from', comparator: 'is_similar_to' as never, value: 'anna' }])] }).slice(1, -1), + ); + const parsed = parseScript(script); + expect(parsed.isOpaque).toBe(false); + expect(parsed.rules[0].conditions[0].comparator).toBe('is_similar_to'); + expect(generateScript(parsed.rules)).toContain('if header :contains "From" "anna" {'); + }); + + it('reads address tests in a script without metadata', () => { + const parsed = parseScript([ + 'require ["fileinto"];', + '# Rule: Hand made', + 'if address :is "from" ["anna@acme.com", "bob@example.org"] {', + ' fileinto "News";', + '}', + 'if address :domain :is "From" "acme.com" {', + ' fileinto "Acme";', + '}', + 'if address :is :domain "to" "example.org" {', + ' fileinto "Example";', + '}', + ].join('\n')); + expect(parsed.isOpaque).toBe(false); + expect(parsed.rules.map(r => r.conditions[0])).toEqual([ + { field: 'from', comparator: 'address_is', value: ['anna@acme.com', 'bob@example.org'] }, + { field: 'from', comparator: 'domain_is', value: 'acme.com' }, + { field: 'to', comparator: 'domain_is', value: 'example.org' }, + ]); + expect(parsed.rules.every(r => r.origin === 'external')).toBe(true); + }); + + it('leaves address tests it has no builder form for as preserved blocks', () => { + const parsed = parseScript([ + 'if not address :is "from" "anna@acme.com" { discard; }', + 'if address :localpart :is "from" "anna" { discard; }', + 'if address :is "subject" "anna" { discard; }', + ].join('\n')); + expect(parsed.rules.map(r => r.origin)).toEqual(['opaque', 'opaque', 'opaque']); + }); +}); diff --git a/src/lib/sieve/__tests__/condition-value.test.ts b/src/lib/sieve/__tests__/condition-value.test.ts index f59fec6c..0cf955fb 100644 --- a/src/lib/sieve/__tests__/condition-value.test.ts +++ b/src/lib/sieve/__tests__/condition-value.test.ts @@ -4,6 +4,7 @@ import { inputStringToValue, isConditionValueEmpty, describeCondition, + isValueLessCondition, summarizeRule, } from '../condition-value'; import type { FilterRule } from '../types'; @@ -58,4 +59,14 @@ describe('summaries', () => { 'from contains "@a.com" or "@b.com" or attachment has_any (+1) → move "Archive", stop', ); }); + + it('describes an all-messages condition by its field label alone', () => { + expect(describeCondition({ field: 'all', comparator: 'any', value: '' }, t)).toBe('All messages'); + }); + + it('treats has_any and all as value-less', () => { + expect(isValueLessCondition({ field: 'all', comparator: 'any', value: '' })).toBe(true); + expect(isValueLessCondition({ field: 'attachment', comparator: 'has_any', value: '' })).toBe(true); + expect(isValueLessCondition({ field: 'from', comparator: 'contains', value: 'x' })).toBe(false); + }); }); diff --git a/src/lib/sieve/__tests__/generator.test.ts b/src/lib/sieve/__tests__/generator.test.ts index ce96214d..67022c94 100644 --- a/src/lib/sieve/__tests__/generator.test.ts +++ b/src/lib/sieve/__tests__/generator.test.ts @@ -225,6 +225,45 @@ describe('generateScript', () => { }); }); + describe('all messages', () => { + const ALL = { field: 'all', comparator: 'any', value: '' } as const; + + it('matches every message', () => { + const script = generateScript([makeRule({ conditions: [ALL], actions: [{ type: 'mark_read' }] })]); + expect(script).toContain('if true {\n addflag "\\\\Seen";\n}'); + // Nothing to require for it. + expect(script).toMatch(/^require \["imap4flags"\];$/m); + }); + + it('still leaves spam out of a move to a folder', () => { + const script = generateScript( + [makeRule({ conditions: [ALL], actions: [{ type: 'move', value: 'Archive' }] })], + undefined, + { extensions: ['fileinto', 'spamtestplus', 'relational'] }, + ); + expect(script).toContain('if allof(true, not spamtest :percent :value "ge" :comparator "i;ascii-numeric" "50") {'); + }); + + it('stands next to other conditions', () => { + const subject = { field: 'subject', comparator: 'contains', value: 'Rechnung' } as const; + expect(generateScript([makeRule({ conditions: [ALL, subject] })])) + .toContain('if allof(true, header :contains "Subject" "Rechnung") {'); + expect(generateScript([makeRule({ matchType: 'any', conditions: [subject, ALL] })])) + .toContain('if anyof(header :contains "Subject" "Rechnung", true) {'); + // Any of them, and still no spam into a folder. + expect(generateScript( + [makeRule({ matchType: 'any', conditions: [subject, ALL] })], + undefined, + { extensions: ['fileinto', 'spamtestplus', 'relational'] }, + )).toContain('if allof(anyof(header :contains "Subject" "Rechnung", true), not spamtest :percent :value "ge" :comparator "i;ascii-numeric" "50") {'); + }); + + it('reads back as written', () => { + const rules = [makeRule({ conditions: [ALL], actions: [{ type: 'mark_read' }] })]; + expect(parseScript(generateScript(rules)).rules).toEqual(rules); + }); + }); + describe('stopProcessing', () => { it('appends stop when stopProcessing is true', () => { const script = generateScript([makeRule({ stopProcessing: true })]); @@ -241,22 +280,27 @@ describe('generateScript', () => { expect(matches).toHaveLength(1); }); - it('does not append stop after discard', () => { - const script = generateScript([makeRule({ - actions: [{ type: 'discard' }], - stopProcessing: true, - })]); - const matches = script.match(/stop;/g); - expect(matches).toBeNull(); + it('writes a stop after discard and reject when the rule says stop', () => { + for (const type of ['discard', 'reject'] as const) { + const script = generateScript([makeRule({ stopProcessing: true, actions: [{ type, value: 'no' }] })]); + expect(script).toMatch(new RegExp(`${type}[^\\n]*;\\n stop;\\n}`)); + } }); - it('does not append stop after reject', () => { - const script = generateScript([makeRule({ - actions: [{ type: 'reject', value: 'No' }], - stopProcessing: true, - })]); - const matches = script.match(/stop;/g); - expect(matches).toBeNull(); + it('writes no second stop when the last action is already stop', () => { + const script = generateScript([makeRule({ stopProcessing: true, actions: [{ type: 'stop' }] })]); + expect(script.match(/stop;/g)).toHaveLength(1); + }); + }); + + describe('address comparators', () => { + it('writes address_is and domain_is as address tests', () => { + const s = generateScript([makeRule({ conditions: [ + { field: 'from', comparator: 'address_is', value: 'anna@acme.com' }, + { field: 'to', comparator: 'domain_is', value: 'acme.com' }, + ] })]); + expect(s).toContain('address :is "From" "anna@acme.com"'); + expect(s).toContain('address :domain :is "To" "acme.com"'); }); }); diff --git a/src/lib/sieve/__tests__/parser.test.ts b/src/lib/sieve/__tests__/parser.test.ts index 2f2343fd..24a25916 100644 --- a/src/lib/sieve/__tests__/parser.test.ts +++ b/src/lib/sieve/__tests__/parser.test.ts @@ -258,3 +258,17 @@ describe('parseScript rule names', () => { expect(parsed.rules).toHaveLength(1); }); }); + +describe('bare true test', () => { + it('reads if true as an all-messages condition without metadata', () => { + const parsed = parseScript('require ["imap4flags"];\n# Rule: Everything\nif true {\n addflag "\\\\Seen";\n}\n'); + expect(parsed.isOpaque).toBe(false); + expect(parsed.rules[0].conditions).toEqual([{ field: 'all', comparator: 'any', value: '' }]); + }); + + it('reads true inside anyof, and leaves not true opaque', () => { + const parsed = parseScript('if anyof(header :contains "Subject" "x", true) { discard; }\nif not true { discard; }'); + expect(parsed.rules[0].conditions[1]).toEqual({ field: 'all', comparator: 'any', value: '' }); + expect(parsed.rules[1].origin).toBe('opaque'); + }); +}); diff --git a/src/lib/sieve/condition-value.ts b/src/lib/sieve/condition-value.ts index ee987d52..1ddc8643 100644 --- a/src/lib/sieve/condition-value.ts +++ b/src/lib/sieve/condition-value.ts @@ -27,21 +27,39 @@ export function isHasAnyCondition(cond: FilterCondition): boolean { return cond.field === 'attachment' && cond.comparator === 'has_any'; } +// Conditions with nothing to type: attachment has_any and every message. +export function isValueLessCondition(cond: FilterCondition): boolean { + return isHasAnyCondition(cond) || cond.field === 'all'; +} + type Translate = (key: string, fallback?: string) => string; // Human-readable value part of a condition: `"a"`, or `"a" or "b"` for lists // using the locale's OR glue, or nothing for the value-less has_any test. export function formatConditionValue(cond: FilterCondition, t: Translate): string { - if (isHasAnyCondition(cond)) return ''; + if (isValueLessCondition(cond)) return ''; if (Array.isArray(cond.value)) { return cond.value.map((v) => `"${v}"`).join(` ${t('settings.filters.or', 'or')} `); } return `"${cond.value}"`; } +const COMPARATOR_FALLBACK: Partial> = { + address_is: 'is the address', + domain_is: 'has the domain', +}; + export function describeCondition(cond: FilterCondition, t: Translate): string { - const field = t(`settings.filters.condition_fields.${cond.field}`, cond.field); - const comparator = t(`settings.filters.comparators.${cond.comparator}`, cond.comparator); + const field = t( + `settings.filters.condition_fields.${cond.field}`, + cond.field === 'all' ? 'All messages' : cond.field, + ); + // "All messages" says it all; there is no comparator to show. + if (cond.field === 'all') return field; + const comparator = t( + `settings.filters.comparators.${cond.comparator}`, + COMPARATOR_FALLBACK[cond.comparator] ?? cond.comparator, + ); const value = formatConditionValue(cond, t); return value ? `${field} ${comparator} ${value}` : `${field} ${comparator}`; } diff --git a/src/lib/sieve/generator.ts b/src/lib/sieve/generator.ts index 4ba9717f..eaba79f5 100644 --- a/src/lib/sieve/generator.ts +++ b/src/lib/sieve/generator.ts @@ -18,6 +18,9 @@ const HEADER_MAP: Record = { subject: 'Subject', }; +/** Fields whose header holds addresses, so the `address` test applies. */ +const ADDRESS_FIELDS = new Set(['from', 'to', 'cc']); + function escapeString(value: string): string { return value.replace(/\\/g, '\\\\').replace(/"/g, '\\"'); } @@ -43,6 +46,9 @@ function formatStringArg(values: string[], transform: (s: string) => string = (s function generateCondition(condition: FilterCondition): string { const { field, comparator, value } = condition; + // Every message: there is nothing to compare. + if (field === 'all') return 'true'; + if (field === 'size') { // Size is numeric, single value only. It is written unquoted, so // anything but a number (with an optional K/M/G quantifier) would be @@ -92,6 +98,14 @@ function generateCondition(condition: FilterCondition): string { throw new Error(`Unsupported filter condition field: ${String(field)}`); } + // RFC 5228 5.1: `address` compares the parsed address, so the display name + // and angle brackets play no part. `header :contains` would let + // "anna@acme.com" match joanna@acme.com. + if ((comparator === 'address_is' || comparator === 'domain_is') && ADDRESS_FIELDS.has(field)) { + const part = comparator === 'domain_is' ? ':domain ' : ''; + return `address ${part}:is "${headerName}" ${formatStringArg(values)}`; + } + switch (comparator) { case 'contains': return `header :contains "${headerName}" ${formatStringArg(values)}`; @@ -364,11 +378,11 @@ export function generateScript( const actionLines = generateActions(rule.actions, useMailboxId); - if (rule.stopProcessing) { - const lastAction = rule.actions[rule.actions.length - 1]; - if (!lastAction || !['stop', 'discard', 'reject'].includes(lastAction.type)) { - actionLines.push('stop;'); - } + // discard and reject only cancel the implicit keep; the script goes on + // and later rules would still act on the message. So "stop processing" + // always writes a stop, unless the block has one already. + if (rule.stopProcessing && !actionLines.includes('stop;')) { + actionLines.push('stop;'); } lines.push(`if ${conditionStr} {`); diff --git a/src/lib/sieve/parser.ts b/src/lib/sieve/parser.ts index cb063e06..2fb32f53 100644 --- a/src/lib/sieve/parser.ts +++ b/src/lib/sieve/parser.ts @@ -344,6 +344,12 @@ function parseAtom(raw: string): FilterCondition | null { } } + // Sieve's `true` test: the all-messages condition, so a script without + // metadata reads back too. Negated, it has no builder form. + if (s === 'true') { + return negated ? null : { field: 'all', comparator: 'any', value: '' }; + } + // Parse the value-tail of a header/body test: either a single quoted // string or a Sieve list literal ["a", "b", ...]. Returns the unwrapped // value(s), preserving the array shape when present so the caller can @@ -457,6 +463,21 @@ function parseAtom(raw: string): FilterCondition | null { return null; } + // `address [:all|:domain] :is` on From/To/Cc, as the generator writes the + // address_is / domain_is comparators. Tagged arguments come in any order, + // so the address part may also follow `:is`. Anything else about an + // address test has no builder equivalent and stays opaque. + m = /^address\s+(?:(:all|:domain)\s+)?:is\s+(?:(:all|:domain)\s+)?"((?:[^"\\]|\\.)*)"\s+([\s\S]+)$/.exec(s); + if (m) { + const [, partBefore, partAfter, headerName, rawTail] = m; + if (negated || (partBefore && partAfter)) return null; + const field = FIELD_FROM_HEADER[unescapeSieveString(headerName).toLowerCase()]; + const value = parseValueTail(rawTail); + if (!field || field === 'subject' || value === null) return null; + const part = partBefore ?? partAfter; + return { field, comparator: part === ':domain' ? 'domain_is' : 'address_is', value }; + } + m = /^header\s+:(contains|is|matches)\s+"((?:[^"\\]|\\.)*)"\s+([\s\S]+)$/.exec(s); if (m) { const [, tag, headerName, rawTail] = m; diff --git a/src/lib/sieve/types.ts b/src/lib/sieve/types.ts index a4a82133..0b628c65 100644 --- a/src/lib/sieve/types.ts +++ b/src/lib/sieve/types.ts @@ -22,7 +22,9 @@ export interface SieveCapabilities { export type FilterConditionField = | 'from' | 'to' | 'cc' | 'subject' | 'header' | 'size' | 'body' - | 'attachment'; + // 'all' matches every message; it always pairs with comparator 'any' and + // an empty value. + | 'attachment' | 'all'; export type FilterComparator = | 'contains' | 'not_contains' @@ -34,7 +36,10 @@ export type FilterComparator = // has_any → message has any attachment (Content-Disposition: attachment) // has_type → message has an attachment whose Content-Type matches `value` // (substring match, e.g. "application/pdf" or "image/") - | 'has_any' | 'has_type'; + | 'has_any' | 'has_type' + // address_is / domain_is: Sieve `address` test on From/To/Cc (exact parsed + // address, or just its domain). 'any' is the 'all' field's comparator. + | 'address_is' | 'domain_is' | 'any'; export type FilterActionType = | 'move' | 'copy' | 'forward' From fd2f129701ffc2bd1566487409946c7be0c34c35 Mon Sep 17 00:00:00 2001 From: waggins Date: Sun, 4 Oct 2026 09:59:47 -0400 Subject: [PATCH 10/20] feat: add the all-messages condition and address/domain matching to the rule editor Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AdqB9PokeySGSsJ92sqngP --- locales/rn/en.json | 7 ++ src/components/filters/FilterRuleModal.tsx | 72 ++++++------------- .../sieve/__tests__/condition-options.test.ts | 59 +++++++++++++++ src/lib/sieve/condition-options.ts | 54 ++++++++++++++ 4 files changed, 141 insertions(+), 51 deletions(-) create mode 100644 src/lib/sieve/__tests__/condition-options.test.ts create mode 100644 src/lib/sieve/condition-options.ts diff --git a/locales/rn/en.json b/locales/rn/en.json index 922e6c02..bd6fa032 100644 --- a/locales/rn/en.json +++ b/locales/rn/en.json @@ -826,6 +826,13 @@ "title": "Experimental Feature" }, "filters": { + "comparators": { + "address_is": "is the address", + "domain_is": "has the domain" + }, + "condition_fields": { + "all": "All messages" + }, "remove_action": "Remove action", "remove_condition": "Remove condition", "sieve_editor": { diff --git a/src/components/filters/FilterRuleModal.tsx b/src/components/filters/FilterRuleModal.tsx index 7128ac31..f907ee63 100644 --- a/src/components/filters/FilterRuleModal.tsx +++ b/src/components/filters/FilterRuleModal.tsx @@ -13,12 +13,13 @@ import Button from '../Button'; import { useLocaleStore } from '../../stores/locale-store'; import { useKeywordsStore } from '../../stores/keywords-store'; import { generateUUID } from '../../lib/uuid'; +import { isValueLessCondition, valueToInputString } from '../../lib/sieve/condition-value'; import { - inputStringToValue, - isConditionValueEmpty, - isHasAnyCondition, - valueToInputString, -} from '../../lib/sieve/condition-value'; + CONDITION_FIELDS, + comparatorsFor, + conditionForField, + conditionsToSave, +} from '../../lib/sieve/condition-options'; import { ACTIONS_WITH_MAILBOX, ACTIONS_WITH_VALUE, @@ -38,19 +39,6 @@ import type { FilterActionType, } from '../../lib/sieve/types'; -const ALL_FIELDS: FilterConditionField[] = ['from', 'to', 'cc', 'subject', 'header', 'size', 'body', 'attachment']; -const TEXT_COMPARATORS: FilterComparator[] = ['contains', 'not_contains', 'is', 'not_is', 'starts_with', 'ends_with', 'matches']; -const SIZE_COMPARATORS: FilterComparator[] = ['greater_than', 'less_than']; -const ATTACHMENT_COMPARATORS: FilterComparator[] = ['has_any', 'has_type']; - -function comparatorsFor(field: FilterConditionField): FilterComparator[] { - if (field === 'size') return SIZE_COMPARATORS; - if (field === 'attachment') return ATTACHMENT_COMPARATORS; - return TEXT_COMPARATORS; -} - -const isHasAny = isHasAnyCondition; - function seedConditions(rule?: FilterRule): FilterCondition[] { return rule?.conditions.length ? rule.conditions.map((cond) => ({ ...cond })) : [makeEmptyCondition()]; } @@ -105,7 +93,7 @@ export function FilterRuleModal({ visible, rule, mailboxes, onSave, onClose }: F const mailboxTargets = useMemo(() => buildMailboxTargets(mailboxes), [mailboxes]); const fieldOptions = useMemo( - () => ALL_FIELDS.map((f) => ({ value: f, label: t(`settings.filters.condition_fields.${f}`, f) })), + () => CONDITION_FIELDS.map((f) => ({ value: f, label: t(`settings.filters.condition_fields.${f}`, f) })), [t], ); const actionTypeOptions = useMemo( @@ -130,19 +118,10 @@ export function FilterRuleModal({ visible, rule, mailboxes, onSave, onClose }: F setConditions((prev) => prev.map((cond, i) => { if (i !== index) return cond; + if (updates.field) return conditionForField({ ...cond, ...updates }, updates.field); const updated = { ...cond, ...updates }; - // Reconcile the comparator when the field family changes so a size - // rule never keeps "contains" and an attachment rule never keeps - // "greater than". - if (updates.field && !comparatorsFor(updates.field).includes(updated.comparator)) { - updated.comparator = comparatorsFor(updates.field)[0]; - } - if (updates.field && updates.field !== 'header') { - delete updated.headerName; - } - if (isHasAny(updated)) { - updated.value = ''; - } + // Picking has_any drops whatever value was typed. + if (isValueLessCondition(updated)) updated.value = ''; return updated; }), ); @@ -188,18 +167,7 @@ export function FilterRuleModal({ visible, rule, mailboxes, onSave, onClose }: F Alert.alert(t('settings.filters.validation_empty_name', 'Rule name is required')); return; } - // While editing, condition.value is the raw string typed into the input - // (commas not yet split). Convert to array form here on save so a user - // typing "a, b, c" persists ["a","b","c"]; splitting on every keystroke - // would eat the comma the moment it is typed. - const validConditions = conditions - .filter((cond) => isHasAny(cond) || !isConditionValueEmpty(cond.value)) - .map((cond) => { - if (isHasAny(cond)) return { ...cond, value: '' }; - if (cond.field === 'size') return cond; // numeric, single-value only - if (typeof cond.value !== 'string') return cond; // already structured - return { ...cond, value: inputStringToValue(cond.value) }; - }); + const validConditions = conditionsToSave(conditions); if (validConditions.length === 0) { Alert.alert(t('settings.filters.validation_empty_conditions', 'At least one condition with a value is required')); return; @@ -316,15 +284,17 @@ export function FilterRuleModal({ visible, rule, mailboxes, onSave, onClose }: F /> )} - updateCondition(index, { comparator: v as FilterComparator })} + options={comparatorOptions(condition.field)} + style={{ alignSelf: 'flex-start' }} + /> + )} - {/* has_any takes no value ("an attachment is present"). */} - {!isHasAny(condition) && ( + {/* has_any and "all messages" take no value. */} + {!isValueLessCondition(condition) && ( updateCondition(index, { value: v })} diff --git a/src/lib/sieve/__tests__/condition-options.test.ts b/src/lib/sieve/__tests__/condition-options.test.ts new file mode 100644 index 00000000..2da429e1 --- /dev/null +++ b/src/lib/sieve/__tests__/condition-options.test.ts @@ -0,0 +1,59 @@ +import { describe, expect, it } from 'vitest'; +import { + CONDITION_FIELDS, + comparatorsFor, + conditionForField, + conditionsToSave, +} from '../condition-options'; +import type { FilterCondition } from '../types'; + +const ALL: FilterCondition = { field: 'all', comparator: 'any', value: '' }; + +describe('condition options', () => { + it('offers All messages last', () => { + expect(CONDITION_FIELDS[CONDITION_FIELDS.length - 1]).toBe('all'); + }); + + it('ends the From/To/Cc comparators with address_is and domain_is', () => { + for (const f of ['from', 'to', 'cc'] as const) { + expect(comparatorsFor(f).slice(-2)).toEqual(['address_is', 'domain_is']); + } + expect(comparatorsFor('subject')).not.toContain('address_is'); + expect(comparatorsFor('subject')).not.toContain('domain_is'); + }); + + it('maps an existing all condition to the any comparator', () => { + expect(comparatorsFor('all')).toEqual(['any']); + }); + + it('turns a row into the exact all-messages condition', () => { + const prev: FilterCondition = { field: 'header', headerName: 'X-A', comparator: 'is', value: 'x' }; + expect(conditionForField(prev, 'all')).toEqual(ALL); + }); + + it('resets comparator and value when leaving all', () => { + expect(conditionForField(ALL, 'from')).toEqual({ field: 'from', comparator: 'contains', value: '' }); + }); + + it('keeps a comparator that is valid for the new field', () => { + const prev: FilterCondition = { field: 'from', comparator: 'address_is', value: 'a@b.c' }; + expect(conditionForField(prev, 'to')).toEqual({ field: 'to', comparator: 'address_is', value: 'a@b.c' }); + }); + + it('replaces a comparator the new field lacks and drops headerName', () => { + const prev: FilterCondition = { field: 'header', headerName: 'X-A', comparator: 'contains', value: 'x' }; + expect(conditionForField(prev, 'size')).toEqual({ field: 'size', comparator: 'greater_than', value: 'x' }); + }); + + it('saves a rule that only has an all-messages condition', () => { + expect(conditionsToSave([ALL])).toEqual([ALL]); + }); + + it('drops empty-valued conditions and splits comma lists', () => { + const out = conditionsToSave([ + { field: 'from', comparator: 'contains', value: '' }, + { field: 'to', comparator: 'is', value: 'a, b' }, + ]); + expect(out).toEqual([{ field: 'to', comparator: 'is', value: ['a', 'b'] }]); + }); +}); diff --git a/src/lib/sieve/condition-options.ts b/src/lib/sieve/condition-options.ts new file mode 100644 index 00000000..8dc10a97 --- /dev/null +++ b/src/lib/sieve/condition-options.ts @@ -0,0 +1,54 @@ +import { + inputStringToValue, + isConditionValueEmpty, + isValueLessCondition, +} from './condition-value'; +import type { FilterComparator, FilterCondition, FilterConditionField } from './types'; + +// "All messages" goes last, as in the webmail. +export const CONDITION_FIELDS: FilterConditionField[] = [ + 'from', 'to', 'cc', 'subject', 'header', 'size', 'body', 'attachment', 'all', +]; +const TEXT_COMPARATORS: FilterComparator[] = ['contains', 'not_contains', 'is', 'not_is', 'starts_with', 'ends_with', 'matches']; +const ADDRESS_COMPARATORS: FilterComparator[] = [...TEXT_COMPARATORS, 'address_is', 'domain_is']; +const SIZE_COMPARATORS: FilterComparator[] = ['greater_than', 'less_than']; +const ATTACHMENT_COMPARATORS: FilterComparator[] = ['has_any', 'has_type']; +const ALL_COMPARATORS: FilterComparator[] = ['any']; + +export function comparatorsFor(field: FilterConditionField): FilterComparator[] { + if (field === 'all') return ALL_COMPARATORS; + if (field === 'size') return SIZE_COMPARATORS; + if (field === 'attachment') return ATTACHMENT_COMPARATORS; + if (field === 'from' || field === 'to' || field === 'cc') return ADDRESS_COMPARATORS; + return TEXT_COMPARATORS; +} + +// What a condition row becomes when its field changes. +export function conditionForField(prev: FilterCondition, field: FilterConditionField): FilterCondition { + if (field === 'all') return { field: 'all', comparator: 'any', value: '' }; + const comparators = comparatorsFor(field); + if (prev.field === 'all') return { field, comparator: comparators[0], value: '' }; + const updated: FilterCondition = { ...prev, field }; + // Reconcile the comparator when the field family changes so a size + // rule never keeps "contains" and an attachment rule never keeps + // "greater than". + if (!comparators.includes(updated.comparator)) updated.comparator = comparators[0]; + if (field !== 'header') delete updated.headerName; + if (isValueLessCondition(updated)) updated.value = ''; + return updated; +} + +// Conditions worth saving. While editing, condition.value is the raw string +// typed into the input (commas not yet split); convert to array form here so +// "a, b, c" persists ["a","b","c"]. Splitting on every keystroke would eat +// the comma the moment it is typed. +export function conditionsToSave(conditions: FilterCondition[]): FilterCondition[] { + return conditions + .filter((cond) => isValueLessCondition(cond) || !isConditionValueEmpty(cond.value)) + .map((cond) => { + if (isValueLessCondition(cond)) return { ...cond, value: '' }; + if (cond.field === 'size') return cond; // numeric, single-value only + if (typeof cond.value !== 'string') return cond; // already structured + return { ...cond, value: inputStringToValue(cond.value) }; + }); +} From ef341b1074e533781101f249b3000a53c306e6bf Mon Sep 17 00:00:00 2001 From: waggins Date: Sun, 4 Oct 2026 10:01:44 -0400 Subject: [PATCH 11/20] fix: send a mailto unsubscribe to the one listed address and show it before sending Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01AdqB9PokeySGSsJ92sqngP --- src/components/email/UnsubscribeBanner.tsx | 20 ++++++------ src/lib/__tests__/unsubscribe.test.ts | 34 ++++++++++++++++++++ src/lib/unsubscribe.ts | 37 ++++++++++++++++++++++ 3 files changed, 82 insertions(+), 9 deletions(-) diff --git a/src/components/email/UnsubscribeBanner.tsx b/src/components/email/UnsubscribeBanner.tsx index 473c23eb..645d08c9 100644 --- a/src/components/email/UnsubscribeBanner.tsx +++ b/src/components/email/UnsubscribeBanner.tsx @@ -10,7 +10,7 @@ import { useLocaleStore } from '../../stores/locale-store'; import { useSettingsStore } from '../../stores/settings-store'; import { useEmailStore } from '../../stores/email-store'; import { sendEmail } from '../../api/email'; -import { isValidUnsubscribeUrl, parseMailtoUrl, isOneClickUnsubscribe } from '../../lib/unsubscribe'; +import { isValidUnsubscribeUrl, parseUnsubscribeMailto, unsubscribeConfirmDetails, isOneClickUnsubscribe } from '../../lib/unsubscribe'; import type { ListHeaders } from '../../lib/email-headers'; import { findReceivingIdentity } from '../../lib/email-headers'; import { mailboxesOfAccount } from '../../lib/mailbox-tree'; @@ -73,7 +73,11 @@ export function UnsubscribeBanner({ email, list, messageKey, jmapAccountId }: Pr return () => { cancelled = true; }; }, [messageKey]); - if (hidden || !url || !method) return null; + // The sender wrote this link: parse it once so the confirmation shows what + // will be sent, and hide the option when it isn't a single valid recipient. + const mailtoFields = method === 'mailto' && url ? parseUnsubscribeMailto(url) : null; + + if (hidden || !url || !method || (method === 'mailto' && !mailtoFields)) return null; const dismiss = () => { dismissed.add(messageKey); @@ -97,8 +101,7 @@ export function UnsubscribeBanner({ email, list, messageKey, jmapAccountId }: Pr await WebBrowser.openBrowserAsync(url); } } else { - const fields = parseMailtoUrl(url); - if (!fields) throw new Error('invalid mailto'); + if (!mailtoFields) throw new Error('invalid mailto'); const identity = findReceivingIdentity(identities, email) ?? identities[0]; if (!identity) throw new Error(t('email_viewer.unsubscribe_banner.no_identity', 'No sending identity available')); // Sent/Drafts of the account the mail is submitted from: the message's. @@ -109,10 +112,9 @@ export function UnsubscribeBanner({ email, list, messageKey, jmapAccountId }: Pr await sendEmail( { from: [{ name: identity.name, email: identity.email }], - to: fields.to.map((address) => ({ email: address })), - cc: fields.cc?.map((address) => ({ email: address })), - subject: fields.subject ?? '', - textBody: fields.body ?? '', + to: [{ email: mailtoFields.to[0] }], + subject: mailtoFields.subject ?? '', + textBody: mailtoFields.body ?? '', }, identity.id, sent.originalId ?? sent.id, @@ -133,7 +135,7 @@ export function UnsubscribeBanner({ email, list, messageKey, jmapAccountId }: Pr t('email_viewer.unsubscribe_banner.confirm_title', 'Unsubscribe from this sender?'), method === 'http' ? t('email_viewer.unsubscribe_banner.confirm_message_http', 'The unsubscribe page will open in a new tab.') - : t('email_viewer.unsubscribe_banner.confirm_message_mailto', 'An unsubscribe email will be sent to the sender.'), + : `${t('email_viewer.unsubscribe_banner.confirm_message_mailto', 'An unsubscribe email will be sent to the sender.')}\n\n${mailtoFields ? unsubscribeConfirmDetails(mailtoFields) : ''}`, [ { text: t('email_viewer.unsubscribe_banner.cancel', 'Cancel'), style: 'cancel' }, { text: t('email_viewer.unsubscribe_banner.confirm_button', 'Confirm'), onPress: () => { void perform(); } }, diff --git a/src/lib/__tests__/unsubscribe.test.ts b/src/lib/__tests__/unsubscribe.test.ts index 7baf2fef..2477bd5b 100644 --- a/src/lib/__tests__/unsubscribe.test.ts +++ b/src/lib/__tests__/unsubscribe.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect } from 'vitest'; import { parseUnsubscribeUrls, isValidUnsubscribeUrl, parseMailtoUrl, isOneClickUnsubscribe, + parseUnsubscribeMailto, unsubscribeConfirmDetails, } from '../unsubscribe'; describe('parseUnsubscribeUrls', () => { @@ -46,3 +47,36 @@ describe('isOneClickUnsubscribe', () => { expect(isOneClickUnsubscribe(undefined, 'https://x/u')).toBe(false); }); }); + +describe('parseUnsubscribeMailto', () => { + it('takes exactly one recipient from the address part', () => { + expect(parseUnsubscribeMailto('mailto:leave@list.example?subject=unsubscribe')).toEqual({ to: ['leave@list.example'], subject: 'unsubscribe' }); + }); + it('refuses a list of addresses', () => { + expect(parseUnsubscribeMailto('mailto:a@x.example,b@y.example')).toBeNull(); + }); + it('ignores to= and cc= query fields', () => { + expect(parseUnsubscribeMailto('mailto:leave@list.example?to=ceo@corp.example&cc=boss@corp.example')) + .toEqual({ to: ['leave@list.example'] }); + }); + it('returns null without an address part, even with a to= field', () => { + expect(parseUnsubscribeMailto('mailto:?to=a@x.example')).toBeNull(); + expect(parseUnsubscribeMailto('https://x')).toBeNull(); + }); + it('keeps the subject on one line and caps subject and body', () => { + const r = parseUnsubscribeMailto(`mailto:l@x.example?subject=a%0D%0Ab&body=${'x'.repeat(600)}`)!; + expect(r.subject).toBe('a b'); + expect(r.body).toHaveLength(500); + expect(parseUnsubscribeMailto(`mailto:l@x.example?subject=${'s'.repeat(300)}`)!.subject).toHaveLength(200); + }); + it('parseMailtoUrl still accepts several addresses', () => { + expect(parseMailtoUrl('mailto:a@x.example,b@y.example')?.to).toEqual(['a@x.example', 'b@y.example']); + }); +}); + +describe('unsubscribeConfirmDetails', () => { + it('lists the recipient, then subject and body on their own lines', () => { + expect(unsubscribeConfirmDetails({ to: ['l@x.example'], subject: 'stop', body: 'please' })).toBe('l@x.example\nstop\nplease'); + expect(unsubscribeConfirmDetails({ to: ['l@x.example'] })).toBe('l@x.example'); + }); +}); diff --git a/src/lib/unsubscribe.ts b/src/lib/unsubscribe.ts index a2b2fb16..0339fd0a 100644 --- a/src/lib/unsubscribe.ts +++ b/src/lib/unsubscribe.ts @@ -117,3 +117,40 @@ export function parseMailtoUrl(url: string): MailtoFields | null { if (to.length === 0 && cc.length === 0) return null; return { to, cc: cc.length ? cc : undefined, subject, body }; } + +export const UNSUBSCRIBE_SUBJECT_MAX = 200; +export const UNSUBSCRIBE_BODY_MAX = 500; + +/** + * Parse a List-Unsubscribe mailto: URL for a one-click send from the user's + * own account. The sender wrote this URL, so it is held to what an + * unsubscribe request needs: exactly one recipient from the address part + * (to=/cc= fields are ignored, a list of addresses is refused), a + * single-line subject and a short body. + */ +export function parseUnsubscribeMailto(url: string): { to: [string]; subject?: string; body?: string } | null { + const parsed = parseMailtoUrl(url); + if (!parsed) return null; + + const rest = url.slice(7); + const queryIndex = rest.indexOf('?'); + const addressPart = queryIndex === -1 ? rest : rest.slice(0, queryIndex); + const addresses = addressPart.split(',').filter((a) => a.trim() !== ''); + if (addresses.length !== 1) return null; + let to: string; + try { + to = decodeURIComponent(addresses[0]).trim(); + } catch { + to = addresses[0].trim(); + } + if (!isValidEmail(to)) return null; + + const subject = parsed.subject?.replace(/[\r\n]+/g, ' ').trim().slice(0, UNSUBSCRIBE_SUBJECT_MAX) || undefined; + const body = parsed.body?.slice(0, UNSUBSCRIBE_BODY_MAX) || undefined; + return { to: [to], subject, body }; +} + +/** The recipient, then subject and body on their own lines, for the confirmation. */ +export function unsubscribeConfirmDetails(fields: { to: [string]; subject?: string; body?: string }): string { + return [fields.to[0], fields.subject, fields.body].filter((line): line is string => !!line).join('\n'); +} From 4d85f289c5c7a59baa26bb9b39b958030d151182 Mon Sep 17 00:00:00 2001 From: waggins Date: Sun, 4 Oct 2026 10:04:44 -0400 Subject: [PATCH 12/20] fix: stop reading a winmail.dat value list that makes no progress Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AdqB9PokeySGSsJ92sqngP --- src/lib/__tests__/tnef.test.ts | 36 ++++++++++++++++++++++++++++++++++ src/lib/tnef.ts | 20 ++++++++++++++++--- 2 files changed, 53 insertions(+), 3 deletions(-) diff --git a/src/lib/__tests__/tnef.test.ts b/src/lib/__tests__/tnef.test.ts index f63b42df..5fc852b7 100644 --- a/src/lib/__tests__/tnef.test.ts +++ b/src/lib/__tests__/tnef.test.ts @@ -13,6 +13,16 @@ function bytes(s: string): number[] { return Array.from(new TextEncoder().encode(s)); } +// The MAPI block sits in an attachment-level attribute; attAttachRendData first +// so the parser has a current attachment to apply it to. +function tnefWithAttachmentProps(mapi: number[]): Uint8Array { + return new Uint8Array([ + ...u32(0x223e9f78), 0x00, 0x00, + ...attr(0x02, 0x00069002, [0]), + ...attr(0x02, 0x00069005, mapi), + ]); +} + describe('parseTnef', () => { it('rejects non-TNEF data', () => { expect(parseTnef(new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8]))).toEqual({ body: null, htmlBody: null, attachments: [] }); @@ -67,6 +77,32 @@ describe('parseTnef', () => { }); }); +describe('parseTnef value-count bounds', () => { + const expectFast = (mapi: number[]) => { + const started = performance.now(); + expect(() => parseTnef(tnefWithAttachmentProps(mapi))).not.toThrow(); + expect(performance.now() - started).toBeLessThan(100); + }; + + it('stops on a truncated variable-length value instead of spinning on the count', () => { + // 1 prop, PT_BINARY id 0x3701, count 0xFFFFFFFF, then a length larger than what is left. + expectFast([...u32(1), 0x02, 0x01, 0x01, 0x37, ...u32(0xffffffff), ...u32(1000)]); + }); + + it('stops a multi-value fixed run that consumes nothing', () => { + // PT_MV_LONG with count 0xFFFFFFFF and only 2 bytes left. + expectFast([...u32(1), 0x03, 0x10, 0x00, 0x30, ...u32(0xffffffff), 0, 0]); + }); + + it.each([ + ['multi-valued STRING8', 0x101e], + ['multi-valued unknown fixed type', 0x1099], + ])('returns at once for a %s property claiming 0xFFFFFFFF values', (_label, propType) => { + // Two bytes are too short to hold even one value. + expectFast([...u32(1), propType & 0xff, propType >> 8, 0x01, 0x00, ...u32(0xffffffff), 0, 0]); + }); +}); + describe('isTnefAttachment', () => { it('matches by name or type', () => { expect(isTnefAttachment('WINMAIL.DAT', 'application/octet-stream')).toBe(true); diff --git a/src/lib/tnef.ts b/src/lib/tnef.ts index 84a436b4..3bcb42ed 100644 --- a/src/lib/tnef.ts +++ b/src/lib/tnef.ts @@ -165,6 +165,15 @@ function decodeMAPIString(data: Uint8Array, propType: number): string { return decodeUtf8(data.subarray(0, len)); } +/** + * Read a multi-value count. Every value takes at least four bytes, so a count + * larger than what is left of the block is a lie; clamp it rather than loop + * up to 2^32 times on the UI thread. + */ +function readValueCount(r: BinaryReader): number { + return Math.min(r.readUint32LE(), Math.floor(r.remaining / 4)); +} + function parseMAPIProps(data: Uint8Array): Map { const props = new Map(); const r = new BinaryReader(data); @@ -198,19 +207,24 @@ function parseMAPIProps(data: Uint8Array): Map 0; j++) { - lastValue = readMAPIVarValue(r); + const value = readMAPIVarValue(r); + // A truncated value ends the list; retrying it would only re-read the same bytes. + if (value === null) break; + lastValue = value; } if (!isMultiValue && lastValue) { props.set(propID, { type: propType, value: lastValue }); } } else if (isMultiValue) { if (r.remaining < 4) break; - const valueCount = r.readUint32LE(); + const valueCount = readValueCount(r); for (let j = 0; j < valueCount && r.remaining > 0; j++) { + const before = r.remaining; readMAPIFixedValue(r, baseType); + if (r.remaining >= before) break; } } else { const value = readMAPIFixedValue(r, baseType); From 0c07c685400851c06d8f240386d2006cb894bd79 Mon Sep 17 00:00:00 2001 From: waggins Date: Sun, 4 Oct 2026 10:06:57 -0400 Subject: [PATCH 13/20] fix: judge an invitation's sender by the receiving server's own authentication results Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AdqB9PokeySGSsJ92sqngP --- src/lib/__tests__/calendar-invitation.test.ts | 30 +++++++++- src/lib/calendar-invitation.ts | 57 +++---------------- 2 files changed, 37 insertions(+), 50 deletions(-) diff --git a/src/lib/__tests__/calendar-invitation.test.ts b/src/lib/__tests__/calendar-invitation.test.ts index ab1b65c7..9c123a29 100644 --- a/src/lib/__tests__/calendar-invitation.test.ts +++ b/src/lib/__tests__/calendar-invitation.test.ts @@ -8,8 +8,8 @@ import { getEmailAuthenticationResults, getInvitationMethod, getInvitationTrustAssessment, - parseAuthenticationResults, } from '../calendar-invitation'; +import { parseAuthenticationResults } from '../email-headers'; const request = { organizerCalendarAddress: 'mailto:alice@example.com', @@ -72,6 +72,34 @@ describe('authentication results', () => { expect(parseAuthenticationResults('spf=pass smtp.mailfrom=a.com; spf=fail smtp.helo=b.com').spf?.result).toBe('fail'); }); + it('does not take a pass from a lower, sender-written header', () => { + const forged = email({ + from: [{ email: 'alice@example.com' }], + headers: [ + { name: 'Authentication-Results', value: 'mx.example; spf=fail smtp.mailfrom=evil.example; dkim=none; dmarc=fail header.from=bank.example' }, + { name: 'Authentication-Results', value: 'evil.example; dkim=pass header.d=bank.example; dmarc=pass header.from=bank.example' }, + ], + }); + const auth = getEmailAuthenticationResults(forged); + expect(auth?.dmarc?.result).toBe('fail'); + expect(auth?.dkim?.result).not.toBe('pass'); + expect(getInvitationTrustAssessment(request, forged, 'request').level).toBe('warning'); + }); + + it('does not let a lower header fill a mechanism the topmost one omits', () => { + const forged = email({ + from: [{ email: 'alice@example.com' }], + headers: [ + { name: 'Authentication-Results', value: 'mx.example; spf=none smtp.mailfrom=evil.example' }, + { name: 'Authentication-Results', value: 'evil.example; dkim=pass header.d=example.com; dmarc=pass header.from=example.com' }, + ], + }); + const auth = getEmailAuthenticationResults(forged); + expect(auth?.dkim?.result).not.toBe('pass'); + expect(auth?.dmarc?.result).not.toBe('pass'); + expect(getInvitationTrustAssessment(request, forged, 'request').reason).toBe('authentication_missing'); + }); + it('reads the Authentication-Results header from the email headers', () => { const e = email({ headers: [{ name: 'Authentication-Results', value: 'x; dmarc=fail header.from=evil.com' }] }); expect(getEmailAuthenticationResults(e)?.dmarc?.result).toBe('fail'); diff --git a/src/lib/calendar-invitation.ts b/src/lib/calendar-invitation.ts index b5bdee51..de4215c2 100644 --- a/src/lib/calendar-invitation.ts +++ b/src/lib/calendar-invitation.ts @@ -1,4 +1,5 @@ import type { CalendarEvent, Participant, Email, Attachment, BodyPart, EmailAddress } from '../api/types'; +import { headerValues, parseAuthenticationResults, type AuthenticationResults } from './email-headers'; // ─── Address helpers ───────────────────────────────────── @@ -266,60 +267,18 @@ export function calendarInvitationKey( // ─── Authentication-Results / trust ────────────────────── -export interface AuthenticationResults { - spf?: { result: string; domain?: string }; - dkim?: { result: string; domain?: string; selector?: string }; - dmarc?: { result: string; domain?: string; policy?: string }; -} - -/** Parse an Authentication-Results header into SPF / DKIM / DMARC results. */ -export function parseAuthenticationResults(header: string): AuthenticationResults { - const results: AuthenticationResults = {}; - // A header can carry several SPF results (HELO and MAIL FROM); the MAIL FROM - // identity is primary, another identity may only escalate to a failure. - const spfRegex = /spf=(\w+)(?:\s+\([^)]*\))?(?:\s+smtp\.(mailfrom|helo)=([^\s;]+))?/g; - const severity: Record = { - fail: 6, softfail: 5, permerror: 4, temperror: 3, neutral: 2, none: 1, pass: 0, - }; - const spf: Array<{ result: string; identity?: string; domain?: string }> = []; - let m: RegExpExecArray | null; - while ((m = spfRegex.exec(header)) !== null) { - spf.push({ result: m[1].toLowerCase(), identity: m[2], domain: m[3] }); - } - if (spf.length > 0) { - let primary = spf.find((e) => e.identity === 'mailfrom') ?? spf[0]; - for (const cur of spf) { - const s = severity[cur.result] ?? -1; - if (s >= severity.temperror && s > (severity[primary.result] ?? -1)) primary = cur; - } - results.spf = { result: primary.result, domain: primary.domain }; - } - const dkim = header.match(/dkim=(\w+)(?:\s+header\.d=([^\s;]+))?(?:\s+header\.s=([^\s;]+))?/); - if (dkim) results.dkim = { result: dkim[1].toLowerCase(), domain: dkim[2], selector: dkim[3] }; - const dmarc = header.match(/dmarc=(\w+)(?:\s+header\.from=([^\s;]+))?(?:\s+policy\.dmarc=(\w+))?/); - if (dmarc) results.dmarc = { result: dmarc[1].toLowerCase(), domain: dmarc[2], policy: dmarc[3] }; - return results; -} - -/** Authentication results of an email, derived from its raw headers. */ +/** + * Authentication results of an email, derived from its raw headers. Uses the + * mail viewer's rule: only the topmost header (our own server's) can supply a + * pass; lower, sender-written headers may only escalate SPF to a failure. + */ export function getEmailAuthenticationResults( email?: Pick | null, ): AuthenticationResults | null { if (!email?.headers) return null; - const values = email.headers - .filter((h) => h.name.toLowerCase() === 'authentication-results') - .map((h) => h.value); + const values = headerValues(email.headers, 'Authentication-Results'); if (values.length === 0) return null; - // Merge: the first (outermost, added by our own server) header wins per - // mechanism; later ones only fill gaps. - const merged: AuthenticationResults = {}; - for (const value of values) { - const parsed = parseAuthenticationResults(value); - if (!merged.spf && parsed.spf) merged.spf = parsed.spf; - if (!merged.dkim && parsed.dkim) merged.dkim = parsed.dkim; - if (!merged.dmarc && parsed.dmarc) merged.dmarc = parsed.dmarc; - } - return merged; + return parseAuthenticationResults(values); } function hasVerifiedAuthentication(auth?: AuthenticationResults | null): boolean { From c565dfe2b636ce88d499c1e2da71acc0fcfd2348 Mon Sep 17 00:00:00 2001 From: waggins Date: Sun, 4 Oct 2026 10:17:23 -0400 Subject: [PATCH 14/20] fix: refuse to save a filter size that is not a whole number with an optional K, M or G Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AdqB9PokeySGSsJ92sqngP --- locales/rn/en.json | 1 + src/components/filters/FilterRuleModal.tsx | 5 +++++ .../sieve/__tests__/condition-options.test.ts | 21 ++++++++++++++++++- src/lib/sieve/condition-options.ts | 8 +++++++ 4 files changed, 34 insertions(+), 1 deletion(-) diff --git a/locales/rn/en.json b/locales/rn/en.json index bd6fa032..c74af925 100644 --- a/locales/rn/en.json +++ b/locales/rn/en.json @@ -833,6 +833,7 @@ "condition_fields": { "all": "All messages" }, + "invalid_size": "Enter a size as a whole number, optionally followed by K, M or G (for example 500K or 10M).", "remove_action": "Remove action", "remove_condition": "Remove condition", "sieve_editor": { diff --git a/src/components/filters/FilterRuleModal.tsx b/src/components/filters/FilterRuleModal.tsx index f907ee63..2790dfa9 100644 --- a/src/components/filters/FilterRuleModal.tsx +++ b/src/components/filters/FilterRuleModal.tsx @@ -19,6 +19,7 @@ import { comparatorsFor, conditionForField, conditionsToSave, + isValidSizeValue, } from '../../lib/sieve/condition-options'; import { ACTIONS_WITH_MAILBOX, @@ -172,6 +173,10 @@ export function FilterRuleModal({ visible, rule, mailboxes, onSave, onClose }: F Alert.alert(t('settings.filters.validation_empty_conditions', 'At least one condition with a value is required')); return; } + if (validConditions.some((c) => c.field === 'size' && !isValidSizeValue(c.value))) { + Alert.alert(t('settings.filters.invalid_size', 'Enter a size as a whole number, optionally followed by K, M or G (for example 500K or 10M).')); + return; + } const validActions = actions .map((a) => withMailboxTarget(a, mailboxTargets)) .filter((a) => !ACTIONS_WITH_VALUE.has(a.type) || a.value?.trim()); diff --git a/src/lib/sieve/__tests__/condition-options.test.ts b/src/lib/sieve/__tests__/condition-options.test.ts index 2da429e1..ceeabb2e 100644 --- a/src/lib/sieve/__tests__/condition-options.test.ts +++ b/src/lib/sieve/__tests__/condition-options.test.ts @@ -4,6 +4,7 @@ import { comparatorsFor, conditionForField, conditionsToSave, + isValidSizeValue, } from '../condition-options'; import type { FilterCondition } from '../types'; @@ -42,7 +43,7 @@ describe('condition options', () => { it('replaces a comparator the new field lacks and drops headerName', () => { const prev: FilterCondition = { field: 'header', headerName: 'X-A', comparator: 'contains', value: 'x' }; - expect(conditionForField(prev, 'size')).toEqual({ field: 'size', comparator: 'greater_than', value: 'x' }); + expect(conditionForField(prev, 'size')).toEqual({ field: 'size', comparator: 'greater_than', value: '' }); }); it('saves a rule that only has an all-messages condition', () => { @@ -57,3 +58,21 @@ describe('condition options', () => { expect(out).toEqual([{ field: 'to', comparator: 'is', value: ['a', 'b'] }]); }); }); + +describe('size values', () => { + it('accepts a whole number with an optional K, M or G', () => { + for (const v of ['500', '10M', '2k']) expect(isValidSizeValue(v)).toBe(true); + }); + + it('rejects anything the generator would write as 0', () => { + for (const v of ['1.5M', '1,000,000', '5 MB', '']) expect(isValidSizeValue(v)).toBe(false); + expect(isValidSizeValue(['a@x.com'])).toBe(false); + }); + + it('drops the value when a row changes into or out of size', () => { + const from: FilterCondition = { field: 'from', comparator: 'contains', value: ['a@x.com', 'b@y.com'] }; + expect(conditionForField(from, 'size').value).toBe(''); + const size: FilterCondition = { field: 'size', comparator: 'greater_than', value: '10M' }; + expect(conditionForField(size, 'subject').value).toBe(''); + }); +}); diff --git a/src/lib/sieve/condition-options.ts b/src/lib/sieve/condition-options.ts index 8dc10a97..bb1433ee 100644 --- a/src/lib/sieve/condition-options.ts +++ b/src/lib/sieve/condition-options.ts @@ -23,6 +23,12 @@ export function comparatorsFor(field: FilterConditionField): FilterComparator[] return TEXT_COMPARATORS; } +// The generator writes any other size as 0, which matches every message, so +// the editor must refuse it rather than save an "act on everything" rule. +export function isValidSizeValue(value: string | string[]): boolean { + return typeof value === 'string' && /^\d+[KMG]?$/i.test(value.trim()); +} + // What a condition row becomes when its field changes. export function conditionForField(prev: FilterCondition, field: FilterConditionField): FilterCondition { if (field === 'all') return { field: 'all', comparator: 'any', value: '' }; @@ -34,6 +40,8 @@ export function conditionForField(prev: FilterCondition, field: FilterConditionF // "greater than". if (!comparators.includes(updated.comparator)) updated.comparator = comparators[0]; if (field !== 'header') delete updated.headerName; + // An address list must never become a size, nor a size an address. + if ((field === 'size') !== (prev.field === 'size')) updated.value = ''; if (isValueLessCondition(updated)) updated.value = ''; return updated; } From 31900f5387b6e1f95fb55b17b5332565916591a8 Mon Sep 17 00:00:00 2001 From: waggins Date: Sun, 4 Oct 2026 10:17:57 -0400 Subject: [PATCH 15/20] fix: accept only one plain address in a mailto unsubscribe link Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AdqB9PokeySGSsJ92sqngP --- src/lib/__tests__/unsubscribe.test.ts | 17 +++++++++++++++++ src/lib/unsubscribe.ts | 15 +++++++++++++-- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/lib/__tests__/unsubscribe.test.ts b/src/lib/__tests__/unsubscribe.test.ts index 2477bd5b..ac6043e4 100644 --- a/src/lib/__tests__/unsubscribe.test.ts +++ b/src/lib/__tests__/unsubscribe.test.ts @@ -69,6 +69,23 @@ describe('parseUnsubscribeMailto', () => { expect(r.body).toHaveLength(500); expect(parseUnsubscribeMailto(`mailto:l@x.example?subject=${'s'.repeat(300)}`)!.subject).toHaveLength(200); }); + it('refuses an address that a server could split into several recipients', () => { + for (const url of [ + 'mailto:unsub@news.example%2Call-staff', + 'mailto:x%3E%2C%3Cvictim@evil.com', + 'mailto:x:ceo@corp.example;', + 'mailto:a%2Cb@x.com', + 'mailto:hr%E2%80%AE@corp.example', + ]) expect(parseUnsubscribeMailto(url)).toBeNull(); + }); + it('ignores repeated to= fields and refuses a non-address with a valid to=', () => { + expect(parseUnsubscribeMailto('mailto:boss@corp.example?to=hr@corp.example&to=press@news.example&subject=I%20resign')?.to) + .toEqual(['boss@corp.example']); + expect(parseUnsubscribeMailto('mailto:nobody?to=victim@corp.example')).toBeNull(); + }); + it('strips bidi and invisible characters from the subject', () => { + expect(parseUnsubscribeMailto('mailto:l@x.example?subject=un%E2%80%AEsub%E2%80%8Bscribe')?.subject).toBe('unsubscribe'); + }); it('parseMailtoUrl still accepts several addresses', () => { expect(parseMailtoUrl('mailto:a@x.example,b@y.example')?.to).toEqual(['a@x.example', 'b@y.example']); }); diff --git a/src/lib/unsubscribe.ts b/src/lib/unsubscribe.ts index 0339fd0a..8334d8cb 100644 --- a/src/lib/unsubscribe.ts +++ b/src/lib/unsubscribe.ts @@ -121,6 +121,17 @@ export function parseMailtoUrl(url: string): MailtoFields | null { export const UNSUBSCRIBE_SUBJECT_MAX = 200; export const UNSUBSCRIBE_BODY_MAX = 500; +// One dot-atom address (RFC 5322 atext local part, LDH domain labels). The +// loose isValidEmail lets "x>, Date: Sun, 4 Oct 2026 10:18:29 -0400 Subject: [PATCH 16/20] fix: keep the refused-recipients warning up longer and do not trust refused addresses Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AdqB9PokeySGSsJ92sqngP --- src/components/email/QuickReplyBox.tsx | 2 +- src/lib/__tests__/send-errors.test.ts | 12 +++++++++++- src/lib/send-errors.ts | 10 ++++++++++ src/screens/ComposeScreen.tsx | 6 +++--- 4 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/components/email/QuickReplyBox.tsx b/src/components/email/QuickReplyBox.tsx index 2473f176..2bef064f 100644 --- a/src/components/email/QuickReplyBox.tsx +++ b/src/components/email/QuickReplyBox.tsx @@ -136,7 +136,7 @@ export function QuickReplyBox({ email, jmapAccountId, onMoreOptions, onSent }: P if (result.rejectedRecipients?.length) { toast.warning( t('email_composer.send_some_recipients_rejected', 'Sent, but not to these recipients - the server rejected them.'), - formatRejectedRecipients(result.rejectedRecipients), + { message: formatRejectedRecipients(result.rejectedRecipients), duration: 10_000 }, ); } // The keyboard would cover the undo bar or the toast. diff --git a/src/lib/__tests__/send-errors.test.ts b/src/lib/__tests__/send-errors.test.ts index ae977677..ae3c2d8e 100644 --- a/src/lib/__tests__/send-errors.test.ts +++ b/src/lib/__tests__/send-errors.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import { sendErrorAlert } from '../send-errors'; +import { sendErrorAlert, withoutRefused } from '../send-errors'; import { RequestTimeoutError } from '../../api/jmap-client'; import { RecipientsRejectedError, SendUnconfirmedError, ScheduleTooLateError } from '../../api/jmap-result'; @@ -32,3 +32,13 @@ describe('sendErrorAlert', () => { expect(sendErrorAlert('x', t)).toEqual({ title: 'Send failed', message: 'Failed to send email' }); }); }); + +describe('withoutRefused', () => { + const to = [{ name: 'A', email: 'a@x.com' }, { email: 'Gone@Example.com' }]; + it('drops refused addresses, ignoring case', () => { + expect(withoutRefused(to, [{ email: 'gone@example.com' }])).toEqual([{ name: 'A', email: 'a@x.com' }]); + }); + it('keeps everyone when nothing was refused', () => { + expect(withoutRefused(to, undefined)).toEqual(to); + }); +}); diff --git a/src/lib/send-errors.ts b/src/lib/send-errors.ts index b90c2e09..14c2a88d 100644 --- a/src/lib/send-errors.ts +++ b/src/lib/send-errors.ts @@ -41,3 +41,13 @@ export function sendErrorAlert( message: e instanceof Error ? e.message : t('notifications.error_sending', 'Failed to send email'), }; } + +/** Recipients the server accepted: a refused address is not someone to trust. */ +export function withoutRefused( + recipients: T[], + refused: { email: string }[] | undefined, +): T[] { + if (!refused?.length) return recipients; + const gone = new Set(refused.map((r) => r.email.toLowerCase())); + return recipients.filter((r) => !gone.has(r.email.toLowerCase())); +} diff --git a/src/screens/ComposeScreen.tsx b/src/screens/ComposeScreen.tsx index 0ce83e39..fffc5a18 100644 --- a/src/screens/ComposeScreen.tsx +++ b/src/screens/ComposeScreen.tsx @@ -45,7 +45,7 @@ import { } from '../api/email'; import { jmapClient } from '../api/jmap-client'; import { formatRejectedRecipients } from '../api/jmap-result'; -import { sendErrorAlert } from '../lib/send-errors'; +import { sendErrorAlert, withoutRefused } from '../lib/send-errors'; import { uploadBlob, uploadBytes } from '../api/blob'; import { buildReplyRecipients, type ReplySource } from '../lib/reply-recipients'; import { buildReplySubject, buildForwardSubject } from '../lib/subject-prefix'; @@ -2133,7 +2133,7 @@ export default function ComposeScreen({ route, navigation }: Props) { if (isReplyLike && mode !== 'forward') { const settings = useSettingsStore.getState(); const contacts = useContactsStore.getState(); - for (const r of [...outgoing.to, ...(outgoing.cc ?? [])]) { + for (const r of withoutRefused([...outgoing.to, ...(outgoing.cc ?? [])], result.rejectedRecipients)) { settings.addTrustedSender(r.email); if (isTrustedSendersSyncOn(trustedSendersAddressBook, hasContacts)) { contacts.addToTrustedSendersBook(r.name ? `${r.name} <${r.email}>` : r.email).catch(() => undefined); @@ -2144,7 +2144,7 @@ export default function ComposeScreen({ route, navigation }: Props) { if (result.rejectedRecipients?.length) { toast.warning( t('email_composer.send_some_recipients_rejected', 'Sent, but not to these recipients - the server rejected them.'), - formatRejectedRecipients(result.rejectedRecipients), + { message: formatRejectedRecipients(result.rejectedRecipients), duration: 10_000 }, ); } if (scheduledAt && result.scheduled) { From 011eb76ce2d9b191919a402725bf88e172599da4 Mon Sep 17 00:00:00 2001 From: waggins Date: Sun, 4 Oct 2026 10:19:04 -0400 Subject: [PATCH 17/20] test: pin what happens to the old draft after an unconfirmed or partly refused send Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AdqB9PokeySGSsJ92sqngP --- .../__tests__/email-send-confirmation.test.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/api/__tests__/email-send-confirmation.test.ts b/src/api/__tests__/email-send-confirmation.test.ts index 94516420..37de07b0 100644 --- a/src/api/__tests__/email-send-confirmation.test.ts +++ b/src/api/__tests__/email-send-confirmation.test.ts @@ -100,6 +100,22 @@ describe('sendEmail delivery confirmation', () => { expect(destroyedIds()).toEqual([]); // the copy may be the only record that it went out }); + it('keeps the old draft too when the send is unconfirmed', async () => { + mockRequest.mockResolvedValueOnce({ methodResponses: [['Email/set', { created: { draft: { id: 'email-9' } } }, '0']] }); + await expect(sendEmail(OUTGOING, 'id-1', 'sent-1', undefined, { draftId: 'draft-1' })).rejects.toBeInstanceOf(SendUnconfirmedError); + expect(destroyedIds()).toEqual([]); + }); + + it('drops the old draft when only some recipients were refused', async () => { + respond([['EmailSubmission/get', { list: [{ deliveryStatus: { + 'ok@example.com': { delivered: 'queued', smtpReply: '250 2.1.5 OK' }, + 'gone@example.com': { delivered: 'no', smtpReply: '550 5.1.1 No such user' }, + } }] }, 'deliveryStatus']]); + mockRequest.mockResolvedValueOnce({ methodResponses: [['Email/set', { destroyed: ['draft-1'] }, '0']] }); + await sendEmail(OUTGOING, 'id-1', 'sent-1', undefined, { draftId: 'draft-1' }); + expect(destroyedIds()).toEqual(['draft-1']); + }); + it('treats a missing or failed read-back as a plain success', async () => { respond([['error', { type: 'unknownMethod' }, 'deliveryStatus']]); await expect(sendEmail(OUTGOING, 'id-1', 'sent-1')).resolves.toMatchObject({ emailSubmissionId: 'sub-9' }); From 071fe8bf1210bad5e372d6568cc9d5cbc0a51a71 Mon Sep 17 00:00:00 2001 From: waggins Date: Sun, 4 Oct 2026 10:21:06 -0400 Subject: [PATCH 18/20] docs: tick the phase 1 parity fixes with their commits Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AdqB9PokeySGSsJ92sqngP --- PARITY_CHECKLIST.md | 16 ++++++++-------- docs/parity/03-email-viewer.md | 6 +++--- docs/parity/04-composer-send.md | 6 +++--- docs/parity/07-filters-vacation-files.md | 6 +++--- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/PARITY_CHECKLIST.md b/PARITY_CHECKLIST.md index c26f14bb..bd40bf0f 100644 --- a/PARITY_CHECKLIST.md +++ b/PARITY_CHECKLIST.md @@ -93,7 +93,7 @@ range was checked against native `main` at `76180b3`, skipping what [docs/audit-2026-09.md](docs/audit-2026-09.md) already lists. The 74 new items sit in a "Webmail 1.10.0 → 1.12.0+ delta" section in each area file (3 P1, 24 P2, 47 P3); what was already at parity is one "1.10–1.12 delta" line at the -end of each area's Verified list. The three P1s, and the P2s worth doing next: +end of each area's Verified list. Phase 1 of the [roadmap](docs/superpowers/plans/2026-10-04-webmail-parity-roadmap.md) closed the three P1s and five security/send P2s below on branch `parity/phase-1-security-send`. The three P1s, and the P2s worth doing next: - P1: forged `Authentication-Results` can fake a DMARC/DKIM pass (03); refused recipients in `deliveryStatus` are never reported, so a send that reached no @@ -115,14 +115,14 @@ end of each area's Verified list. The three P1s, and the P2s worth doing next: |---|---|---|---|---|---|---|---|---| | 01 | Authentication, login, session, multi-account | [docs/parity/01-auth-accounts.md](docs/parity/01-auth-accounts.md) | 38 | 29 | 9 | 4 | 12 | 22 | | 02 | Mail list, folders, unified views, search, tags | [docs/parity/02-mail-list-folders.md](docs/parity/02-mail-list-folders.md) | 76 | 50 | 26 | 1 | 24 | 51 | -| 03 | Email viewer, thread view, rendering, attachments | [docs/parity/03-email-viewer.md](docs/parity/03-email-viewer.md) | 52 | 45 | 7 | 7 | 17 | 27 | -| 04 | Composer, drafts, sending, identities, templates, scheduled send | [docs/parity/04-composer-send.md](docs/parity/04-composer-send.md) | 60 | 46 | 14 | 7 | 16 | 37 | +| 03 | Email viewer, thread view, rendering, attachments | [docs/parity/03-email-viewer.md](docs/parity/03-email-viewer.md) | 52 | 48 | 4 | 7 | 17 | 27 | +| 04 | Composer, drafts, sending, identities, templates, scheduled send | [docs/parity/04-composer-send.md](docs/parity/04-composer-send.md) | 60 | 49 | 11 | 7 | 16 | 37 | | 05 | Calendar and tasks | [docs/parity/05-calendar.md](docs/parity/05-calendar.md) | 61 | 43 | 18 | 5 | 23 | 33 | | 06 | Contacts and address books | [docs/parity/06-contacts.md](docs/parity/06-contacts.md) | 53 | 46 | 7 | 1 | 21 | 31 | -| 07 | Filters (Sieve), vacation responder, Files | [docs/parity/07-filters-vacation-files.md](docs/parity/07-filters-vacation-files.md) | 38 | 29 | 9 | 3 | 11 | 24 | +| 07 | Filters (Sieve), vacation responder, Files | [docs/parity/07-filters-vacation-files.md](docs/parity/07-filters-vacation-files.md) | 38 | 32 | 6 | 3 | 11 | 24 | | 08 | Settings, sync, push, i18n, themes, updates, misc UI | [docs/parity/08-settings-push-i18n-ui.md](docs/parity/08-settings-push-i18n-ui.md) | 54 | 38 | 16 | 0 | 15 | 39 | | 09 | JMAP client core, live sync, offline, security, S/MIME | [docs/parity/09-jmap-core-sync-security.md](docs/parity/09-jmap-core-sync-security.md) | 57 | 54 | 3 | 7 | 23 | 27 | -| | **Total** | | **489** | **380** | **109** | **35** | **162** | **291** | +| | **Total** | | **489** | **389** | **100** | **35** | **162** | **291** | Counts are of the `- [ ]` and `- [x]` items per file as of 2026-10-04. Done and Open split them by tick; the P columns count the priority tags on those items @@ -166,9 +166,9 @@ Open split them by tick; the P columns count the priority tags on those items - [x] Webmail password handoff sends the clear-text password in a custom-scheme redirect fragment that any app can register; OAuth `state` uses `Math.random`; `server_url`/`token_endpoint` in the callback are trusted as-is. → [09](docs/parity/09-jmap-core-sync-security.md), [01](docs/parity/01-auth-accounts.md) *(fixed in 2c0dbd1)* ### Webmail 1.10–1.12 delta (2026-10-04) -- [ ] A forged lower `Authentication-Results` header can supply a DKIM/DMARC pass in the security badge. → [03](docs/parity/03-email-viewer.md) -- [ ] Recipients refused at RCPT TO (`deliveryStatus`, #1123) are never read back; a send that reached nobody shows as sent. → [04](docs/parity/04-composer-send.md) -- [ ] `splitRecipients` ignores escaped quotes, so a crafted display name splits off an extra recipient. → [04](docs/parity/04-composer-send.md) +- [x] A forged lower `Authentication-Results` header can supply a DKIM/DMARC pass in the security badge. → [03](docs/parity/03-email-viewer.md) *(fixed in f66084f, 0c07c68)* +- [x] Recipients refused at RCPT TO (`deliveryStatus`, #1123) are never read back; a send that reached nobody shows as sent. → [04](docs/parity/04-composer-send.md) *(fixed in 600f355, e113118)* +- [x] `splitRecipients` ignores escaped quotes, so a crafted display name splits off an extra recipient. → [04](docs/parity/04-composer-send.md) *(fixed in 5f9812a)* ### Repo health - [x] `npm test` is red on `main` (see Baseline health above). *(fixed in b84d4d8)* diff --git a/docs/parity/03-email-viewer.md b/docs/parity/03-email-viewer.md index 8c2fd2a8..ea496944 100644 --- a/docs/parity/03-email-viewer.md +++ b/docs/parity/03-email-viewer.md @@ -247,16 +247,16 @@ at `76180b3`. Items already listed in [../audit-2026-09.md](../audit-2026-09.md) are not repeated. "Unverified" means read from the code but not confirmed on a device. -- [ ] **A sender can fake a DMARC/DKIM pass in the security badge** — `P1` — `bugfix-parity` (1.11.0, security) +- [x] **A sender can fake a DMARC/DKIM pass in the security badge** — `P1` — `bugfix-parity` (1.11.0, security) — fixed in f66084f; calendar-invitation trust uses the same parser since 0c07c68 - What WEB does: splits each `Authentication-Results` header into results (skipping quotes and comments) and takes DKIM, DMARC and iprev only from the topmost header; a sender's own header can only downgrade SPF (`lib/email-headers.ts:40-145`, `lib/jmap/client.ts:2444-2448`). - What RN does: `deriveHeaderInfo` joins every `Authentication-Results` header with `; ` and takes the first regex match for `dkim=` / `dmarc=` (`src/lib/email-headers.ts:107-160,262-264`), so a header the sender added, or text inside a comment, can supply the pass. - Fix hint: port WEB's parser and the topmost-header rule; add tests with a forged lower header. -- [ ] **A `mailto:` unsubscribe can go to several addresses without showing them** — `P2` — `bugfix-parity` (1.11.0, security) +- [x] **A `mailto:` unsubscribe can go to several addresses without showing them** — `P2` — `bugfix-parity` (1.11.0, security) — fixed in ef341b1; strict single plain address in 31900f5 - What WEB does: sends to the single address in the link and shows recipient, subject and body before sending (`lib/validation.ts:180`, `components/email/unsubscribe-banner.tsx:44-49,164,198`). - What RN does: takes every comma-separated address plus `?to=`/`?cc=` and confirms with a generic alert (`src/lib/unsubscribe.ts:79-118`, `src/components/email/UnsubscribeBanner.tsx:99-140`). -- [ ] **A crafted `winmail.dat` can freeze the app** — `P2` — `bugfix-parity` (1.11.0, security) +- [x] **A crafted `winmail.dat` can freeze the app** — `P2` — `bugfix-parity` (1.11.0, security) — fixed in 4d85f28 - What WEB does: stops when a value fails to parse or makes no progress, and bounds the count with `readValueCount` (`lib/tnef.ts`). - What RN does: `parseMAPIProps` (`src/lib/tnef.ts:168-220`) can loop up to a sender-chosen 32-bit count on a truncated value, blocking the JS thread. diff --git a/docs/parity/04-composer-send.md b/docs/parity/04-composer-send.md index 0a0c9736..9ae581b7 100644 --- a/docs/parity/04-composer-send.md +++ b/docs/parity/04-composer-send.md @@ -288,15 +288,15 @@ at `76180b3`. Items already listed in [../audit-2026-09.md](../audit-2026-09.md) are not repeated. "Unverified" means read from the code but not confirmed on a device. -- [ ] **Recipients the server refuses at send are never reported** — `P1` — `bugfix-parity` (1.12.0, #1123) +- [x] **Recipients the server refuses at send are never reported** — `P1` — `bugfix-parity` (1.12.0, #1123) — fixed in 600f355; alerts and warning toast in e113118, e1c8b0c - What WEB does: Stalwart runs RCPT TO while creating the submission and records refusals in `deliveryStatus` while the create succeeds. WEB reads `deliveryStatus` back (`EmailSubmission/get` on `#creationId`); if every recipient was refused it fails the send, drops the Sent copy and keeps the draft, and if some were it warns and names them (`lib/jmap/client.ts` ~716-790: `deliveryStatusCall`, `rejectedRecipients`, `RecipientsRejectedError`). - What RN does: never reads `deliveryStatus` (`src/api/email.ts:1586-1680` `sendEmail`), so a send that reached nobody shows as sent. -- [ ] **A quote in a display name can create an extra recipient** — `P1` — `bugfix-parity` (1.11.0, security; lower exposure than WEB) +- [x] **A quote in a display name can create an extra recipient** — `P1` — `bugfix-parity` (1.11.0, security; lower exposure than WEB) — fixed in 5f9812a - What WEB does: honours escaped quotes when splitting (`lib/email-composer-utils.ts:283-289`). - What RN does: `splitRecipients` (`src/lib/recipients.ts:87`) toggles on every `"` and ignores `\"`, so `Support\", ceo@corp.example, \"x` splits off an address; `findTopLevelColon` (~:155) has the same gap. Callers: `ComposeScreen.tsx:948,2238`, `IdentitySettings.tsx:84` (pasted recipients and identity settings). -- [ ] **A send with no `EmailSubmission/set` response counts as a success** — `P2` — `bugfix-parity` (15740fa, 07625bc) +- [x] **A send with no `EmailSubmission/set` response counts as a success** — `P2` — `bugfix-parity` (15740fa, 07625bc) — fixed in 600f355; alert in e113118 - What WEB does: throws `SendUnconfirmedError`, keeps the draft and shows "check Sent before sending again". - What RN does: when the response has no submission entry, `sendEmail` returns success with an undefined `emailSubmissionId` (`src/api/email.ts:1644-1680`) and the composer closes as sent. diff --git a/docs/parity/07-filters-vacation-files.md b/docs/parity/07-filters-vacation-files.md index a2f88c55..b8361367 100644 --- a/docs/parity/07-filters-vacation-files.md +++ b/docs/parity/07-filters-vacation-files.md @@ -172,15 +172,15 @@ at `76180b3`. Items already listed in [../audit-2026-09.md](../audit-2026-09.md) are not repeated. "Unverified" means read from the code but not confirmed on a device. -- [ ] **Sieve values are not escaped (injection through rule names, header names, sizes)** — `P2` — `bugfix-parity` (1.11.0–1.11.1, security) +- [x] **Sieve values are not escaped (injection through rule names, header names, sizes)** — `P2` — `bugfix-parity` (1.11.0–1.11.1, security) — fixed in 2996c02, f11bfbd; invalid sizes refused at save in c565dfe - What WEB does: escapes the header name, checks sizes against `^\d+[KMG]?$` and collapses whitespace in rule names (`lib/sieve/generator.ts:47,79,343`). - What RN does: writes them raw (`src/lib/sieve/generator.ts:46-50,80-108`; `# Rule: ${rule.name}` at `:339`). A newline in a rule name can inject commands such as `redirect`; a name with doubled or trailing spaces is duplicated on every save, because the parser compares the trimmed name (`src/lib/sieve/parser.ts:838-841`). -- [ ] **"Stop processing" writes no `stop` after "Delete silently" or "Reject"** — `P2` — `bugfix-parity` (cbdc4de) +- [x] **"Stop processing" writes no `stop` after "Delete silently" or "Reject"** — `P2` — `bugfix-parity` (cbdc4de) — fixed in f15be2c - What RN does: skips `stop;` when the last action is `discard` or `reject` (`src/lib/sieve/generator.ts:361-364`), so a later rule still files the message. - Fix hint: only skip when the last action is `stop`. -- [ ] **Rules from the current webmail break or loosen on a native save** — `P2` — `bugfix-parity` (c55b9f4) +- [x] **Rules from the current webmail break or loosen on a native save** — `P2` — `bugfix-parity` (c55b9f4) — fixed in f15be2c; editor options in fd2f129 - What WEB does: has a `field: 'all'` condition ("matches every message") and `address_is` / `domain_is` comparators. - What RN does: the generator throws "Unsupported filter condition field" for `all` (`src/lib/sieve/generator.ts:86-88`), so once such a rule exists every native filter save fails; `address_is` / `domain_is` fall through to `header :contains` (`:107`), silently loosening the rule. Neither is in the UI (`src/lib/sieve/types.ts:23-38`, `FilterRuleModal.tsx:42`). From a680476b069311ea0e3ebbdb5578073a87339857 Mon Sep 17 00:00:00 2001 From: waggins Date: Sun, 4 Oct 2026 10:21:21 -0400 Subject: [PATCH 19/20] docs: list what phase 1 left for later Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AdqB9PokeySGSsJ92sqngP --- .../2026-10-04-webmail-parity-roadmap.md | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/docs/superpowers/plans/2026-10-04-webmail-parity-roadmap.md b/docs/superpowers/plans/2026-10-04-webmail-parity-roadmap.md index c2dbec79..e3eb1d43 100644 --- a/docs/superpowers/plans/2026-10-04-webmail-parity-roadmap.md +++ b/docs/superpowers/plans/2026-10-04-webmail-parity-roadmap.md @@ -137,3 +137,43 @@ each sweep stays in one area: - from audit-2026-09: icon badge, themes, Tabler icons, the favicon source. - **Security (09):** a screenshot / recent-apps protection option. - **Accounts (01):** ending the SSO session on sign-out, and the settings scope of shared accounts. + +## Phase 1 follow-ups (left open at merge, 2026-10-04) + +Phase 1 is done on `parity/phase-1-security-send`. The final review rated the items below "later". Pick them up with Phase 2 or when working in the same files. + +- **Before release, on a device:** + - Rule editor: create an "All messages → Mark as read" rule on the phone, confirm webmail shows the same rule, then save it once from each side; the script must not grow. + - A size like `1.5M` is refused with an alert. + - Send to a non-existent address alone (alert, composer stays open, nothing in Sent), then together with a real one (warning toast, message in Sent). + - Upstream to webmail: the same two input-validation gaps exist there: + - an invalid filter size is written as `0`, so "greater than" matches every message; + - the loose `isValidEmail` in `parseUnsubscribeMailto` lets `, < > : ;` and bidi characters through after decoding. +- **Authentication-Results:** + - Known limitation, the same as webmail: if the receiving server adds no Authentication-Results header, the sender's own header is treated as topmost. + - Follow-up for both clients: trust only a configured or learned authserv-id per account. + - Tests to add: `;` inside quotes or comments, `dkim/1=`, uppercase results, empty or authserv-id-only headers, a lower `iprev=pass` being ignored, a lower `spf=fail` escalating through `getEmailAuthenticationResults`. +- **Send:** + - Inline `deliveryStatus` shape vs the unexported `DeliveryStatus` type in `src/api/jmap-result.ts`. + - Test `sendErrorAlert` with several refused recipients. + - The unsubscribe banner shows a generic error for an unconfirmed send. +- **Sieve:** + - Commit a webmail↔native round-trip fixture covering `all`, `address_is`/`domain_is` and discard + stop. + - Tests: + - an empty or whitespace-only rule name; + - a metadata-less `if true` / `address` script resaving byte-identically; + - a custom header with `address_is`; + - an attachment `has_any` row switched to From. + - The "condition with a value is required" alert text. + - The parser reads back only plain-digit sizes (as webmail does). + - An empty size row is silently dropped. +- **Recipients:** the colon test doesn't reach `findTopLevelColon` (the input needs a trailing `;`). +- **Unsubscribe:** + - Uppercase `MAILTO:`/`HTTPS:` are ignored. + - The first mailto that fails the strict parse hides the banner even when a later one would work. + - There are three copies of `parseMailtoUrl`/`isValidEmail` (`unsubscribe.ts`, `mailto.ts`, `recipients.ts`). +- **TNEF:** + - Add a positive multi-value parse test. + - Port webmail's truncated-attribute test. + - Assert on parse results, not only on timing. +- **Calendar trust:** `hasVerifiedAuthentication` accepts an unaligned DKIM/SPF pass (as webmail does). From 429d6f88dc9ecbb5b552cd481dcbad4e328cb58f Mon Sep 17 00:00:00 2001 From: waggins Date: Sun, 4 Oct 2026 10:45:16 -0400 Subject: [PATCH 20/20] docs: plan phase 2 of the webmail parity work Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AdqB9PokeySGSsJ92sqngP --- ...6-10-04-parity-phase-2-data-correctness.md | 739 ++++++++++++++++++ 1 file changed, 739 insertions(+) create mode 100644 docs/superpowers/plans/2026-10-04-parity-phase-2-data-correctness.md diff --git a/docs/superpowers/plans/2026-10-04-parity-phase-2-data-correctness.md b/docs/superpowers/plans/2026-10-04-parity-phase-2-data-correctness.md new file mode 100644 index 00000000..0ddbeef0 --- /dev/null +++ b/docs/superpowers/plans/2026-10-04-parity-phase-2-data-correctness.md @@ -0,0 +1,739 @@ +# Parity Phase 2: Data Correctness Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Stop the app from making writes the server silently refuses, writing into the wrong account, or rewriting data. That covers: +- contacts that Stalwart rejects; +- address books that can't be deleted; +- moved mail that loses its date; +- drafts that follow an account switch; +- screens and caches that outlive their account; +- calendar subscriptions shared across logins; +- daily events that stop at DST; +- events that can't be saved when invitations are refused; +- the Files edge cases. + +**Architecture:** Each task ports a fix the webmail already shipped (1.10.0–1.12.0+). Pure mapping and validation logic goes in `src/lib/*` and is unit-tested. API calls in `src/api/*` take explicit account ids. Stores own their per-account cleanup, and `auth-store` calls them. Screens stay thin: tests run in vitest's node environment with no React Native render harness, so any screen logic worth testing is pulled into a pure function first. + +**Tech Stack:** React Native / Expo, TypeScript, Zustand (persist with AsyncStorage), vitest (`npm test`), and JMAP against Stalwart (RFC 8620/8621, JSContact RFC 9553, JSCalendar, FileNode). + +**Spec:** the Phase 2 rows of [2026-10-04-webmail-parity-roadmap.md](2026-10-04-webmail-parity-roadmap.md). Each row's finding, with WEB and RN pointers, is in the "Webmail 1.10.0 → 1.12.0+ delta" sections of these files in `docs/parity/`: +- [01-auth-accounts.md](../../parity/01-auth-accounts.md) +- [02-mail-list-folders.md](../../parity/02-mail-list-folders.md) +- [04-composer-send.md](../../parity/04-composer-send.md) +- [05-calendar.md](../../parity/05-calendar.md) +- [06-contacts.md](../../parity/06-contacts.md) +- [07-filters-vacation-files.md](../../parity/07-filters-vacation-files.md) + +Webmail had no new commits after `a4e313f` on 2026-10-04, so the delta is current. + +## Global Constraints + +- **Branch:** create `parity/phase-2-data-correctness` from `parity/phase-1-security-send`. Phase 1 isn't merged yet, and this branch builds on its docs, its `src/api/jmap-result.ts` errors and its Sieve changes. Rebase onto `main` once Phase 1 merges. +- **Webmail reference checkout:** `git clone https://github.com/bulwarkmail/webmail && git -C checkout a4e313f`. Below, "WEB `path`" means a path in that checkout. +- **Gate on every commit:** `npm run typecheck && npm test && npm run i18n:check` must all pass. +- **User-visible strings:** use `t('key', 'English fallback')`. Reuse the webmail key when the plan names one. If `i18n:check` reports a missing key, run `npm run i18n:harvest`. Keys looked up dynamically (template strings) can't be harvested, so add them to `locales/rn/en.json` by hand when the vendored catalog lacks them. +- **Commits:** one per task, using the given subject. End the message with a `Co-Authored-By:` trailer naming the model that wrote it. +- **Parity docs:** do not edit `docs/parity/*.md` or `PARITY_CHECKLIST.md` inside a task. Tick the items with their commit hashes in one `docs:` commit after the final review, as Phase 1 did. +- **Test locations:** vitest only runs `src/**/__tests__/**/*.test.ts`, in a node environment. There is no `.tsx` component test harness. +- **Never** change how data already on the server is read in a way that loses fields. Every new write mapping needs its read counterpart in the same task. + +## Review Focus + +1. **Contacts the server sends without `calendars` / `schedulingAddresses` / `directories`** must load, show and save unchanged. `contactFromWire` returns the same card, and an edit that never touched calendar links sends no `calendars` key. Task 1 owns this: test `leaves calendar links alone when the update does not mention them`, plus `returns a card without links unchanged`. +2. **iCal subscriptions saved before this phase**, with no `owner`, must still appear for the login that created them after the store migration. They must not vanish, and they must not leak to other logins. Task 7 owns this: test `adopts a legacy subscription only for the login whose calendars contain it`. +3. **A composer opened and used without any account switch** must make exactly the calls it makes today, with the same account id. Task 4 owns this: test `resolves to the active account's mailboxes when nothing switched`. +4. **Weekly, monthly and timed events in zones with no DST transition in range** must expand exactly as before. Task 6 owns this: the existing `recurrence-expansion.test.ts` stays green unchanged, plus `a daily series in UTC is unchanged`. +5. **Signing out of one account** must leave the other signed-in accounts' offline bodies, outbox and subscriptions intact. Task 8 owns this: test `forgets only the signed-out account's data`. + +--- + +### Task 1: Write contacts in the shape Stalwart accepts, and read the links back + +**Files:** +- Create: `src/lib/contact-wire.ts` +- Modify: + - `src/api/types.ts:341-371` (`ContactCard`: add `calendars?`, `schedulingAddresses?`) + - `src/api/contacts.ts:83-89` (`stripClientFields`), `:157-173` (`getContacts`), `:220-282` (`createContact`, `updateContact`), and any other function that returns cards from `ContactCard/get` (`getContact`, `getAllContacts` via `tagContact` ~`:176-198`) +- Test: + - `src/lib/__tests__/contact-wire.test.ts` (new) + - `src/api/__tests__/contacts.test.ts` + +**Interfaces:** +- Produces: + - `contactToWire(card: Partial, mode: 'create' | 'update'): Record` + - `contactFromWire(card: T): T` +- Both are ports of WEB `lib/jmap/contact-wire.ts`, with the same behaviour line for line, including the private `addressToWire`. +- Client-only keys dropped on write: `originalId`, `accountId`, `accountName`, `isShared`, `localAccountId`. +- Flat keys dropped on write: `calendarUri`, `freeBusyUri`, `schedulingUri`, `source`. + +- [ ] **Step 1: Write the failing tests** + +Port WEB `lib/__tests__/contact-wire.test.ts` unchanged. It covers: +- `drops client-only fields, which ContactCard/set rejects` +- `writes the calendar URIs as RFC 9553 calendars and schedulingAddresses` +- `sends null for cleared fields on update, but omits them on create` +- `leaves calendar links alone when the update does not mention them` +- `converts flat vCard addresses to components and SOURCE to a directory entry` +- `fills the flat URI fields from the server card` + +Add `returns a card without links unchanged`: `contactFromWire(card)` is `toBe(card)` for a card with no `calendars`, `schedulingAddresses` or `directories`. + +In `contacts.test.ts`, add `createContact sends calendars, not calendarUri`. Create with `{ name, calendarUri: 'https://c.example/cal' }`, then assert `create['new-contact'].calendars.cal` equals `{ '@type': 'Calendar', kind: 'calendar', uri: 'https://c.example/cal' }` and `calendarUri` is undefined. + +Add `getContacts fills calendarUri from calendars`. + +- [ ] **Step 2: Run to verify they fail** + +Run `npx vitest run src/lib/__tests__/contact-wire.test.ts src/api/__tests__/contacts.test.ts`. + +Expected: FAIL, because the module is missing and the API tests fail. + +- [ ] **Step 3: Implement** + +- Port the module. +- Add to `ContactCard`: + - `calendars?: Record` + - `schedulingAddresses?: Record` +- In `createContact` and `updateContact`, replace `stripClientFields(...)` with `contactToWire(..., 'create' | 'update')`. Delete `stripClientFields` if nothing else uses it. +- Pass every card returned from `ContactCard/get` through `contactFromWire`. +- `ContactFormScreen`, `ContactDetailScreen` and `vcard.ts` keep using the flat fields: the mapping layer is the only place the wire shape exists. +- vCard import (`contacts-store.ts:471-479` → `createContact`) now goes through `contactToWire` too. That fixes the import finding with no change to `vcard.ts`. + +- [ ] **Step 4: Run to verify they pass** + +Run `npx vitest run src/lib src/api src/stores` and then `npm run typecheck`. + +Expected: PASS. + +- [ ] **Step 5: Commit** + +`fix: save contacts' calendar links and vCard addresses in the form Stalwart accepts` + +Device check: on Stalwart, add a calendar link to a contact, save, reopen and confirm the link is shown. Import a vCard that has `ADR`, `CALURI` and `SOURCE`, and confirm it imports. + +--- + +### Task 2: Delete an address book together with its contacts + +**Files:** +- Modify: `src/api/contacts.ts:362-370` (`deleteAddressBook`), `src/stores/contacts-store.ts:628-642` (and the comment at `:631`) +- Test: `src/api/__tests__/contacts.test.ts` (`describe('deleteAddressBook')` ~`:365`) + +**Interfaces:** +- Produces: `deleteAddressBook(id: string, accountId?: string, options?: { removeContents?: boolean }): Promise`. It matches WEB `lib/jmap/client.ts:5821-5842`. + +- [ ] **Step 1: Write the failing test** + +Add `asks the server to remove the contents when told to`: call `deleteAddressBook('ab-1', undefined, { removeContents: true })` and assert `call[1].onDestroyRemoveContents === true`. + +Add `sends no onDestroyRemoveContents by default`. + +- [ ] **Step 2: Run to verify it fails** + +Run `npx vitest run src/api/__tests__/contacts.test.ts`. + +Expected: the first new test FAILS. + +- [ ] **Step 3: Implement** + +Spread `...(options?.removeContents ? { onDestroyRemoveContents: true } : {})` into the `AddressBook/set` args. + +The store's `deleteAddressBook` passes `{ removeContents: true }`. The confirm dialog in `ContactsSettings.tsx:394-407` already says the contacts go with the book. Fix the store comment so it says the request asks for this explicitly (RFC 9610 §2.3). + +- [ ] **Step 4: Run to verify it passes** + +Run the same command. Expected: PASS. + +- [ ] **Step 5: Commit** + +`fix: delete an address book that still has contacts` + +--- + +### Task 3: Keep a moved message's date when it changes account + +**Files:** +- Modify: `src/api/email.ts:976-989` (`importEmailBlob`), `src/stores/email-store.ts:2175-2190` (`crossAccountMove`) +- Test: + - `src/stores/__tests__/email-store.test.ts` (cross-account move test ~`:1436-1450`) + - `src/api/__tests__/email.test.ts` + +**Interfaces:** +- Produces: `importEmailBlob(blobId: string, mailboxId: string, keywords?: Record, accountIdOverride?: string, receivedAt?: string): Promise`. The two other callers (`email-store.ts:1165` .eml import, `email.ts:2039` sent-copy filing) do not pass it. + +- [ ] **Step 1: Write the failing tests** + +- In `email.test.ts`, add `importEmailBlob sends receivedAt when given`: the `Email/import` entry for `import-0` has `receivedAt: '2026-01-02T03:04:05Z'`. Also add `importEmailBlob omits receivedAt when not given`. +- In `email-store.test.ts`, give the moved row `receivedAt: '2026-01-02T03:04:05Z'` and extend the existing assertion to `toHaveBeenCalledWith('blob-new', 'mb-1', { $seen: true }, undefined, '2026-01-02T03:04:05Z')`. + +- [ ] **Step 2: Run to verify they fail** + +Run `npx vitest run src/api/__tests__/email.test.ts src/stores/__tests__/email-store.test.ts`. + +Expected: the new assertions FAIL. + +- [ ] **Step 3: Implement** + +Spread `...(receivedAt ? { receivedAt } : {})` after `keywords`, as in WEB 71a1fad. `crossAccountMove` passes `e.receivedAt` (the list row always has it, per `EMAIL_LIST_PROPERTIES`). + +- [ ] **Step 4: Run to verify they pass** + +Run the same command. Expected: PASS. + +- [ ] **Step 5: Commit** + +`fix: keep a moved message's original date in the other account` + +--- + +### Task 4: Keep an open draft on the account it was started in + +**Files:** +- Create: `src/lib/composer-account.ts` +- Modify: + - `src/screens/ComposeScreen.tsx`: + - the identity load at `:744-757`; + - the mailbox picks at `:441-451`; + - the `createDraft` call at `:1300`; + - the `destroyEmails` call at `:1406`; + - the uploads at `:798`, `:831` and `:1542`; + - the `sendEmail` call at `:2110`; + - the render-time reads at `:884-885` and `:925`. + - `src/api/blob.ts` (`uploadBytes` / `uploadBlob`), only if they lack an account parameter. Check `:67`. +- Test: `src/lib/__tests__/composer-account.test.ts` (new) + +**Interfaces:** +- Produces, in `src/lib/composer-account.ts`: + - `interface ComposerAccount { appAccountId: string; jmapAccountId: string }` + - `resolveComposerMailboxes(owner: ComposerAccount, activeAppAccountId: string | null, liveMailboxes: Mailbox[], snapshots: Record): Mailbox[] | null`. It returns the live mailboxes when `owner.appAccountId === activeAppAccountId`. Otherwise it returns the snapshot's mailboxes for `owner.appAccountId`, or `null` when there is none. + - Before writing this, read `src/stores/email-store.ts` for the actual name and key of the per-account snapshot map (`accountSnapshots`) and match it. + +- [ ] **Step 1: Write the failing tests** + +- `resolves to the active account's mailboxes when nothing switched` (Review Focus 3). +- `resolves to the owner's snapshot after a switch`. +- `returns null when the owner has no snapshot`. + +- [ ] **Step 2: Run to verify they fail** + +Run `npx vitest run src/lib/__tests__/composer-account.test.ts`. + +Expected: FAIL, because the module is missing. + +- [ ] **Step 3: Implement** + +- Port the helper. +- In `ComposeScreen`, capture the owner once on mount in a ref. Take `appAccountId` from `useAuthStore.getState().activeAccountId` and `jmapAccountId` from `jmapClient.accountId`. For a reopened draft, use the draft route param's `jmapAccountId` when present, as `seedOwnerAccountId` does. +- Pass `owner.jmapAccountId` explicitly to: + - `getIdentities` + - `createDraft` (4th argument) + - `destroyEmails` + - `uploadBytes` / `uploadBlob` + - `sendEmail` (`opts.accountId`) +- Take Sent and Drafts from `resolveComposerMailboxes(...)` instead of the live store. +- Replace the render-time `jmapClient.accountId` / `getActiveAccount()` reads with the owner. +- When `resolveComposerMailboxes` returns `null`, block send and autosave and show `Alert.alert(t('email_composer.account_switched_title', 'Account changed'), t('email_composer.account_switched_body', 'Switch back to the account this message was started in to send it.'))`. +- This port does not swap the JMAP client: these calls already accept an account override, and `jmapClient` serves every signed-in account's session. +- If, while implementing, a call turns out to work only for the active session (for example an upload URL tied to the active account), stop and report it rather than guessing. + +- [ ] **Step 4: Run to verify they pass** + +Run `npx vitest run src/lib src/api` and then `npm run typecheck`. + +Expected: PASS. + +- [ ] **Step 5: Commit** + +`fix: keep an open draft and its send on the account it was started in` + +Device check: with two accounts, start a reply in account A. Tap a notification for account B, return to the composer, and send. The mail must go from A, file into A's Sent, and leave nothing in B's Drafts. + +--- + +### Task 5: Reload filters, vacation and the security screen after an account switch + +**Files:** +- Modify: + - `src/stores/auth-store.ts:658-742` (`switchAccount`) + - `src/components/settings/FilterSettings.tsx:160-167` + - `src/components/settings/AccountSecuritySettings.tsx:844`, `:870-896` +- Test: `src/stores/__tests__/auth-store.test.ts` + +**Interfaces:** +- Consumes: `useFilterStore.getState().clearState()` (`filter-store.ts:247`) and `useVacationStore.getState().reset()` (`vacation-store.ts:113`). + +- [ ] **Step 1: Write the failing test** + +Add `switchAccount clears the filter and vacation stores`. Seed `useFilterStore` with a rule and `useVacationStore` with `isEnabled: true`, call `switchAccount` to another registered account, and assert that both are back at their initial state. Use the existing `auth-store.test.ts` mocks: see `describe('logout')` ~`:120` for how a switch is driven. + +- [ ] **Step 2: Run to verify it fails** + +Run `npx vitest run src/stores/__tests__/auth-store.test.ts`. + +Expected: FAIL. + +- [ ] **Step 3: Implement** + +- In `switchAccount`, call both resets next to the existing `useContactsStore.reset()` / `useCalendarStore.reset()` (`:675-676`). +- `FilterSettings`: add `useAuthStore((s) => s.activeAccountId)` to the `selectAccount` effect's dependencies, so a mounted screen refetches. +- `AccountSecuritySettings`: + - add the same value to the load effect's dependencies; + - reset the local state (`auth`, `displayName`, `loadError`, crypto) at the top of the effect; + - read `jmapClient.usesBearerAuth` inside the effect. + +- [ ] **Step 4: Run to verify it passes** + +Run the same command, then `npm run typecheck`. + +Expected: PASS. + +- [ ] **Step 5: Commit** + +`fix: reload filters, the auto-reply and account security after switching account` + +--- + +### Task 6: Recognize a daily series across a DST change + +**Files:** +- Modify: `src/lib/recurrence-expansion.ts`: + - `generateCandidatesForPeriod` `:383-410`; + - `matchesByX` `:538`, with its time checks at `:584-586`. +- Test: `src/lib/__tests__/recurrence-dst-gap.test.ts` (new) + +**Interfaces:** +- Produces: `matchesByX(date: Date, rule: RecurrenceRule, timeOf: Date = date): boolean`. Byhour/minute/second compare against `timeOf`. + +- [ ] **Step 1: Write the failing tests** + +Port WEB `lib/__tests__/recurrence-dst-gap.test.ts` unchanged. It includes the `daily(start, allDay, timeZone?)` helper and the save/restore of `process.env.TZ` in `beforeEach`/`afterEach`. The cases are: +- `keeps a 02:30 series going past the spring-forward night (Berlin)`: 31 results, the last starting `2026-03-31T02:30`. +- `keeps an all-day series going after a midnight gap (Santiago)`: 7 results, all at `00:00`. + +Add `a daily series in UTC is unchanged`, with `TZ='UTC'`, 10 days, and every start at the same `HH:mm` (Review Focus 4). + +- [ ] **Step 2: Run to verify they fail** + +Run `npx vitest run src/lib/__tests__/recurrence-dst-gap.test.ts`. + +Expected: Berlin and Santiago FAIL; UTC passes. + +- [ ] **Step 3: Implement** + +Port WEB `lib/recurrence-expansion.ts:419-428` and `:444`: +- Split `'daily'` out of the shared fallthrough. It builds `new Date(periodStart.getFullYear(), periodStart.getMonth(), periodStart.getDate(), eventStart.getHours(), eventStart.getMinutes(), eventStart.getSeconds(), eventStart.getMilliseconds())`. +- Filter with `matchesByX(d, rule, freq === 'daily' ? eventStart : undefined)`. +- Hourly, minutely and secondly keep `new Date(periodStart)`. + +- [ ] **Step 4: Run to verify they pass** + +Run `npx vitest run src/lib/__tests__/recurrence-dst-gap.test.ts src/lib/__tests__/recurrence-expansion.test.ts src/lib/__tests__/recurrence-instances.test.ts`. + +Expected: PASS, with the existing files unchanged. + +- [ ] **Step 5: Commit** + +`fix: keep a daily series going past a daylight-saving change` + +--- + +### Task 7: Tie iCal subscriptions to the login that created them + +**Files:** +- Modify: `src/stores/calendar-subscriptions-store.ts`: + - `CalendarSubscription` `:20-33`; + - `selectAccountSubscriptions` `:62-67`; + - `addSubscription` ~`:163`; + - `syncAll` / `syncDue` ~`:253-262`; + - persist config ~`:271-276`. +- Test: `src/stores/__tests__/calendar-subscriptions-store.test.ts` + +**Interfaces:** +- Produces: + - `subscriptionOwner(serverUrl: string, username: string): string`, which returns `` `${serverUrl.replace(/\/+$/, '').toLowerCase()}|${username.toLowerCase()}` `` (WEB `stores/calendar-store.ts:494-497`). + - `CalendarSubscription.owner?: string`. + - Store action `forgetSubscriptions(owner: string): void`. + - `selectAccountSubscriptions(subscriptions, owner: string | null, accountId: string | null, calendars: { id: string; originalId?: string; name: string }[])`. It implements WEB `claimSubscription` (`:499-521`): + - an owned sub matches only its own owner; + - an unowned sub whose `accountId` differs is excluded; + - an unowned sub is adopted, with `owner` set on write, only when `calendars` contains one where `(originalId ?? id) === sub.calendarId && name === sub.name`. + - Update every caller of the old two-argument selector. The callers are `ICalSubscriptionSheet.tsx`, `CalendarInvitationBanner.tsx`, `device-sync/use-sync-collections.ts` and `CalendarScreen.tsx`; check each one. + +- [ ] **Step 1: Write the failing tests** + +- `stamps the owner on a new subscription` (from `jmapClient.serverUrl` / `username`; extend the mock). +- `shows a subscription only to its owner`. +- `adopts a legacy subscription only for the login whose calendars contain it` (Review Focus 2). +- `does not refresh another login's subscription`. +- `forgetSubscriptions removes only that owner's subscriptions`. +- `migrates version 0 state without dropping subscriptions`. + +- [ ] **Step 2: Run to verify they fail** + +Run `npx vitest run src/stores/__tests__/calendar-subscriptions-store.test.ts`. + +Expected: FAIL. + +- [ ] **Step 3: Implement** + +- Add `version: 1` and a `migrate` that keeps every subscription as it is. Unowned entries are claimed lazily by the selector, as in WEB. +- Persist adopted owners through the store's normal `set`. + +- [ ] **Step 4: Run to verify they pass** + +Run `npx vitest run src/stores src/lib` and then `npm run typecheck`. + +Expected: PASS. + +- [ ] **Step 5: Commit** + +`fix: keep each login's calendar subscriptions to itself` + +--- + +### Task 8: Forget a signed-out account's data on the device + +**Files:** +- Create: `src/stores/account-data-cleanup.ts` +- Modify: + - `src/stores/offline-cache-store.ts`: add a per-account clear next to `clearAll` `:292-303`. + - `src/stores/outbox-store.ts`: add a per-account clear next to `clear` `:384-390`. + - `src/stores/auth-store.ts`: `logout` `:575-621`, `logoutAll` `:623-654`, `removeAccount` `:744-760`. +- Test: `src/stores/__tests__/account-data-cleanup.test.ts` (new) and `src/stores/__tests__/auth-store.test.ts` + +**Interfaces:** +- Consumes: `forgetSubscriptions` and `subscriptionOwner` (Task 7). +- Produces: + - `forgetAccountData(account: { appAccountId: string; jmapAccountId?: string; serverUrl?: string | null; username?: string | null }): Promise`. It clears: + - that account's offline-cache index and entries (`webmail:offline-cache:index:v2:` and `…entry:v2::*`; first check which id `` is in `offline-cache-store.ts:17-18`); + - that account's outbox keys (`webmail:outbox:v1:` + `storageKey(acct)` and the failed suffix); + - its calendar subscriptions (`forgetSubscriptions(subscriptionOwner(serverUrl, username))` when both are known); + - the global search history (`useSearchHistoryStore.getState().clearRecentSearches()`), on every sign-out, because its entries are not per account. + - `useOfflineCacheStore.getState().clearAccount(accountId: string): Promise` + - `useOutboxStore.getState().clearAccount(accountId: string): Promise` + +- [ ] **Step 1: Write the failing tests** + +- `forgets only the signed-out account's data` (Review Focus 5). Seed two accounts' offline-cache keys, outbox keys and subscriptions in the in-memory AsyncStorage mock, call `forgetAccountData` for one, and assert the other's data is untouched. +- `clears the search history`. +- In `auth-store.test.ts`: + - `logout forgets the account's data`; + - `removeAccount forgets a non-active account's data, using its registry serverUrl and username`; + - `logoutAll forgets every account's data`. + - Mock `account-data-cleanup` and assert the arguments. + +- [ ] **Step 2: Run to verify they fail** + +Run `npx vitest run src/stores/__tests__/account-data-cleanup.test.ts src/stores/__tests__/auth-store.test.ts`. + +Expected: FAIL. + +- [ ] **Step 3: Implement** + +Capture the account's `serverUrl` and `username` before its credentials are cleared: +- in `logout`, from `jmapClient`; +- in `removeAccount`, from `useAccountStore.getState().getAccountById(id)`. + +Call `forgetAccountData` from all three paths. Keep settings, locale, templates, keywords and the account-independent stores, as WEB `lib/sign-out-cleanup.ts` does. + +- [ ] **Step 4: Run to verify they pass** + +Run `npx vitest run src/stores` and then `npm run typecheck`. + +Expected: PASS. + +- [ ] **Step 5: Commit** + +`fix: forget an account's offline mail, outbox, subscriptions and search history when it signs out` + +--- + +### Task 9: Save an event without invitations when the server refuses to send them + +**Files:** +- Modify: + - `src/api/calendar.ts`: `createEvent` `:606-633`, `updateEvent` ~`:680-697`. + - `src/screens/CalendarScreen.tsx`: `handleSave` `:734-759`, `handleScopeSelect` `:641-731`. + - `src/components/calendar/EventModal.tsx`: `handleSave` `:299-366`. +- Create: `src/lib/scheduling-denied.ts` +- Modify: `src/api/jmap-result.ts` (add `SchedulingDeniedError` next to the Phase 1 send errors) +- Test: `src/api/__tests__/calendar.test.ts` and `src/lib/__tests__/scheduling-denied.test.ts` (new) + +**Interfaces:** +- Produces: + - `class SchedulingDeniedError extends Error { readonly reason: string }` in `src/api/jmap-result.ts`, with `name = 'SchedulingDeniedError'` (WEB `lib/jmap/scheduling-error.ts`). + - In `src/lib/scheduling-denied.ts`: `saveWithSchedulingFallback(save: (send: boolean | undefined) => Promise, send: boolean | undefined, confirm: (reason: string) => Promise): Promise<'saved' | 'saved_without_invitations' | 'cancelled'>`. It calls `save(send)`. On a `SchedulingDeniedError` when `send` is true, it asks `confirm(reason)`, and on yes calls `save(false)`. Any other error is rethrown. + +- [ ] **Step 1: Write the failing tests** + +In `calendar.test.ts`: +- `createEvent throws SchedulingDeniedError when scheduling is refused`: `notCreated['new-event'] = { type: 'forbidden', description: 'Not allowed to schedule' }` with `sendSchedulingMessages: true`. +- The same case for `updateEvent`. +- `a forbidden error without scheduling stays a plain error`. + +In `scheduling-denied.test.ts`: +- `retries without invitations when the user agrees` +- `stops when the user declines` +- `rethrows other errors` +- `does not ask when invitations were not being sent` + +- [ ] **Step 2: Run to verify they fail** + +Run `npx vitest run src/api/__tests__/calendar.test.ts src/lib/__tests__/scheduling-denied.test.ts`. + +Expected: FAIL. + +- [ ] **Step 3: Implement** + +In the API, before the generic error, check `if (sendSchedulingMessages && err?.type === 'forbidden') throw new SchedulingDeniedError(err.description || 'forbidden')` (WEB `client.ts:7185,7380`). + +Wrap the three save paths in `saveWithSchedulingFallback`. The `confirm` callback is an `Alert.alert` promise: +- Title: `calendar.notifications.invitations_denied_title`. +- Message: `calendar.notifications.invitations_denied` with `{ reason }`. +- Buttons: cancel, and `calendar.notifications.save_without_invitations`. +- Reuse the webmail keys and texts (look them up in WEB `locales/en/common.json`). + +Also give `EventModal.handleSave` a `catch` that shows the existing `calendar.notifications.event_error` alert. Today a failed save is an unhandled rejection. + +- [ ] **Step 4: Run to verify they pass** + +Run `npx vitest run src/api src/lib` and then `npm run typecheck && npm run i18n:check`. + +Expected: PASS. + +- [ ] **Step 5: Commit** + +`fix: offer to save an event without invitations when the server refuses to send them` + +--- + +### Task 10: Leave a participant's name out when it is blank + +**Files:** +- Modify: `src/lib/calendar-participants.ts:271-310` (`buildParticipantMap`) +- Test: `src/lib/__tests__/calendar-participants.test.ts` + +**Interfaces:** +- Produces: participants in the returned map have `name` only when the trimmed name is non-empty. If the native `Participant` type requires `name`, make it optional. + +- [ ] **Step 1: Write the failing tests** + +- `omits a blank organizer name`: `buildParticipantMap({ name: '', email: 'me@x.example' }, [])` gives an organizer entry with no `name` key. +- `omits a whitespace-only attendee name and trims a real one`: `' Ann '` becomes `'Ann'`. + +- [ ] **Step 2: Run to verify they fail** + +Run `npx vitest run src/lib/__tests__/calendar-participants.test.ts`. + +Expected: FAIL. + +- [ ] **Step 3: Implement** + +Use WEB's helper `const named = (name: string) => (name.trim() ? { name: name.trim() } : {})`, and spread it in place of `name:` for the organizer and for each attendee (WEB `lib/calendar-participants.ts:252-290`). Run `npm run typecheck` and fix any reader that assumed `name` is always set. + +- [ ] **Step 4: Run to verify they pass** + +Run the same command and then `npm run typecheck`. + +Expected: PASS. + +- [ ] **Step 5: Commit** + +`fix: leave a participant's name out of an invitation when it is blank` + +--- + +### Task 11: Let the server rename a new folder or file whose name is taken + +**Files:** +- Modify: + - `src/api/files.ts`: `createFolder` `:248-267`, `createFileNodeFromBlob` `:333-352`. + - `src/lib/filenode-name.ts` +- Test: `src/api/__tests__/files.test.ts` (the `createFolder` tests at `:84-105`) and `src/lib/__tests__/filenode-name.test.ts` + +**Interfaces:** +- Produces: + - `numberedFileName(name: string, n: number): string`, exported from `src/lib/filenode-name.ts`. It gives `"report.pdf", 2 → "report (2).pdf"` and `"notes", 3 → "notes (3)"`, matching WEB `client.ts:799-802` (the `dot > 0` rule). + - A private helper in `files.ts`, `createFileNode(accountId: string, props: Record): Promise`, port of WEB `createFileNodeIn` (`:8072-8105`). It: + - sends `onExists: 'rename'`; + - on a `notCreated` whose description matches `/already exists/i` (older servers ignore `onExists`), retries with `numberedFileName(base, attempt)` for attempts 2–20; + - returns `{ ...props, name, ...created }`, so a server-side rename wins. + - `createFolder` and `createFileNodeFromBlob` both use it. + +- [ ] **Step 1: Write the failing tests** + +- `numberedFileName` cases, including a dotfile `.env` → `.env (2)`. +- `createFolder sends onExists rename`. +- `createFolder takes the server's renamed name`. +- `createFolder retries with a numbered name on a server that ignores onExists`. +- `gives up after 20 attempts`. + +- [ ] **Step 2: Run to verify they fail** + +Run `npx vitest run src/api/__tests__/files.test.ts src/lib/__tests__/filenode-name.test.ts`. + +Expected: FAIL. Also update the existing `createFolder` assertions for the new `onExists` arg. + +- [ ] **Step 3: Implement** + +Do what the interfaces above specify. + +- [ ] **Step 4: Run to verify they pass** + +Run the same command. Expected: PASS. + +- [ ] **Step 5: Commit** + +`fix: give a new folder or file a numbered name when its name is taken` + +--- + +### Task 12: Speak the older FileNode rights and type limits to servers before Stalwart 0.16.6 + +**Files:** +- Modify: `src/api/files.ts`: + - `FILE_NODE_PROPERTIES` reads `:18-25`; + - `setFileNodeShare` `:400-420`; + - `safeMimeType` `:327-331` and its callers `:366`, `:386`; + - the node read path that maps `myRights` / `shareWith`. +- Test: `src/api/__tests__/files.test.ts`. Add `getAccountCapability: vi.fn()` to the `jmapClient` mock; it is missing today. + +**Interfaces:** +- Produces, all in `files.ts`: + - `isLegacyFileNodeServer(accountId: string): boolean`. It is `!!cap && !('forbiddenNameChars' in cap)`, where `cap = jmapClient.getAccountCapability(CAPABILITIES.FILES, accountId)` (WEB `:8039-8042`). + - `toLegacyRights(r: FileNodeRights)`, which returns `{ mayRead, mayWrite: mayAddChildren || mayRename || mayDelete || mayModifyContent, mayShare }`. + - `fromLegacyRights(r)`. When `'mayWrite' in r`, it spreads `mayWrite` over the four finer rights (WEB `:43-66`). Apply it to `myRights` and every `shareWith` entry on read. + - `safeMimeType(type: string | undefined, fallback: string, accountId: string): string`, with a limit of 30 on a legacy server and 255 otherwise (WEB `:8122-8131`). + +- [ ] **Step 1: Write the failing tests** + +Port the cases: +- WEB `lib/__tests__/jmap-filenode-writes.test.ts:92`, `shares with the mayWrite rights of servers before 0.16.6`; +- WEB `:103`, `reads old mayWrite rights as the finer rights`. + +Add: +- `keeps a 40-character office MIME type on a current server`; +- `falls back to octet-stream for a long type on a legacy server`; +- `shares with the finer rights on a current server`. + +- [ ] **Step 2: Run to verify they fail** + +Run `npx vitest run src/api/__tests__/files.test.ts`. + +Expected: FAIL. + +- [ ] **Step 3: Implement** + +Do what the interfaces above specify. + +- [ ] **Step 4: Run to verify they pass** + +Run the same command. Expected: PASS. + +- [ ] **Step 5: Commit** + +`fix: share files and keep office file types on every Stalwart version` + +--- + +### Task 13: Check file and folder names against the server's rules before sending them + +**Files:** +- Create: `src/lib/file-name-rules.ts`, a port of WEB `lib/file-name-rules.ts` +- Modify: + - `src/api/files.ts`: add `getFileNameRules(accountId?: string)`; + - `src/screens/FilesScreen.tsx`: the new-folder check `:477-496`, rename ~`:505`, upload ~`:611`. +- Test: `src/lib/__tests__/file-name-rules.test.ts` (new) + +**Interfaces:** +- Produces (WEB signatures): + - `FileNameRules { forbiddenChars: string; forbiddenNames: string[] }` + - `FileNameProblem = { kind: 'chars'; chars: string } | { kind: 'reserved' }` + - `fileNameRulesFrom(capability): FileNameRules | null` + - `fileNameProblem(name, rules): FileNameProblem | null` + - `acceptedFileName(name, rules): string` +- Consumes: the `getAccountCapability` mock from Task 12. + +- [ ] **Step 1: Write the failing tests** + +Port WEB `lib/__tests__/file-name-rules.test.ts`, all three cases: +- `has no rules for servers that publish none (Stalwart before 0.16.6)` +- `reports forbidden characters and reserved names` +- `turns an uploaded name into one the server accepts` + +Add `matches a reserved name only as the whole name`: `CON` is reserved, `CON.txt` is not. + +- [ ] **Step 2: Run to verify they fail** + +Run `npx vitest run src/lib/__tests__/file-name-rules.test.ts`. + +Expected: FAIL. + +- [ ] **Step 3: Implement** + +- New folder and rename: keep the existing `/` check. Add `fileNameProblem` and show an alert that names the forbidden characters, or says the name is reserved. Use WEB's keys `files.name_forbidden_chars` (with `{chars}`) and `files.name_reserved`. +- Upload: pass the name through `acceptedFileName` before `getUniqueName`. + +- [ ] **Step 4: Run to verify they pass** + +Run `npx vitest run src/lib src/api` and then `npm run typecheck && npm run i18n:check`. + +Expected: PASS. + +- [ ] **Step 5: Commit** + +`fix: check file and folder names against the server's rules before saving them` + +--- + +### Task 14: Copy a folder with everything in it + +**Files:** +- Modify: + - `src/api/files.ts`: `copyFileNode` `:374-390`, including its comment; + - `src/screens/FilesScreen.tsx`: `duplicateFile` `:521-527`. +- Test: `src/api/__tests__/files.test.ts` + +**Interfaces:** +- Consumes: the `createFileNode` private helper (Task 11), so name clashes rename. +- Produces: `copyFileNode(node: FileNode, parentId: string | null, newName?: string, tree?: FileNode[]): Promise`. + - A file copies as today. + - A folder creates itself, then recursively copies each child where `n.parentId === node.id`, into the new folder's id. The children come from `tree`, or from the existing all-nodes fetch (`files.ts:106-140`) when no tree is given (WEB `:8273-8316`). + - Cross-account copy stays out of scope: shared rows remain blocked in `duplicateFile`. + +- [ ] **Step 1: Write the failing tests** + +- `copies a folder with its whole subtree`, after WEB `jmap-filenode-writes.test.ts:71`. Use a folder containing a file and a subfolder that contains a file, and assert: + - four creates; + - the right parent ids; + - the files reuse `blobId`. +- `copies an empty folder`. + +- [ ] **Step 2: Run to verify they fail** + +Run `npx vitest run src/api/__tests__/files.test.ts`. + +Expected: FAIL. Today the code throws "Folders cannot be duplicated". + +- [ ] **Step 3: Implement** + +Do the recursion as specified. In `duplicateFile`, drop the `isFolder(row)` early return and pass the screen's loaded node list as `tree`. + +- [ ] **Step 4: Run to verify they pass** + +Run the same command, then `npm run typecheck`. + +Expected: PASS. + +- [ ] **Step 5: Commit, then close the phase** + +`feat: copy a folder with everything in it` + +After the final whole-branch review, tick the 12 roadmap rows' parity items with their commit hashes, and update the counts table in `PARITY_CHECKLIST.md` in one `docs:` commit. Then run: + +```bash +npm run typecheck && npm test && npm run i18n:check +``` + +Expected: all pass.