Keep a mail's styles from drawing over the app around it

Inline styles survive the sanitiser on purpose: in mail they are the
design. In the reading pane that costs nothing, because the message
is in a sandboxed frame. The quote under a reply and the print page
are the sender's HTML in the app's own document, and there
`position: fixed` on a full-size element covered the composer — or
the whole window — with a page of the sender's choosing the moment
somebody pressed Reply. Reported in #34.

Two layers, since either alone has a hole:

  the sanitiser   drops `position: fixed` and `sticky` from every
                  body, and `absolute` and `z-index` too from one
                  being composed. Absolute stays for mail that is
                  only read: real newsletters use it, and in the
                  frame it can only rearrange the message.
  the stylesheet  the composer's editor and the print page's body
                  are containing blocks that clip. This is what
                  holds for a body stored before the sanitiser
                  changed, or a declaration written some way the
                  pattern does not anticipate.

Refs #34

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

src/Service/Mail/EscapingStyleAttributeSanitizer.php

@@ -0,0 +1,89 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Service\Mail;
+
+use Symfony\Component\HtmlSanitizer\HtmlSanitizerConfig;
+use Symfony\Component\HtmlSanitizer\Visitor\AttributeSanitizer\AttributeSanitizerInterface;
+
+/**
+ * Takes out of a `style` attribute the declarations that let an element leave
+ * the place it was put.
+ *
+ * The sanitiser keeps inline styles, because in mail they ARE the design. In
+ * the reading pane that costs nothing: the message is in a sandboxed frame and
+ * can only rearrange itself. But a message is also shown in the app's own
+ * document — quoted in the reply composer, and on the print page — and there
+ * `position: fixed` is not a layout choice, it is a way to draw over plMail: a
+ * mail could cover the composer with a page of its own the moment somebody
+ * pressed Reply (issue #34).
+ *
+ * Two strengths, because the two places differ in what they can afford:
+ *
+ *   everywhere   `position: fixed` and `sticky`. Neither means anything in
+ *                mail — there is no viewport to be fixed to in a message — so
+ *                nothing genuine is lost by dropping them at ingest.
+ *
+ *   composing    `position: absolute` and `z-index` as well. Absolutely
+ *                positioned content with no positioned ancestor is placed
+ *                against the page, which in the composer is the app. Some real
+ *                newsletters do position absolutely, which is why this half is
+ *                not applied to mail that is only being read.
+ *
+ * This is one of two layers. The other is in the stylesheets: the composer's
+ * editor and the print page's body are containing blocks that clip, so a
+ * declaration that got past this — in a body stored before it existed, or
+ * written some way this pattern does not anticipate — still cannot reach
+ * outside its own box.
+ */
+final readonly class EscapingStyleAttributeSanitizer implements AttributeSanitizerInterface
+{
+    public function __construct(
+        private bool $composing = false,
+    ) {
+    }
+
+    public function getSupportedElements(): ?array
+    {
+        return null;
+    }
+
+    public function getSupportedAttributes(): array
+    {
+        return ['style'];
+    }
+
+    public function sanitizeAttribute(string $element, string $attribute, string $value, HtmlSanitizerConfig $config): ?string
+    {
+        $kept = [];
+
+        foreach (explode(';', $value) as $declaration) {
+            if ('' === trim($declaration) || true === $this->escapes($declaration)) {
+                continue;
+            }
+
+            $kept[] = trim($declaration);
+        }
+
+        return implode('; ', $kept);
+    }
+
+    private function escapes(string $declaration): bool
+    {
+        [$property, $rest] = array_pad(explode(':', $declaration, 2), 2, '');
+
+        // Compared without comments, whitespace or case: `pos/**/ition` and
+        // `POSITION : Fixed` are the same declaration to a browser.
+        $property = strtolower((string) preg_replace('~/\*.*?\*/|\s+~s', '', $property));
+        $rest     = strtolower((string) preg_replace('~/\*.*?\*/|\s+~s', '', $rest));
+
+        if ('position' === $property) {
+            return true === str_contains($rest, 'fixed')
+                || true === str_contains($rest, 'sticky')
+                || (true === $this->composing && true === str_contains($rest, 'absolute'));
+        }
+
+        return true === $this->composing && 'z-index' === $property;
+    }
+}

src/Service/Mail/MailBodySanitizer.php

@@ -290,6 +290,10 @@ final readonly class MailBodySanitizer
             $config = $config->allowAttribute($marker, '*');
         }
 
+        // A composed body is shown in the app's own document, not in a frame,
+        // so the stricter half applies: see EscapingStyleAttributeSanitizer.
+        $config = $config->withAttributeSanitizer(new EscapingStyleAttributeSanitizer(composing: true));
+
         // `cid:` on top of the mail schemes, because of WHERE this runs.
         // DraftPersister::save() rewrites the editor's attachment URLs into
         // `cid:` references BEFORE the draft is persisted — the stored body is
@@ -343,6 +347,9 @@ final readonly class MailBodySanitizer
             // view puts it straight into an app page — and becomes a request,
             // with the reader's session, somewhere the sender chose.
             ->withAttributeSanitizer(new RelativeUrlAttributeSanitizer())
+            // Styles stay, but not the ones that take an element out of the
+            // message and onto the page around it.
+            ->withAttributeSanitizer(new EscapingStyleAttributeSanitizer())
             ->withMaxInputLength(self::MAX_INPUT_LENGTH);
 
         return $config;

templates/compose/_window.html.twig

