Mail HTML: sanitized messages lose the sender's body class and attributes #1533

Closed
opened 2026-10-10 10:29:24 +00:00 by kayg · 5 comments
Owner

apps/web/e2e/mail-html-726.mjs fails at line 124 on origin/dev (production build, shared prebuilt server): the rendered HTML message body has body class = null instead of the sender's newsletter726. The sanitized HTML loses the sender's attributes, so newsletters whose CSS targets their body class render unstyled. Expected: the sanitizer keeps safe body attributes (class, style subject to the same CSS sanitizing, bgcolor) by moving them onto the sandboxed wrapper, per DESIGN §45 HTML rendering. Found by the Mail UI pass (job/ui-mail, 2026-10-10).

apps/web/e2e/mail-html-726.mjs fails at line 124 on origin/dev (production build, shared prebuilt server): the rendered HTML message body has `body class` = null instead of the sender's `newsletter726`. The sanitized HTML loses the sender's <body> attributes, so newsletters whose CSS targets their body class render unstyled. Expected: the sanitizer keeps safe body attributes (class, style subject to the same CSS sanitizing, bgcolor) by moving them onto the sandboxed wrapper, per DESIGN §45 HTML rendering. Found by the Mail UI pass (job/ui-mail, 2026-10-10).
Author
Owner

Starting #1533 on branch job/mailbody-1533, based on dbd621434d (origin/dev at job start). I am tracing the mail HTML sanitizer, its existing tests, the requested e2e fixture, and the adversarial probes before implementing body attribute handling.

Starting #1533 on branch job/mailbody-1533, based on dbd621434d0a1133f16230cca5c6a83af0d8aed3 (origin/dev at job start). I am tracing the mail HTML sanitizer, its existing tests, the requested e2e fixture, and the adversarial probes before implementing body attribute handling.
Author
Owner

Baseline finding: using the shared release server at revision dbd621434 without CALTERNAL_E2E_ASSET_OVERRIDE=1 reproduces #1533 at apps/web/e2e/mail-html-726.mjs:124 (null !== newsletter726). Routing the worktree's production build over that same server makes the class/ID/direction/language assertions pass, so the server's embedded web bundle is stale for this check. I will make this e2e always route the current production build. The sanitizer already keeps class/ID/direction/language, but its frame handoff leaves the sanitized body style on the inner div, and it does not map text or link colors to CSS. I am extending that bounded handoff and adding the hostile-attribute cases.

Baseline finding: using the shared release server at revision dbd621434 without `CALTERNAL_E2E_ASSET_OVERRIDE=1` reproduces #1533 at `apps/web/e2e/mail-html-726.mjs:124` (`null !== newsletter726`). Routing the worktree's production build over that same server makes the class/ID/direction/language assertions pass, so the server's embedded web bundle is stale for this check. I will make this e2e always route the current production build. The sanitizer already keeps class/ID/direction/language, but its frame handoff leaves the sanitized body style on the inner div, and it does not map `text` or link colors to CSS. I am extending that bounded handoff and adding the hostile-attribute cases.
Author
Owner

Verification findings and fixes:

  • mail-html-726.mjs used an async predicate with Playwright waitForFunction, which treats the returned Promise as truthy. I switched it to the existing awaited poll helper so it waits for actual prepared HTML.
  • The reload assertion ran after Escape. Escape closes the reader and removes message-only query state, so I moved it after the reload check without changing the assertion.
  • The large fixture had no local sanitized body. A permission refresh temporarily clears the remote body; the reader then requested a missing provider MIME body and got HTTP 500. The reader now waits for its detail refresh before starting a full-body read, and the fixture holds its local data:, version. The e2e asserts no full-body fallback during the revocation/Undo path.

The completed focused e2e run passed all 42 macOS-emulated screenshots, containment checks, and two-tab revocation/Undo. tests/adversarial/mail_api.mjs passed its API checks up to the attachment UI step, then timed out because the selected row intercepts pointer events on Save to Files. I filed that separate finding as #1535. The Mail network-policy probe passed.

