aboutsummaryrefslogtreecommitdiffstats
path: root/docs/superpowers/specs
diff options
context:
space:
mode:
Diffstat (limited to 'docs/superpowers/specs')
-rw-r--r--docs/superpowers/specs/2026-08-26-card-avatars-design.md10
-rw-r--r--docs/superpowers/specs/2026-08-27-forward-html-design.md181
2 files changed, 191 insertions, 0 deletions
diff --git a/docs/superpowers/specs/2026-08-26-card-avatars-design.md b/docs/superpowers/specs/2026-08-26-card-avatars-design.md
index 14829c0..e396a34 100644
--- a/docs/superpowers/specs/2026-08-26-card-avatars-design.md
+++ b/docs/superpowers/specs/2026-08-26-card-avatars-design.md
@@ -217,6 +217,16 @@ of the newly arrived mail and appends CANDIDATES, commented out:
# noreply@cofidis.it (47 messages)
```
+**How much mail the scan covers** depends on whether the list has ever been
+used. With no file, or a file holding no active entry, it scans the WHOLE
+database; afterwards it scans the last week. The first run is exactly when a
+full scan earns its cost: a week of mail proposes almost nothing, so a
+week-only rule would leave the list taking months to become useful. It is
+affordable because it happens once, measured at 76 ms over 5105 messages.
+
+A file holding only rejected candidates still counts as unused. Rescanning it
+re-proposes none of them, since anything already mentioned is skipped.
+
A candidate is an address whose local part is in a small built-in word list
(`noreply`, `no-reply`, `donotreply`, `info`, `support`, `billing`,
`newsletter`, `notifications`, `mailer-daemon`), or one that recurs with no
diff --git a/docs/superpowers/specs/2026-08-27-forward-html-design.md b/docs/superpowers/specs/2026-08-27-forward-html-design.md
new file mode 100644
index 0000000..a71cc54
--- /dev/null
+++ b/docs/superpowers/specs/2026-08-27-forward-html-design.md
@@ -0,0 +1,181 @@
+# Forwarding an HTML message with its formatting
+
+Item 171. Design, 2026-08-27. **Read this before the backlog row**, which
+records only the cause.
+
+## The defect
+
+`ComposeContextBuilder::quoteBody()` reads `ParsedMessage::plainBody` and
+nothing else, so the forward path drops the original's `htmlBody` entirely.
+Two consequences, and they are not the same severity:
+
+- An original with both parts forwards its text/plain alternative. The words
+ survive, the sender's formatting does not. This is what the user reported.
+- **An original with an HTML part ONLY has an empty `plainBody`**, so the
+ forward carries an attribution line and an empty quote. The content is gone
+ and nothing says so.
+
+Measured on the developer's own inbox 2026-08-27: **30 of 342 sampled
+messages** (~9%) declare `text/html` with no `text/plain` part. The silent
+half is not an edge case.
+
+## What was decided, and by whom
+
+The user chose, 2026-08-27, from three routes put to them:
+
+1. **Inline, carrying the original's markup** — CHOSEN. The forward carries
+ the original's own HTML rather than a flattened quote. Highest inline
+ fidelity, and the most work, because it is the only route that puts a
+ stranger's markup into an outgoing message.
+
+ **Amended 2026-08-27, after the first build**: a forward sends ONE part,
+ not a `multipart/alternative`. The Send-as-HTML toggle chooses which — the
+ original's markup when on, the text quote when off. The user's reasoning is
+ that a forward's shape is something they have already decided by flipping
+ that toggle, and sending both halves hands the choice to the recipient's
+ client instead. An ordinary (non-forward) message still sends the
+ alternative as before; only the forward path is single-part.
+2. Attach the original as `message/rfc822` (item 130's mechanism). Rejected
+ here, though item 130 may still build it for its own sake: the user wants
+ the content inline, not as an attachment.
+3. Text fallback only. Rejected: it fixes the silent-loss half and does not
+ answer the note at all, since formatting is still lost on every forward.
+
+**Remote content is STRIPPED BY DEFAULT, with a per-forward opt-out**, also
+the user's choice against "always strip" and "keep everything". A control in
+the composer, checked by default, reading roughly "Strip remote content from
+the forwarded message".
+
+## Why this is the security-critical item in the backlog
+
+Every other HTML path in this application renders a stranger's markup *to the
+user*, behind protections that live in `MessageView`: an off-the-record
+profile, JavaScript disabled, and `RequestInterceptor` blocking every request
+by default and failing closed.
+
+**None of those protections apply here.** The markup leaves this process and
+is rendered by somebody else's mail client, under their policy, on their
+machine. The interceptor cannot help: it intercepts requests *we* would make.
+So the sanitising has to happen to the bytes, before they are handed to
+`MessageSender`, and there is no second line of defence behind it.
+
+The concrete harm, in the user's own words when the decision was put to them:
+forwarding a tracking pixel forwards the tracking. The original sender learns
+that the forwarded copy was opened, by whom, and how many times, and the
+user's recipient never consented to that.
+
+## The allow-list rule, which is not negotiable
+
+`HtmlBuilder::namespaceCids()` is the closest prior art and it is a
+**block-list**: it names the attributes that can carry a `cid:` and rewrites
+those. It documents scoping `srcset=` out, on the reasoning that its quoting
+grammar differs and `cid:` in `srcset` is not seen in the wild.
+
+**That trade is correct for rewriting and WRONG for stripping**, and the
+asymmetry is the whole design:
+
+| | a missed reference means |
+|---|---|
+| `namespaceCids` (rewrite) | one broken image |
+| this sanitiser (strip) | a tracking beacon reaching the recipient |
+
+So the sanitiser must **allow-list what may remain**, not block-list what must
+go. Anything not recognised is removed. A new HTML attribute, a quoting form
+not anticipated, a `srcset`, a CSS `image-set()`, an `@import` — each is
+handled by the default, which is removal, rather than by having been
+enumerated in advance.
+
+Stated as the invariant to test against: **after sanitising, no attribute
+value and no CSS construct in the output may contain a URL whose scheme is
+anything other than `cid:`, and no element that fetches may remain without
+one.**
+
+## What is kept and what goes
+
+Kept:
+
+- `cid:` references. They travel inside the message, fetch nothing, and are
+ what makes an inline logo survive. Item 129 will need the same machinery.
+- Structural and presentational markup: tables, lists, headings, spans,
+ `style=""` attributes with their remote constructs removed.
+
+Removed:
+
+- Any `src`, `href`, `background`, `poster`, `srcset`, `data-*` or other
+ attribute value carrying a non-`cid:` URL scheme. `http:`, `https:`,
+ `//host/path`, `data:` (which can carry markup), `file:` above all.
+- `<link>`, `<script>`, `<iframe>`, `<object>`, `<embed>`, `<base>`, and
+ `<meta http-equiv="refresh">`.
+- CSS `url()`, `@import` and `image-set()` naming anything but a `cid:`,
+ whether in a `style=""` attribute or a `<style>` block.
+- Event-handler attributes (`onload`, `onerror`, …). The recipient's client
+ most likely disables scripting, but that is their policy and not ours to
+ assume.
+
+A stripped `<img>` leaves a gap where the image was. That is the correct
+outcome and should not be papered over with a placeholder that itself fetches.
+
+## Shape of the change
+
+- **`HtmlSanitiser`, a new namespace of free functions over values**
+ (`src/htmlsanitiser.h/.cpp`), matching `MarkdownRenderer` and
+ `MessageBuilder`. No widget, so the security property is testable without a
+ painter or a web engine. This is where the allow-list lives.
+- **`OutgoingMessage` gains the forwarded HTML and the strip flag.** It
+ currently carries `markdownBody` and `sendHtml` only, so there is nowhere to
+ put a second HTML source; both are new fields rather than a changed call
+ site.
+- **`MessageBuilder::build()` chooses the part.** On a forward with the toggle
+ ON, the body is one `text/html` part: the user's rendered markdown, a rule,
+ then the sanitised original. With the toggle OFF it is one `text/plain` part
+ carrying the markdown and the text quote. No alternative either way.
+
+ **The toggle is honoured even when the original had no plain-text part**, so
+ an HTML-only original forwarded with the toggle off goes out as the text
+ fallback and its formatting is lost. Chosen deliberately 2026-08-27 over
+ forcing HTML for those messages, so that the toggle means what it says; the
+ first build forced it and that was reversed.
+- **`quoteBody()` is not the fix and must not become it.** It is shared with
+ Reply, and the behaviour asked for is the forward's alone. It keeps
+ producing the text quote for the plain half. Its one change is the
+ silent-loss case: when `plainBody` is empty it should render `htmlBody` down
+ to text rather than emitting an empty quote, so the plain half is never
+ blank.
+- **The composer control** sits with the existing per-message toggles, checked
+ by default, and only appears on a Forward carrying HTML.
+
+## Traps recorded in advance
+
+- **`quoteBody()` is shared with Reply.** A change there reaches both paths.
+- **`QString::arg()` does not collapse `%%`.** CLAUDE.md records the 0.11.0
+ placeholder losing its mask, its glow and both gradients this way while
+ still painting a plausible pane. Any generated CSS here is subject to it.
+- **A geometry or rendering probe cannot see this.** The property under test
+ is "no remote URL survives", which is a property of the STRING. Assert on
+ the generated bytes. CLAUDE.md's "Rendering probes lie" section is the
+ general warning; here a probe that renders the result and looks at it would
+ endorse a tracking pixel it cannot see, because a 1x1 transparent image is
+ invisible by design.
+- **Test with real hostile shapes**, not tidy markup: unquoted attribute
+ values (`<img src=http://x/p>`), mixed quoting, whitespace and newlines
+ around `=`, uppercase tags and attributes, a `cid:` whose id contains a
+ URL-looking substring, and a `style` attribute carrying `url(...)` with no
+ quotes. `namespaceCids()` documents each of these forms as real.
+- **`MimeParser` must parse what we build.** A round trip through
+ `MessageBuilder` and back is the check that the nesting is right, and it is
+ cheaper than reading the RFC.
+
+## Order of work
+
+1. `HtmlSanitiser` with the allow-list, tested against the hostile shapes
+ above. Nothing else depends on decisions inside it.
+2. `quoteBody()`'s empty-plain fallback, which closes the silent-loss half on
+ its own and is independently useful.
+3. `OutgoingMessage` fields and the `MessageBuilder` nesting, with a
+ parse-back round trip.
+4. The composer control, defaulting to strip.
+5. Hand test: forward a real HTML message to yourself with the box checked and
+ unchecked, and confirm what arrives.
+
+Steps 1 and 2 are separable and each ship a real improvement, so this does not
+have to land as one commit.