@@ -523,7 +523,16 @@ Variables: form, message, ctx (an App\Domain\DTO\Mail\ComposeContext)
                      aria-multiline="true"
                      aria-label="{{ 'compose.body_placeholder'|trans }}"
                      data-placeholder="{{ 'compose.body_placeholder'|trans }}"
-                     class="w-full min-h-[170px] bg-transparent outline-none
+                     {# [contain:paint] makes the editor the containing block for
+                        anything positioned inside it, and clips to its box.
+                        The quote under a reply is the sender's HTML in the
+                        app's own document: without this, `position: fixed`
+                        in a mail covered the composer — or the whole app —
+                        with content of the sender's choosing the moment
+                        somebody pressed Reply. The sanitiser drops such
+                        declarations too; this is what holds when one gets
+                        past it. #}
+                     class="w-full min-h-[170px] bg-transparent outline-none [contain:paint]
                             text-sm leading-relaxed
                             text-ink {{ plainText ? 'hidden' : '' }}
                             empty:before:content-[attr(data-placeholder)]

templates/mail/print.html.twig

@@ -35,7 +35,13 @@ does not care which stylesheet fired it.
         .meta { border-bottom: 1px solid #e0e0e0; padding-bottom: 12px; margin-bottom: 20px; }
         .meta p { margin: 2px 0; }
         .meta .muted { color: #5f6368; font-size: 12px; }
-        .body { word-break: break-word; }
+        /* A containing block that clips. The body is a stranger's HTML put
+           straight into this page, and without this a positioned element in it
+           is placed against the page — over the header, and over the spam and
+           sender warnings above it, which are the two things this page most
+           needs the reader to see. The sanitiser drops the declarations that
+           do this; bodies stored before it did are why this is here as well. */
+        .body { word-break: break-word; position: relative; contain: layout; overflow: hidden; }
         .body img { max-width: 100%; height: auto; }
         .attachments { margin-top: 24px; border-top: 1px solid #e0e0e0; padding-top: 12px; font-size: 12px; color: #5f6368; }
         .warning { border: 1px solid #b91c1c; color: #b91c1c; border-radius: 6px; padding: 8px 12px; margin-bottom: 16px; font-size: 12px; }

tests/Service/Mail/EscapingStylesInMailTest.php

@@ -0,0 +1,80 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Service\Mail;
+
+use App\Service\Mail\MailBodySanitizer;
+use PHPUnit\Framework\TestCase;
+use Psr\Log\NullLogger;
+use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
+
+/**
+ * A mail's inline styles cannot take its content out of the message.
+ *
+ * Inline styles are kept on purpose — in mail they are the design — and the
+ * reading pane can afford that because it is a sandboxed frame. The reply
+ * composer and the print page are not: there the sender's HTML sits in the
+ * app's own document, and `position: fixed` on a full-size element drew the
+ * sender's page over plMail when the reader pressed Reply (issue #34).
+ */
+final class EscapingStylesInMailTest extends TestCase
+{
+    private MailBodySanitizer $sanitizer;
+
+    protected function setUp(): void
+    {
+        $this->sanitizer = new MailBodySanitizer(self::createStub(UrlGeneratorInterface::class), new NullLogger());
+    }
+
+    public function testAFixedElementLosesItsPositionAndKeepsItsOtherStyles(): void
+    {
+        $html = $this->sanitizer->sanitizeFragment(
+            '<div style="position:fixed; top:0; left:0; color: red">Sign in again</div>',
+        );
+
+        self::assertStringNotContainsString('position', $html);
+        self::assertStringContainsString('color: red', $html);
+        self::assertStringContainsString('Sign in again', $html);
+    }
+
+    /** The spellings a pattern match on "position:fixed" would wave through. */
+    public function testTheDeclarationIsRecognisedHoweverItIsWritten(): void
+    {
+        foreach (['POSITION : Fixed', 'position:/**/fixed', "position:\tsticky", 'position: fixed !important'] as $declaration) {
+            $html = $this->sanitizer->sanitizeFragment(sprintf('<p style="margin: 0; %s">x</p>', $declaration));
+
+            self::assertStringNotContainsStringIgnoringCase('fixed', $html, $declaration);
+            self::assertStringNotContainsStringIgnoringCase('sticky', $html, $declaration);
+            self::assertStringContainsString('margin: 0', $html, $declaration);
+        }
+    }
+
+    /**
+     * Mail that is only read keeps absolute positioning — real newsletters use
+     * it, and in the frame it can only rearrange the message. A body being
+     * composed is in the app's document, where "absolute" with no positioned
+     * ancestor means "against the page".
+     */
+    public function testAbsolutePositioningSurvivesReadingAndNotComposing(): void
+    {
+        $body = '<div style="position:absolute; z-index: 99999; inset: 0">x</div>';
+
+        self::assertStringContainsString('absolute', $this->sanitizer->sanitizeFragment($body));
+
+        $composed = $this->sanitizer->sanitizeComposedBody($body);
+
+        self::assertStringNotContainsString('absolute', $composed);
+        self::assertStringNotContainsString('z-index', $composed);
+        self::assertStringContainsString('inset: 0', $composed);
+    }
+
+    public function testOrdinaryStylesAreUntouched(): void
+    {
+        $html = $this->sanitizer->sanitizeFragment('<div style="padding: 12px; background-position: center; position: relative">x</div>');
+
+        self::assertStringContainsString('padding: 12px', $html);
+        self::assertStringContainsString('background-position: center', $html);
+        self::assertStringContainsString('position: relative', $html);
+    }
+}