Verification findings and fixes: - `mail-html-726.mjs` used an async predicate with Playwright `waitForFunction`, which treats the returned Promise as truthy. I switched it to the existing awaited `poll` helper so it waits for actual prepared HTML. - The reload assertion ran after Escape. Escape closes the reader and removes message-only query state, so I moved it after the reload check without changing the assertion. - The large fixture had no local sanitized body. A permission refresh temporarily clears the remote body; the reader then requested a missing provider MIME body and got HTTP 500. The reader now waits for its detail refresh before starting a full-body read, and the fixture holds its local `data:,` version. The e2e asserts no full-body fallback during the revocation/Undo path. The completed focused e2e run passed all 42 macOS-emulated screenshots, containment checks, and two-tab revocation/Undo. `tests/adversarial/mail_api.mjs` passed its API checks up to the attachment UI step, then timed out because the selected row intercepts pointer events on Save to Files. I filed that separate finding as #1535. The Mail network-policy probe passed.
Author
Owner

Completed #1533 on job/mailbody-1533.

Commits: 794a365bf189aa8e23ad2def4333b1d672451eae and 92ba13fc3ad47930ebc22e19fda49b9793000e84 (head; base dbd621434d0a1133f16230cca5c6a83af0d8aed3). origin/dev was merged once before final checks and was already up to date.

Built: the Mail sanitizer retains bounded class, id, dir, lang and sanitized inline style on its body marker. Legacy body color attributes become parsed CSS before sender styles, so sender selectors and dark-mode rules keep working. The frame transfers safe attributes onto its real sandboxed body. Script/event attributes, executable CSS and blocked external URLs remain removed. Body refresh now waits for current detail before requesting a full provider body.

Files: crates/plugins/mail/src/html.rs; apps/web/src/lib/mail/frame.ts, frame.test.ts, MailView.svelte, MailReaderContent.svelte; apps/web/e2e/mail-html-726.mjs; tests/adversarial/mail_html_fixture.py; contracts/perf/exceptions.json, adoption-1058.json.

Decisions: Keep the sender's class and ID unchanged inside the isolated frame. DESIGN §45 does not require rewriting them, and rewriting would break sender selectors. Map legacy colors into sanitized CSS before sender styles to preserve the existing cascade. Expected cost is one bounded body-attribute/style sanitization during message preparation (style capped at 64 KiB); no added IO or per-render work. The existing bench/mail-html-726.mjs covers these cached message reads, but the profile was not measured: local 5-minute load was 7.12 and the perf VM was held by another job (tabcomp-1233-composer-fixed).

Gate output:

  • cargo fmt --check: no output; exit 0.
  • cargo clippy -p calternal-plugin-mail --all-targets -- -D warnings:
    Finished dev profile [unoptimized + debuginfo] target(s) in 37.12s
  • cargo test -p calternal-plugin-mail:
    test result: ok. 172 passed; 0 failed; 10 ignored; 0 measured; 0 filtered out; finished in 25.78s
    test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
  • cd apps/web && bun run check:
    perf-lint: PASS; 0 violations; 22247 scoped exceptions
    Focus policy passes: 50 focus ring rules, 133 DOM focus calls and 2 data-input writers audited.
    UI transitions and animation options use shared motion tokens or documented exceptions.
    svelte-check found 0 errors and 3 warnings in 1 file
    exit_code=0
  • bunx vitest run src/lib/mail/frame.test.ts src/lib/mail/remoteContent.test.ts --maxWorkers=2:
    Test Files 2 passed (2); Tests 19 passed (19); duration 1.55s; exit 0.
  • CALTERNAL_SERVER_BIN=/home/kayg/build/targets/mailbody-1533/debug/calternal-server node apps/web/e2e/mail-html-726.mjs:
    Mail HTML #726: 42 macOS screenshots, cached-image ownership/anonymous/off-state checks, two-tab revocation, body dark selectors, sender-font decode, HTML containment, Undo and zero external resource requests passed.
  • tests/adversarial/mail_network_policy.mjs:
    Mail network-policy probe: IMAP and SMTP reject unlisted private, loopback, metadata, link-local, multicast, unspecified, broadcast, mapped/compatible IPv6, NAT64 and 6to4 targets even when those ranges are listed; malformed Mail and Calendar CIDRs fail startup; listed private access keeps TLS verification enabled

