M6: Sanitize inbound HTML email bodies before they leave the engine #204

New Issue

2026-07-28T14:04:52Z

KofTwentyTwo commented

2026-07-28 14:04:52 +00:00

(Migrated from github.com)

Context

Story for milestone M6. Read docs/STORY-WORKFLOW.md first — it defines the process, gates, and patterns; this issue defines only the specific work. crates/nuncio-mail/src/parser.rs::MimeParserAdapter::parse_mime extracts the HTML body of a fetched message straight from mail-parser's output with zero sanitization: let body_html = msg.body_html(0).map(|b| b.to_string()); (parser.rs:91), stored directly into nuncio_core::model::Email::body_html (parser.rs:130, field defined at crates/nuncio-core/src/model.rs:85). Verified in the tree: this raw, untrusted HTML flows unmodified through DatabaseEngine::save_email/get_message/list_messages (encrypted at rest and decrypted back byte-for-byte, crates/nuncio-store/src/db.rs:734-737,839,891 etc.), is placed directly into the gRPC Message.body_html field (crates/nuncio-proto/proto/nuncio/v1/nuncio.proto:298, wired at crates/nunciod/src/grpc.rs:399: body_html: email.body_html,), and is emitted as-is by the CLI's JSON output (crates/nuncio-cli/src/runner.rs:56: "body_html": message.body_html,). There is no ammonia (or any HTML-sanitizing) dependency anywhere in the active workspace today (confirmed via a full-tree search — the only sanitization code that ever existed lived in _reference/nuncio-gui/src/sandbox.rs, which is out-of-tree per this repo's current architecture: presentation shells are being rebuilt as separate client repos, and CLAUDE.md requires the engine — not each client — to be the thing that guarantees this). Any script tag, inline event handler, javascript: link, or CSS/<img>-based tracking pixel present in a received email's HTML part is preserved verbatim all the way out to every current and future client.

Goal

Add a real HTML sanitizer to the engine (using ammonia) and wire it into the one place untrusted HTML enters the system — MimeParserAdapter::parse_mime — so Email.body_html is always safe-to-render HTML by the time it reaches storage, the gRPC API, or the CLI. This must not merely strip everything to plain text (that would silently degrade every HTML email); it must produce sanitized HTML: no <script>, no inline event-handler attributes (onerror, onclick, etc.), no javascript:/data:text/html URLs, and no <style>/CSS-based exfiltration vectors, while preserving ordinary formatting markup (bold, links, lists, tables, etc.). Outbound compose/send paths (crates/nuncio-mail/src/smtp.rs) are user-authored content, not untrusted input, and are explicitly OUT of scope — do not sanitize outgoing mail.

Files to touch / patterns to mirror

Cargo.toml (workspace root, [workspace.dependencies], Cargo.toml:36-86) — add ammonia = "4.1" (verified current on docs.rs as of this writing; confirm the latest 4.x patch when implementing).
crates/nuncio-mail/Cargo.toml:12-27 ([dependencies]) — add ammonia.workspace = true.
crates/nuncio-mail/src/html_sanitize.rs (new file) — a small module exposing something like:
```
pub fn sanitize_inbound_html(raw_html: &str) -> String { ... }
```
Build an ammonia::Builder (do not use the bare ammonia::clean(..) free function unmodified — verified via the ammonia docs that its default tag allowlist includes img with src as an allowed attribute, meaning a bare ammonia::clean() call would NOT strip a remote tracking-pixel <img src="http://evil.example/track.gif">; ammonia's defaults DO already strip <script>/<style> tag contents and do not grant a generic style= attribute, so script injection and most inline-CSS exfil are already covered by the defaults). On top of Builder::default(), additionally remove the tracker/exfil vectors the defaults leave open — e.g. .rm_tags(["img"]) to drop remote-image tracking pixels entirely (simplest, safest choice for an engine-level sanitizer with no rendering-policy context; a future client-side "load remote images on request" feature is a presentation-layer concern for the separate client repos, not this engine). Document this specific defaults gap and the mitigation in a doc comment on sanitize_inbound_html, citing what was verified rather than assuming ammonia's defaults were already sufficient.
crates/nuncio-mail/src/lib.rs:6-18 — add pub mod html_sanitize; and re-export pub use html_sanitize::sanitize_inbound_html; alongside the existing pub use lines.

crates/nuncio-mail/src/parser.rs:91 (MimeParserAdapter::parse_mime) — change:

let body_html = msg.body_html(0).map(|b| b.to_string());

to sanitize before storing:

let body_html = msg.body_html(0).map(|b| crate::html_sanitize::sanitize_inbound_html(&b));

crates/nuncio-mail/src/parser.rs:136-... (#[cfg(test)] mod tests) — the existing parse_valid_mime_email_with_text_and_html test (parser.rs:141 onward) asserts on body_html content; verify/update its fixture and assertions still make sense once sanitization is wired in (a plain <p>...</p> fixture should pass through unchanged; if the existing fixture contains anything ammonia would alter, update the assertion, don't weaken the sanitizer to match the old assertion).

Implementation steps

Add the ammonia dependency at the workspace and nuncio-mail crate level.
Write crates/nuncio-mail/src/html_sanitize.rs with sanitize_inbound_html(raw_html: &str) -> String, using an ammonia::Builder configured to remove the tracking/exfil gaps identified above. Keep the function total (never panics, never returns Result — sanitization of arbitrary untrusted HTML should always produce some safe output, even if the input is malformed markup; ammonia is designed for exactly this and does not error on malformed input).
Wire it into parser.rs:91 as shown above.
Add pub mod html_sanitize; + re-export to lib.rs.
Add malicious-input test fixtures per Acceptance Criteria.
Run cargo check-all/cargo test-all and confirm the existing parse_valid_mime_email_with_text_and_html test and any other test touching body_html (search body_html across crates/nuncio-mail, crates/nuncio-store, crates/nunciod, crates/nuncio-cli for fixtures that assert exact string equality — a handful exist, e.g. crates/nuncio-store/src/db.rs:1893, crates/nunciod/src/grpc.rs message-roundtrip tests) — none of those construct Email.body_html via MimeParserAdapter::parse_mime (they build the struct literal directly with pre-set values), so they are unaffected by this change; confirm this while implementing rather than assuming.

Acceptance criteria

sanitize_inbound_html strips <script>...</script> content entirely from a fixture containing <script>alert('xss')</script>.
sanitize_inbound_html strips inline event-handler attributes — a fixture like <img src="x" onerror="alert('xss')"> must not contain onerror in the output (and, given the img-removal choice above, the <img> tag itself is gone too — assert both).
sanitize_inbound_html neutralizes javascript: URLs — a fixture like <a href="javascript:alert('xss')">click</a> must not retain an executable javascript: href in the output.
sanitize_inbound_html removes remote tracking pixels — a fixture like <img src="http://evil.example/track.gif" width="1" height="1"> produces output containing neither <img nor evil.example.
sanitize_inbound_html preserves ordinary safe formatting — a fixture like <p><b>Hello</b> <i>world</i>, see <a href="https://example.com">this link</a>.</p> still renders as recognizable safe HTML with the <b>/<i>/<a href="https://example.com"> markup intact (not degraded to plain text).
MimeParserAdapter::parse_mime calls the sanitizer on the extracted HTML body before it is returned in the Email — add a test constructing a raw RFC 5322 MIME payload (mirror parse_valid_mime_email_with_text_and_html, parser.rs:141) whose HTML part contains a <script> tag, and assert the resulting Email.body_html does not contain <script>.
crates/nuncio-mail/src/smtp.rs (outbound send path) is untouched — no sanitization is applied to user-composed outbound HTML.
cargo verify green (fmt + clippy -D warnings all-targets + test-all), 0 ignored.

Definition of done & process

Follow docs/STORY-WORKFLOW.md: no fabricated data; comment convention (intent-only, no story/GH/phase refs in .rs/.proto); mock-only tests. Branch story/m6-html-sanitize off dev; PR to dev with the gate output pasted as evidence; the PO reviews before the issue is closed.

Dependencies / notes

Not a [PROTO] story — Message.body_html already exists in nuncio.proto (nuncio.proto:298,394); this story changes what value flows into it, not the contract shape. No file overlap with other M6 stories. Verify the exact ammonia version's default tag/attribute allowlist against the version actually pulled in (cargo tree -p nuncio-mail | grep ammonia or equivalent) before finalizing the Builder configuration — this issue's claims about defaults (script/style stripped, img+src allowed, no generic style= attribute) were verified against ammonia 4.1.2's published docs, but double-check against whatever version cargo add actually resolves. Size: M.

## Context Story for milestone M6. **Read [`docs/STORY-WORKFLOW.md`](https://github.com/KofTwentyTwo/nuncio/blob/dev/docs/STORY-WORKFLOW.md) first** — it defines the process, gates, and patterns; this issue defines only the specific work. `crates/nuncio-mail/src/parser.rs::MimeParserAdapter::parse_mime` extracts the HTML body of a fetched message straight from `mail-parser`'s output with **zero sanitization**: `let body_html = msg.body_html(0).map(|b| b.to_string());` (`parser.rs:91`), stored directly into `nuncio_core::model::Email::body_html` (`parser.rs:130`, field defined at `crates/nuncio-core/src/model.rs:85`). Verified in the tree: this raw, untrusted HTML flows unmodified through `DatabaseEngine::save_email`/`get_message`/`list_messages` (encrypted at rest and decrypted back byte-for-byte, `crates/nuncio-store/src/db.rs:734-737,839,891` etc.), is placed directly into the gRPC `Message.body_html` field (`crates/nuncio-proto/proto/nuncio/v1/nuncio.proto:298`, wired at `crates/nunciod/src/grpc.rs:399`: `body_html: email.body_html,`), and is emitted as-is by the CLI's JSON output (`crates/nuncio-cli/src/runner.rs:56`: `"body_html": message.body_html,`). There is no `ammonia` (or any HTML-sanitizing) dependency anywhere in the active workspace today (confirmed via a full-tree search — the only sanitization code that ever existed lived in `_reference/nuncio-gui/src/sandbox.rs`, which is out-of-tree per this repo's current architecture: presentation shells are being rebuilt as separate client repos, and CLAUDE.md requires the *engine* — not each client — to be the thing that guarantees this). Any script tag, inline event handler, `javascript:` link, or CSS/`<img>`-based tracking pixel present in a received email's HTML part is preserved verbatim all the way out to every current and future client. ## Goal Add a real HTML sanitizer to the engine (using `ammonia`) and wire it into the one place untrusted HTML enters the system — `MimeParserAdapter::parse_mime` — so `Email.body_html` is *always* safe-to-render HTML by the time it reaches storage, the gRPC API, or the CLI. This must not merely strip everything to plain text (that would silently degrade every HTML email); it must produce sanitized HTML: no `<script>`, no inline event-handler attributes (`onerror`, `onclick`, etc.), no `javascript:`/`data:text/html` URLs, and no `<style>`/CSS-based exfiltration vectors, while preserving ordinary formatting markup (bold, links, lists, tables, etc.). Outbound compose/send paths (`crates/nuncio-mail/src/smtp.rs`) are user-authored content, not untrusted input, and are explicitly OUT of scope — do not sanitize outgoing mail. ## Files to touch / patterns to mirror - `Cargo.toml` (workspace root, `[workspace.dependencies]`, `Cargo.toml:36-86`) — add `ammonia = "4.1"` (verified current on docs.rs as of this writing; confirm the latest 4.x patch when implementing). - `crates/nuncio-mail/Cargo.toml:12-27` (`[dependencies]`) — add `ammonia.workspace = true`. - `crates/nuncio-mail/src/html_sanitize.rs` (**new file**) — a small module exposing something like: ```rust pub fn sanitize_inbound_html(raw_html: &str) -> String { ... } ``` Build an `ammonia::Builder` (do not use the bare `ammonia::clean(..)` free function unmodified — verified via the ammonia docs that its *default* tag allowlist includes `img` with `src` as an allowed attribute, meaning a bare `ammonia::clean()` call would NOT strip a remote tracking-pixel `<img src="http://evil.example/track.gif">`; ammonia's defaults DO already strip `<script>`/`<style>` tag *contents* and do not grant a generic `style=` attribute, so script injection and most inline-CSS exfil are already covered by the defaults). On top of `Builder::default()`, additionally remove the tracker/exfil vectors the defaults leave open — e.g. `.rm_tags(["img"])` to drop remote-image tracking pixels entirely (simplest, safest choice for an engine-level sanitizer with no rendering-policy context; a future client-side "load remote images on request" feature is a presentation-layer concern for the separate client repos, not this engine). Document this specific defaults gap and the mitigation in a doc comment on `sanitize_inbound_html`, citing what was verified rather than assuming ammonia's defaults were already sufficient. - `crates/nuncio-mail/src/lib.rs:6-18` — add `pub mod html_sanitize;` and re-export `pub use html_sanitize::sanitize_inbound_html;` alongside the existing `pub use` lines. - `crates/nuncio-mail/src/parser.rs:91` (`MimeParserAdapter::parse_mime`) — change: ```rust let body_html = msg.body_html(0).map(|b| b.to_string()); ``` to sanitize before storing: ```rust let body_html = msg.body_html(0).map(|b| crate::html_sanitize::sanitize_inbound_html(&b)); ``` - `crates/nuncio-mail/src/parser.rs:136-...` (`#[cfg(test)] mod tests`) — the existing `parse_valid_mime_email_with_text_and_html` test (`parser.rs:141` onward) asserts on `body_html` content; verify/update its fixture and assertions still make sense once sanitization is wired in (a plain `<p>...</p>` fixture should pass through unchanged; if the existing fixture contains anything ammonia would alter, update the assertion, don't weaken the sanitizer to match the old assertion). ## Implementation steps 1. Add the `ammonia` dependency at the workspace and `nuncio-mail` crate level. 2. Write `crates/nuncio-mail/src/html_sanitize.rs` with `sanitize_inbound_html(raw_html: &str) -> String`, using an `ammonia::Builder` configured to remove the tracking/exfil gaps identified above. Keep the function total (never panics, never returns `Result` — sanitization of arbitrary untrusted HTML should always produce *some* safe output, even if the input is malformed markup; `ammonia` is designed for exactly this and does not error on malformed input). 3. Wire it into `parser.rs:91` as shown above. 4. Add `pub mod html_sanitize;` + re-export to `lib.rs`. 5. Add malicious-input test fixtures per Acceptance Criteria. 6. Run `cargo check-all`/`cargo test-all` and confirm the existing `parse_valid_mime_email_with_text_and_html` test and any other test touching `body_html` (search `body_html` across `crates/nuncio-mail`, `crates/nuncio-store`, `crates/nunciod`, `crates/nuncio-cli` for fixtures that assert exact string equality — a handful exist, e.g. `crates/nuncio-store/src/db.rs:1893`, `crates/nunciod/src/grpc.rs` message-roundtrip tests) — none of those construct `Email.body_html` via `MimeParserAdapter::parse_mime` (they build the struct literal directly with pre-set values), so they are unaffected by this change; confirm this while implementing rather than assuming. ## Acceptance criteria - [ ] `sanitize_inbound_html` strips `<script>...</script>` content entirely from a fixture containing `<script>alert('xss')</script>`. - [ ] `sanitize_inbound_html` strips inline event-handler attributes — a fixture like `<img src="x" onerror="alert('xss')">` must not contain `onerror` in the output (and, given the `img`-removal choice above, the `<img>` tag itself is gone too — assert both). - [ ] `sanitize_inbound_html` neutralizes `javascript:` URLs — a fixture like `<a href="javascript:alert('xss')">click</a>` must not retain an executable `javascript:` href in the output. - [ ] `sanitize_inbound_html` removes remote tracking pixels — a fixture like `<img src="http://evil.example/track.gif" width="1" height="1">` produces output containing neither `<img` nor `evil.example`. - [ ] `sanitize_inbound_html` preserves ordinary safe formatting — a fixture like `<p><b>Hello</b> <i>world</i>, see <a href="https://example.com">this link</a>.</p>` still renders as recognizable safe HTML with the `<b>`/`<i>`/`<a href="https://example.com">` markup intact (not degraded to plain text). - [ ] `MimeParserAdapter::parse_mime` calls the sanitizer on the extracted HTML body before it is returned in the `Email` — add a test constructing a raw RFC 5322 MIME payload (mirror `parse_valid_mime_email_with_text_and_html`, `parser.rs:141`) whose HTML part contains a `<script>` tag, and assert the resulting `Email.body_html` does not contain `<script>`. - [ ] `crates/nuncio-mail/src/smtp.rs` (outbound send path) is untouched — no sanitization is applied to user-composed outbound HTML. - [ ] `cargo verify` green (fmt + clippy `-D warnings` all-targets + test-all), 0 ignored. ## Definition of done & process Follow [`docs/STORY-WORKFLOW.md`](https://github.com/KofTwentyTwo/nuncio/blob/dev/docs/STORY-WORKFLOW.md): no fabricated data; comment convention (intent-only, no story/GH/phase refs in `.rs`/`.proto`); mock-only tests. Branch `story/m6-html-sanitize` off `dev`; PR to `dev` with the gate output pasted as evidence; the PO reviews before the issue is closed. ## Dependencies / notes Not a [PROTO] story — `Message.body_html` already exists in `nuncio.proto` (`nuncio.proto:298,394`); this story changes what value flows into it, not the contract shape. No file overlap with other M6 stories. Verify the exact `ammonia` version's default tag/attribute allowlist against the version actually pulled in (`cargo tree -p nuncio-mail | grep ammonia` or equivalent) before finalizing the `Builder` configuration — this issue's claims about defaults (script/style stripped, `img`+`src` allowed, no generic `style=` attribute) were verified against ammonia 4.1.2's published docs, but double-check against whatever version `cargo add` actually resolves. Size: M.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: KofTwentyTwo/nuncio#204