The Mail API adversarial probe reached its attachment UI step and timed out because the selected attachment row intercepted the Save to Files click (tests/adversarial/mail_api.mjs:414). Filed separately as #1535. The earlier API/security assertions in that probe passed. The focused frame/remote-content Vitest files passed. Full bun run test --maxWorkers=2 and release-server e2e were deferred under the merge-round verification policy. For the merge round: build calternal-server in release mode and run CALTERNAL_SERVER_BIN=<release-server> node apps/web/e2e/mail-html-726.mjs to prove the Rust change against release; run cd apps/web && bun run test --maxWorkers=2 for the full web suite. Run the existing Mail HTML profile through perf-run when the perf VM is free.

UX gaps closed: sender body selectors, inline styles and legacy colors now reach the real frame body; access refresh no longer triggers a redundant provider body read; e2e verifies dark selectors, safe styles, no external requests and no fallback read. UX gaps left: unrelated Mail attachment hit target, tracked in #1535.

Screenshots attached: before 1440 light, before 1440 dark, after 390 light, after 390 dark, after 820 light, after 820 dark, after 1440 light, after 1440 dark.

No macOS VM used. cargo clean removed 13,404 files (9.2 GiB); web build output was removed.

Completed #1533 on `job/mailbody-1533`. **Commits:** `794a365bf189aa8e23ad2def4333b1d672451eae` and `92ba13fc3ad47930ebc22e19fda49b9793000e84` (head; base `dbd621434d0a1133f16230cca5c6a83af0d8aed3`). `origin/dev` was merged once before final checks and was already up to date. **Built:** the Mail sanitizer retains bounded `class`, `id`, `dir`, `lang` and sanitized inline `style` on its body marker. Legacy body color attributes become parsed CSS before sender styles, so sender selectors and dark-mode rules keep working. The frame transfers safe attributes onto its real sandboxed body. Script/event attributes, executable CSS and blocked external URLs remain removed. Body refresh now waits for current detail before requesting a full provider body. **Files:** `crates/plugins/mail/src/html.rs`; `apps/web/src/lib/mail/frame.ts`, `frame.test.ts`, `MailView.svelte`, `MailReaderContent.svelte`; `apps/web/e2e/mail-html-726.mjs`; `tests/adversarial/mail_html_fixture.py`; `contracts/perf/exceptions.json`, `adoption-1058.json`. **Decisions:** Keep the sender's class and ID unchanged inside the isolated frame. DESIGN §45 does not require rewriting them, and rewriting would break sender selectors. Map legacy colors into sanitized CSS before sender styles to preserve the existing cascade. Expected cost is one bounded body-attribute/style sanitization during message preparation (style capped at 64 KiB); no added IO or per-render work. The existing `bench/mail-html-726.mjs` covers these cached message reads, but the profile was not measured: local 5-minute load was 7.12 and the perf VM was held by another job (`tabcomp-1233-composer-fixed`). **Gate output:** - `cargo fmt --check`: no output; exit 0. - `cargo clippy -p calternal-plugin-mail --all-targets -- -D warnings`: `Finished `dev` profile [unoptimized + debuginfo] target(s) in 37.12s` - `cargo test -p calternal-plugin-mail`: `test result: ok. 172 passed; 0 failed; 10 ignored; 0 measured; 0 filtered out; finished in 25.78s` `test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s` - `cd apps/web && bun run check`: `perf-lint: PASS; 0 violations; 22247 scoped exceptions` `Focus policy passes: 50 focus ring rules, 133 DOM focus calls and 2 data-input writers audited.` `UI transitions and animation options use shared motion tokens or documented exceptions.` `svelte-check found 0 errors and 3 warnings in 1 file` `exit_code=0` - `bunx vitest run src/lib/mail/frame.test.ts src/lib/mail/remoteContent.test.ts --maxWorkers=2`: `Test Files 2 passed (2)`; `Tests 19 passed (19)`; duration `1.55s`; exit 0. - `CALTERNAL_SERVER_BIN=/home/kayg/build/targets/mailbody-1533/debug/calternal-server node apps/web/e2e/mail-html-726.mjs`: `Mail HTML #726: 42 macOS screenshots, cached-image ownership/anonymous/off-state checks, two-tab revocation, body dark selectors, sender-font decode, HTML containment, Undo and zero external resource requests passed.` - `tests/adversarial/mail_network_policy.mjs`: `Mail network-policy probe: IMAP and SMTP reject unlisted private, loopback, metadata, link-local, multicast, unspecified, broadcast, mapped/compatible IPv6, NAT64 and 6to4 targets even when those ranges are listed; malformed Mail and Calendar CIDRs fail startup; listed private access keeps TLS verification enabled` The Mail API adversarial probe reached its attachment UI step and timed out because the selected attachment row intercepted the `Save to Files` click (`tests/adversarial/mail_api.mjs:414`). Filed separately as [#1535](https://git.kayg.org/kayg/calternal/issues/1535). The earlier API/security assertions in that probe passed. The focused frame/remote-content Vitest files passed. Full `bun run test --maxWorkers=2` and release-server e2e were deferred under the merge-round verification policy. **For the merge round:** build `calternal-server` in release mode and run `CALTERNAL_SERVER_BIN=<release-server> node apps/web/e2e/mail-html-726.mjs` to prove the Rust change against release; run `cd apps/web && bun run test --maxWorkers=2` for the full web suite. Run the existing Mail HTML profile through `perf-run` when the perf VM is free. **UX gaps closed:** sender body selectors, inline styles and legacy colors now reach the real frame body; access refresh no longer triggers a redundant provider body read; e2e verifies dark selectors, safe styles, no external requests and no fallback read. **UX gaps left:** unrelated Mail attachment hit target, tracked in #1535. **Screenshots attached:** [before 1440 light](https://git.kayg.org/attachments/dfd03b84-d7b5-434d-9f13-ee3058b51366), [before 1440 dark](https://git.kayg.org/attachments/6e1533d0-a112-4e2d-8205-0b6ef9056327), [after 390 light](https://git.kayg.org/attachments/662669e8-837e-4a6f-9750-be61604f537d), [after 390 dark](https://git.kayg.org/attachments/7cf2e2f9-719f-4e75-8712-e4fbba7e72c2), [after 820 light](https://git.kayg.org/attachments/4298fe71-341e-40cd-bbd1-a5c0b4be4721), [after 820 dark](https://git.kayg.org/attachments/a15b16d8-5c37-4f66-b727-c4eb2ed6cd8f), [after 1440 light](https://git.kayg.org/attachments/c44a2a54-8db6-4a2a-8db2-21bc69ae7e48), [after 1440 dark](https://git.kayg.org/attachments/e76dcf60-4966-48a1-83bb-e29583221c6b). No macOS VM used. `cargo clean` removed 13,404 files (9.2 GiB); web build output was removed.
Author
Owner

Deployed to production in merge round 37 (ac7fda1a8, 2026-10-10 21:37 IST).

Deployed to production in merge round 37 (ac7fda1a8, 2026-10-10 21:37 IST).
kayg closed this issue 2026-10-10 16:07:34 +00:00
Commenting is not possible because the repository is archived.
No labels
No milestone
No project
No assignees
1 participant
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
kayg/calternal-v0#1533
No description provided.