Inline images in a message load through a signed link (#218)

The message body renders in a sandbox that sends no session cookie, so every cid: image was a 302 to the login page. They now load through /mail/inline/{id}, signed with an expiry of one to two days, for the message's own image parts only; parts labelled octet-stream are served when their bytes are an image. Closes #213.

Co-authored-by: Philippe Delodder <philippe@delodder.be>
Co-authored-by: MontyWei <62734446+montywei@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

CHANGELOG.md

@@ -8,6 +8,16 @@ The published image tags: `latest` follows the most recent release below,
 
 ## Unreleased
 
+**Inline images in a message show up again.** Pictures that a mail carries inside itself, such as
+a logo or the icons in a booking confirmation, rendered as empty boxes: the message body is shown in
+a sandbox that sends no session cookie, so each image was answered with a redirect to the login
+page. They now load through a signed link (`/mail/inline/{id}`) that only answers for an image of
+the message being read, and nothing that is not a picture. The link stops working after a day or
+two, so one that leaks through a forward or a log does not open the image for good; the message
+signs a fresh one each time it is shown. Images labelled `application/octet-stream` or
+`image/octet-stream` load the same way when their bytes are a picture. Messages already in the
+mailbox are fixed too, no backfill needed. No schema change.
+
 **Single sign-on can be the only way in.** A new setting under Admin → SSO, **Single sign-on only**
 (or `OIDC_DISABLE_PASSWORD_LOGIN=true` in the environment), hides the password form and makes the
 server refuse a password sign-in, so everybody comes in through the provider. It is in force only

config/packages/security.yaml

@@ -242,6 +242,12 @@ security:
         # check still runs per fetch in App\Service\Mail\ImageProxyFetcher. See
         # App\Controller\Mail\ImageProxyController for the full argument.
         - { path: ^/mail/image-proxy$, roles: PUBLIC_ACCESS }
+        # An inline (cid:) image of a message, anonymous for the same reason as
+        # the proxy above: the sandboxed body sends no session cookie, so behind
+        # ROLE_USER every inline image was a 302 to /login. The gate is the HMAC
+        # over the part id that the route checks itself, and it serves nothing
+        # but raster images. See App\Controller\Mail\InlineAttachmentController.
+        - { path: ^/mail/inline/\d+$, roles: PUBLIC_ACCESS }
         - { path: ^/, roles: ROLE_USER }
 
 when@test:

src/Controller/Mail/AttachmentController.php

@@ -90,26 +90,18 @@ final class AttachmentController extends AbstractController
         return $response;
     }
 
-    /** Enough of a file for its magic bytes; libmagic reads no further for raster formats. */
-    private const int SNIFF_BYTES = 4096;
-
     #[Route('/mail/attachment/{id}', name: 'app_mail_attachment', methods: ['GET'], requirements: ['id' => '\d+'])]
     public function serve(MessagePart $part, Request $request): Response
     {
         $this->denyAccessUnlessGranted(OwnershipVoter::OWN, $part);
 
         $absolutePath = $this->attachmentResolver->absolutePathFor($part);
-        $contentType  = $part->contentType ?? 'application/octet-stream';
-
         // A part labelled `application/octet-stream` or `image/octet-stream` is
-        // very often an inline image whose sender did not say so. For those
-        // labels only, the bytes decide — and only a type on the allow-list can
-        // come out of InlineDisposition::sniff(), so this cannot make anything
-        // inline that the list would refuse.
-        if (true === InlineDisposition::isUnlabelled($contentType)) {
-            $head        = @file_get_contents($absolutePath, false, null, 0, self::SNIFF_BYTES);
-            $contentType = (false === $head ? null : InlineDisposition::sniff($head)) ?? $contentType;
-        }
+        // very often an inline image whose sender did not say so; the resolver
+        // lets the bytes decide for those labels only. See inlineTypeFor().
+        $contentType  = $this->attachmentResolver->inlineTypeFor($part, $absolutePath)
+            ?? $part->contentType
+            ?? 'application/octet-stream';
 
         // An allow-list, not a prefix test. `image/svg+xml` starts with
         // "image/" and executes script when loaded as a document, and the type

src/Controller/Mail/InlineAttachmentController.php

@@ -0,0 +1,89 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Controller\Mail;
+
+use App\Domain\Helper\InlineDisposition;
+use App\Entity\Mail\MessagePart;
+use App\Service\Mail\AttachmentResolver;
+use App\Service\Mail\InlineAttachmentSigner;
+use Doctrine\ORM\EntityManagerInterface;
+use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
+use Symfony\Component\HttpFoundation\BinaryFileResponse;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\HttpFoundation\ResponseHeaderBag;
+use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
+use Symfony\Component\Routing\Attribute\Route;
+
+/**
+ * The one route an inline image in a message body is loaded through.
+ *
+ * WHY THIS ROUTE IS ANONYMOUS (no `#[IsGranted]`, PUBLIC_ACCESS in security.yaml)
+ * ------------------------------------------------------------------------------
+ * The body renders in a sandbox WITHOUT `allow-same-origin`, so the frame has an
+ * opaque origin and the browser sends NO session cookie with its subresource
+ * loads. Behind `ROLE_USER` a `/mail/attachment/{id}` image was answered with a
+ * 302 to the login page, and every inline image showed as an empty box. The
+ * signature is the authorization instead, as it is for the image proxy (see
+ * {@see ImageProxyController}): an HMAC over the part id that only
+ * {@see \App\Service\Mail\InlineAttachmentLinker} mints, for the parts of a
+ * message it is rendering for someone who may read it.
+ *
+ * Deliberately narrower than {@see AttachmentController::serve()}: this route
+ * serves a part only when {@see AttachmentResolver::inlineTypeFor()} finds a
+ * raster type for it (its stated type, or its bytes when the sender labelled it
+ * `octet-stream`), so an anonymous caller holding a signature can fetch an
+ * image and nothing else. The signature expires (see
+ * {@see InlineAttachmentSigner}). Every refusal is the same 404, so a caller
+ * cannot tell a bad or expired signature from a part that does not exist or is
+ * not an image.
+ */
+final class InlineAttachmentController extends AbstractController
+{
+    public function __construct(
+        private readonly InlineAttachmentSigner $signer,
+        private readonly AttachmentResolver $attachmentResolver,
+    ) {}
+
+    #[Route('/mail/inline/{id}', name: 'app_mail_inline_attachment', methods: ['GET'], requirements: ['id' => '\d+'])]
+    public function serve(int $id, Request $request, EntityManagerInterface $entityManager): Response
+    {
+        $signature = (string) $request->query->get('s', '');
+        $expires   = $request->query->getInt('e');
+
+        if ('' === $signature || false === $this->signer->isValid($id, $expires, $signature)) {
+            throw new NotFoundHttpException();
+        }
+
+        $part = $entityManager->find(MessagePart::class, $id);
+
+        if (false === $part instanceof MessagePart) {
+            throw new NotFoundHttpException();
+        }
+
+        $absolutePath = $this->attachmentResolver->absolutePathFor($part);
+        $contentType  = $this->attachmentResolver->inlineTypeFor($part, $absolutePath);
+
+        if (null === $contentType) {
+            throw new NotFoundHttpException();
+        }
+
+        $response = new BinaryFileResponse($absolutePath);
+        $response->headers->set('Content-Type', InlineDisposition::servedType($contentType));
+        $response->headers->set('X-Content-Type-Options', 'nosniff');
+        $response->headers->set('Content-Security-Policy', InlineDisposition::SANDBOX_CSP);
+        // The bytes of a part never change under its id, so the browser may
+        // keep them, but no longer than the link works; `private` keeps a
+        // shared cache out of it.
+        $response->setPrivate();
+        $response->setMaxAge(min(InlineAttachmentSigner::LIFETIME, $this->signer->secondsLeft($expires)));
+        $response->setContentDisposition(
+            ResponseHeaderBag::DISPOSITION_INLINE,
+            InlineDisposition::headerFilename($part->filename),
+        );
+
+        return $response;
+    }
+}

src/Service/Mail/AttachmentResolver.php

@@ -5,6 +5,7 @@ declare(strict_types=1);
 namespace App\Service\Mail;
 
 use App\Domain\Helper\AttachmentStorageHelper;
+use App\Domain\Helper\InlineDisposition;
 use App\Entity\Mail\Account;
 use App\Entity\Mail\MessagePart;
 use Doctrine\ORM\EntityManagerInterface;
@@ -30,6 +31,9 @@ final readonly class AttachmentResolver
     private const string GMAIL_SCHEME = 'gmail://';
     private const string GRAPH_SCHEME = 'msgraph://';
 
+    /** Enough of a file for its magic bytes; libmagic reads no further for raster formats. */
+    private const int SNIFF_BYTES = 4096;
+
     public function __construct(
         private AttachmentStorageHelper $attachmentStorage,
         private GmailApiClient          $gmailApiClient,
@@ -57,6 +61,34 @@ final readonly class AttachmentResolver
         return $this->attachmentStorage->getAbsolutePath($storagePath);
     }
 
+    /**
+     * The raster type this part may be shown inline as, or null when it may not.
+     *
+     * A stated type on the {@see InlineDisposition} list is taken at its word.
+     * A part labelled `application/octet-stream` or `image/octet-stream` is
+     * very often an inline image whose sender did not say so: for those labels
+     * only, the bytes decide, and only a type on the list can come out of
+     * {@see InlineDisposition::sniff()}. Any other stated type is never
+     * second-guessed, so a declared `text/html` cannot be re-read as an image.
+     *
+     * Shared by the attachment route and the signed inline route, so the two
+     * cannot disagree about which parts are pictures.
+     */
+    public function inlineTypeFor(MessagePart $part, string $absolutePath): ?string
+    {
+        if (true === InlineDisposition::allows($part->contentType)) {
+            return InlineDisposition::normalise($part->contentType);
+        }
+
+        if (false === InlineDisposition::isUnlabelled($part->contentType)) {
+            return null;
+        }
+
+        $head = @file_get_contents($absolutePath, false, null, 0, self::SNIFF_BYTES);
+
+        return false === $head ? null : InlineDisposition::sniff($head);
+    }
+
     // ── Private ───────────────────────────────────────────────────────────────
 
     private function materialise(MessagePart $part, string $storagePath, string $scheme): string

src/Service/Mail/InlineAttachmentLinker.php

@@ -0,0 +1,85 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Service\Mail;
+
+use App\Domain\Helper\InlineDisposition;
+use App\Entity\Mail\Message;
+
+/**
+ * Points a message's own inline images at their signed URL, at render time.
+ *
+ * MailBodySanitizer turns `cid:` into `/mail/attachment/{id}` at ingest, and
+ * that route needs a session the sandboxed reading frame cannot send. This
+ * rewrites those references on the way to the template, so it also repairs
+ * every message already in the database without a backfill, and a rotated
+ * secret never leaves a stale signature stored in a body.
+ *
+ * WHY ONLY THE MESSAGE'S OWN PARTS
+ * --------------------------------
+ * The stored body is the sender's HTML. RelativeUrlAttributeSanitizer lets any
+ * `/mail/attachment/<digits>` through, because it cannot know which ids belong
+ * to this message, so a sender can write the id of somebody else's attachment.
+ * Signing every id found in the body would turn that into a capability for it.
+ * The map is built from {@see Message::$messageParts} instead, so an id that
+ * is not one of this message's parts is left exactly as it was, still behind
+ * the login.
+ *
+ * Only parts that can be an image are signed: a type on the
+ * {@see InlineDisposition} allow-list, or an `octet-stream` label, which many
+ * senders put on their inline logos. Whether such a part really is an image is
+ * decided from its bytes when it is fetched, not here, so rendering a body
+ * never reads a file or downloads a Gmail or Graph attachment. Any other part
+ * stays on the authenticated route, where a click from the top-level page (with
+ * a cookie) still downloads it.
+ *
+ * Only `src` attributes and CSS `url()` are rewritten, the places a browser
+ * loads an image from. A link (`href`) keeps the authenticated URL.
+ */
+final readonly class InlineAttachmentLinker
+{
+    public function __construct(
+        private InlineAttachmentSigner $signer,
+    ) {
+    }
+
+    public function link(?string $html, Message $message): ?string
+    {
+        if (null === $html || false === str_contains($html, '/mail/attachment/')) {
+            return $html;
+        }
+
+        $signable = [];
+
+        foreach ($message->messageParts as $part) {
+            if (
+                null === $part->id
+                || (false === InlineDisposition::allows($part->contentType)
+                    && false === InlineDisposition::isUnlabelled($part->contentType))
+            ) {
+                continue;
+            }
+
+            $signable[$part->id] = true;
+        }
+
+        if ([] === $signable) {
+            return $html;
+        }
+
+        // Anchored on what a URL is loaded from (`src="…"`, `url(…)`, quoted or
+        // not, including the `&quot;` an attribute holds a quote as) and on the
+        // digits ending right there, so `/mail/attachment/12/preview` or
+        // `/mail/attachment/12?download=1` are not touched.
+        return (string) preg_replace_callback(
+            '#(\bsrc\s*=\s*["\']|\burl\(\s*(?:["\']|&quot;|&\#0*39;)?)/mail/attachment/(\d+)(?=["\')&])#i',
+            fn (array $m): string => true === isset($signable[(int) $m[2]])
+                // Escaped: the URL now carries two parameters, and a bare `&`
+                // in an attribute is a character reference waiting to happen.
+                ? $m[1] . htmlspecialchars($this->signer->urlFor((int) $m[2]), ENT_QUOTES | ENT_HTML5)
+                : $m[0],
+            $html,
+        );
+    }
+}

src/Service/Mail/InlineAttachmentSigner.php

@@ -0,0 +1,97 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Service\Mail;
+
+use Symfony\Component\Clock\ClockInterface;
+use Symfony\Component\DependencyInjection\Attribute\Autowire;
+use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
+
+/**
+ * Mints and checks the signed URLs an inline image is loaded through.
+ *
+ * Why an inline image needs a signature at all: the message body renders in a
+ * sandbox without `allow-same-origin`, so its origin is opaque and the browser
+ * sends no session cookie with the frame's `<img>` loads. Behind `ROLE_USER`
+ * every `cid:` image came back as a 302 to the login page and rendered as an
+ * empty box. The image proxy hit the same wall and answers on an HMAC instead
+ * (see {@see ImageProxySigner}); this is the same design for a part we already
+ * hold.
+ *
+ * What the signature authorises, exactly: reading ONE part, by id, as an image,
+ * until it expires. It carries no user and no message, so it is a capability
+ * for that part and nothing else. It is minted only by
+ * {@see InlineAttachmentLinker}, which signs the parts of the message being
+ * rendered for a reader who is allowed to see it, and the route behind it still
+ * refuses anything that is not an inline raster image.
+ *
+ * WHY IT EXPIRES
+ * --------------
+ * A link without an expiry would outlive the reason it was minted: one that
+ * leaks through a forward, a copied link or a log would serve that image to
+ * anyone, for good, even after the reader lost access to the mailbox. Inline
+ * parts are signatures, receipts and photos, which is more personal than the
+ * remote pictures the image proxy fetches. The body is re-rendered, and so
+ * re-signed, on every view, so a short lifetime costs a reader nothing.
+ *
+ * The expiry is the end of the day AFTER the current UTC day, not "now + 24h":
+ * every render on the same day mints the same URL, so the browser's cache still
+ * answers a message opened twice. A link therefore lives between one and two
+ * days.
+ *
+ * The payload is prefixed so a signature minted here can never be replayed as
+ * one for the image proxy, which signs a bare URL with the same secret.
+ */
+final readonly class InlineAttachmentSigner
+{
+    private const int SIGNATURE_LENGTH = 32;
+
+    /** One day; a link lives between one and two of them. */
+    public const int LIFETIME = 86400;
+
+    public function __construct(
+        private UrlGeneratorInterface $urlGenerator,
+        private ClockInterface $clock,
+        #[Autowire('%kernel.secret%')]
+        private string $secret,
+    ) {
+    }
+
+    public function sign(int $partId, int $expires): string
+    {
+        return substr(
+            hash_hmac('sha256', 'inline-attachment:' . $partId . ':' . $expires, $this->secret),
+            0,
+            self::SIGNATURE_LENGTH,
+        );
+    }
+
+    public function isValid(int $partId, int $expires, string $signature): bool
+    {
+        return $this->secondsLeft($expires) > 0
+            && hash_equals($this->sign($partId, $expires), $signature);
+    }
+
+    /** How long a link with this expiry still works; zero or less once it has expired. */
+    public function secondsLeft(int $expires): int
+    {
+        return $expires - $this->clock->now()->getTimestamp();
+    }
+
+    /**
+     * Absolute PATH, like every other URL written into a mail body: no request
+     * context is needed, and a relative src inside the frame resolves against
+     * the parent document, which is this app.
+     */
+    public function urlFor(int $partId): string
+    {
+        $expires = (intdiv($this->clock->now()->getTimestamp(), self::LIFETIME) + 2) * self::LIFETIME;
+
+        return $this->urlGenerator->generate(
+            'app_mail_inline_attachment',
+            ['id' => $partId, 'e' => $expires, 's' => $this->sign($partId, $expires)],
+            UrlGeneratorInterface::ABSOLUTE_PATH,
+        );
+    }
+}

src/Service/Mail/MessageRenderer.php

@@ -55,6 +55,7 @@ final readonly class MessageRenderer
 
     public function __construct(
         private RemoteContentBlocker         $blocker,
+        private InlineAttachmentLinker       $inlineImages,
         private QuoteCollapser               $quoteCollapser,
         private SenderIdentityChecker        $identityChecker,
         private ThreadSenderFacts            $trustedSenders,
@@ -116,7 +117,7 @@ final readonly class MessageRenderer
         // are pure display transformations over the same safe HTML — the image
         // counts the blocker measured are about images, so wrapping a quote
         // leaves them untouched.
-        $blocked = $this->blocker->rewrite($message->bodyHtmlSafe, $allow);
+        $blocked = $this->blocker->rewrite($this->inlineImages->link($message->bodyHtmlSafe, $message), $allow);
         $content = new RemoteContent(
             $this->quoteCollapser->collapse($blocked->html),
             $blocked->blocked,

tests/Controller/Mail/InlineAttachmentAccessTest.php

@@ -0,0 +1,220 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Controller\Mail;
+
+use App\Domain\Enum\Mail\LabelRole;
+use App\Entity\Mail\Message;
+use App\Entity\Mail\MessagePart;
+use App\Service\Mail\InlineAttachmentSigner;
+use App\Tests\Support\Mail\SeedsMarkerFixtures;
+use Doctrine\ORM\EntityManagerInterface;
+use Symfony\Bundle\FrameworkBundle\KernelBrowser;
+use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
+use Symfony\Component\Clock\MockClock;
+use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
+
+/**
+ * An inline image answers a browser that holds no session (#213).
+ *
+ * The mail body renders in a sandbox with no `allow-same-origin`, so the frame
+ * sends no cookie with its `<img>` loads; behind ROLE_USER every `cid:` image
+ * was a 302 to /login and an empty box. The route is authorized by an HMAC over
+ * the part id instead, and serves nothing but raster images.
+ */
+final class InlineAttachmentAccessTest extends WebTestCase
+{
+    use SeedsMarkerFixtures;
+
+    /** 1×1 transparent GIF. */
+    private const string GIF = 'R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7';
+
+    private KernelBrowser $client;
+    private string $dir;
+
+    protected function setUp(): void
+    {
+        $this->client = static::createClient();
+        $this->client->disableReboot();
+
+        $this->em = static::getContainer()->get(EntityManagerInterface::class);
+        $this->em->getConnection()->beginTransaction();
+
+        $this->user    = $this->seedUser();
+        $this->account = $this->seedAccount();
+        $this->inbox   = $this->seedLabel('Inbox', LabelRole::Inbox);
+
+        $this->dir = static::getContainer()->getParameter('kernel.project_dir') . '/var/test-inline-' . bin2hex(random_bytes(4));
+        mkdir($this->dir, 0o775, true);
+    }
+
+    protected function tearDown(): void
+    {
+        if (true === $this->em->getConnection()->isTransactionActive()) {
+            $this->em->getConnection()->rollBack();
+        }
+
+        array_map('unlink', glob($this->dir . '/*') ?: []);
+        rmdir($this->dir);
+
+        parent::tearDown();
+    }
+
+    public function testAnAnonymousRequestWithAValidSignatureGetsTheImage(): void
+    {
+        $part = $this->part('image/gif');
+
+        // No loginUser(): this is the sandbox frame's cookieless request.
+        $this->client->request('GET', $this->signedUrl($part));
+
+        $response = $this->client->getResponse();
+
+        self::assertResponseIsSuccessful();
+        self::assertFalse($response->isRedirection(), 'the frame must not be bounced to the login page');
+        self::assertSame('image/gif', $response->headers->get('Content-Type'));
+        self::assertSame('nosniff', $response->headers->get('X-Content-Type-Options'));
+    }
+
+    public function testWithoutASignatureTheImageIsNotThere(): void
+    {
+        $part = $this->part('image/gif');
+
+        $this->client->request('GET', '/mail/inline/' . $part->id);
+        self::assertResponseStatusCodeSame(404);
+
+        $this->client->request('GET', '/mail/inline/' . $part->id, ['s' => 'deadbeefdeadbeefdeadbeefdeadbeef']);
+        self::assertResponseStatusCodeSame(404);
+    }
+
+    public function testASignatureForAnotherPartDoesNotOpenThisOne(): void
+    {
+        $mine  = $this->part('image/gif');
+        $other = $this->part('image/gif');
+
+        $expires = time() + 3600;
+
+        $this->client->request('GET', '/mail/inline/' . $mine->id, [
+            'e' => $expires,
+            's' => static::getContainer()->get(InlineAttachmentSigner::class)->sign((int) $other->id, $expires),
+        ]);
+
+        self::assertResponseStatusCodeSame(404);
+    }
+
+    public function testEvenASignedRequestNeverServesWhatIsNotARasterImage(): void
+    {
+        $svg = $this->part('image/svg+xml');
+        $pdf = $this->part('application/pdf');
+
+        foreach ([$svg, $pdf] as $part) {
+            $this->client->request('GET', $this->signedUrl($part));
+
+            self::assertResponseStatusCodeSame(404, (string) $part->contentType . ' must stay behind the login');
+        }
+    }
+
+    /** A link that leaked through a forward or a log stops working. */
+    public function testAnExpiredLinkIsNotThere(): void
+    {
+        $part = $this->part('image/gif');
+
+        $signer = new InlineAttachmentSigner(
+            static::getContainer()->get(UrlGeneratorInterface::class),
+            new MockClock('-3 days'),
+            (string) static::getContainer()->getParameter('kernel.secret'),
+        );
+
+        $this->client->request('GET', $signer->urlFor((int) $part->id));
+
+        self::assertResponseStatusCodeSame(404);
+    }
+
+    public function testTheBrowserKeepsTheImageNoLongerThanTheLinkWorks(): void
+    {
+        $this->client->request('GET', $this->signedUrl($this->part('image/gif')));
+
+        $cacheControl = (string) $this->client->getResponse()->headers->get('Cache-Control');
+
+        self::assertStringContainsString('private', $cacheControl);
+        self::assertSame(1, preg_match('/max-age=(\d+)/', $cacheControl, $m));
+        self::assertLessThanOrEqual(InlineAttachmentSigner::LIFETIME, (int) $m[1]);
+    }
+
+    /**
+     * Vaultwarden's logo is labelled `image/octet-stream` (#213): the bytes
+     * decide, and the image goes out under what it really is.
+     */
+    public function testAnUnlabelledImageIsServedAsWhatItsBytesAre(): void
+    {
+        foreach (['image/octet-stream', 'application/octet-stream'] as $label) {
+            $this->client->request('GET', $this->signedUrl($this->part($label)));
+
+            self::assertResponseIsSuccessful($label);
+            self::assertSame('image/gif', $this->client->getResponse()->headers->get('Content-Type'));
+            self::assertSame('nosniff', $this->client->getResponse()->headers->get('X-Content-Type-Options'));
+        }
+    }
+
+    /** An unlabelled part that is markup, not a picture, never comes out of here. */
+    public function testAnUnlabelledPartThatIsNoImageIsNotThere(): void
+    {
+        $part = $this->part('application/octet-stream', '<svg xmlns="http://www.w3.org/2000/svg"><script>alert(1)</script></svg>');
+
+        $this->client->request('GET', $this->signedUrl($part));
+
+        self::assertResponseStatusCodeSame(404);
+    }
+
+    /**
+     * The logged-in route shares the decision (AttachmentResolver::inlineTypeFor),
+     * so opening an unlabelled logo shows it rather than downloading it, and an
+     * unlabelled SVG is still a download.
+     */
+    public function testTheAttachmentRouteDecidesAnUnlabelledPartTheSameWay(): void
+    {
+        $this->client->loginUser($this->user);
+
+        $image = $this->part('image/octet-stream');
+        $this->client->request('GET', '/mail/attachment/' . $image->id);
+
+        self::assertResponseIsSuccessful();
+        self::assertSame('image/gif', $this->client->getResponse()->headers->get('Content-Type'));
+        self::assertStringStartsWith('inline', (string) $this->client->getResponse()->headers->get('Content-Disposition'));
+
+        $svg = $this->part('application/octet-stream', '<svg xmlns="http://www.w3.org/2000/svg"><script>alert(1)</script></svg>');
+        $this->client->request('GET', '/mail/attachment/' . $svg->id);
+
+        self::assertResponseIsSuccessful();
+        self::assertStringStartsWith('attachment', (string) $this->client->getResponse()->headers->get('Content-Disposition'));
+    }
+
+    private function signedUrl(MessagePart $part): string
+    {
+        return static::getContainer()->get(InlineAttachmentSigner::class)->urlFor((int) $part->id);
+    }
+
+    private function part(string $contentType, ?string $bytes = null): MessagePart
+    {
+        /** @var Message $message */
+        $message = $this->thread('Mit Bild')->messages->first();
+
+        $name = bin2hex(random_bytes(4));
+        file_put_contents($this->dir . '/' . $name, $bytes ?? base64_decode(self::GIF, true));
+
+        $part              = new MessagePart();
+        $part->contentType = $contentType;
+        $part->filename    = $name;
+        $part->contentId   = $name . '@test';
+        $part->disposition = 'inline';
+        $part->isInline    = true;
+        $part->size        = 43;
+        $part->storagePath = 'var/' . basename($this->dir) . '/' . $name;
+
+        $message->addMessagePart($part);
+        $this->em->persist($part);
+        $this->em->flush();
+
+        return $part;
+    }
+}

tests/Service/Mail/InlineAttachmentLinkerTest.php

@@ -0,0 +1,200 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Service\Mail;
+
+use App\Entity\Mail\Message;
+use App\Entity\Mail\MessagePart;
+use App\Service\Mail\InlineAttachmentLinker;
+use App\Service\Mail\InlineAttachmentSigner;
+use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
+use Symfony\Component\Clock\MockClock;
+use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
+
+/**
+ * The reading frame has no session cookie, so an inline image has to be
+ * reachable by a signed URL (#213) — but only the images of the message being
+ * read.
+ *
+ * The stored body is the sender's HTML and the sanitizer lets any
+ * `/mail/attachment/<digits>` through, so a sender can name somebody else's
+ * attachment. The case that matters most below is the one where the id is not
+ * one of this message's parts: it must come out untouched.
+ */
+final class InlineAttachmentLinkerTest extends KernelTestCase
+{
+    private InlineAttachmentLinker $linker;
+    private InlineAttachmentSigner $signer;
+
+    protected function setUp(): void
+    {
+        self::bootKernel();
+
+        $this->linker = static::getContainer()->get(InlineAttachmentLinker::class);
+        $this->signer = static::getContainer()->get(InlineAttachmentSigner::class);
+    }
+
+    public function testAnImageOfThisMessageGetsItsSignedUrl(): void
+    {
+        $message = $this->message([101 => 'image/png']);
+
+        $html = $this->linker->link('<img src="/mail/attachment/101" width="16">', $message);
+
+        self::assertSame(
+            '<img src="' . $this->signed(101) . '" width="16">',
+            $html,
+        );
+        self::assertStringStartsWith('<img src="/mail/inline/101?e=', (string) $html);
+    }
+
+    public function testAnIdThatIsNotThisMessagesIsLeftBehindTheLogin(): void
+    {
+        $message = $this->message([101 => 'image/png']);
+
+        $html = '<img src="/mail/attachment/999">';
+
+        self::assertSame($html, $this->linker->link($html, $message));
+    }
+
+    public function testNothingThatIsNotARasterImageIsSigned(): void
+    {
+        $message = $this->message([
+            101 => 'image/svg+xml',
+            102 => 'application/pdf',
+            103 => 'text/html',
+        ]);
+
+        $html = '<img src="/mail/attachment/101"><img src="/mail/attachment/102"><img src="/mail/attachment/103">';
+
+        self::assertSame($html, $this->linker->link($html, $message));
+    }
+
+    /**
+     * Vaultwarden's logo is `image/octet-stream`, and `application/octet-stream`
+     * is common too (#213). Whether the bytes are an image is the route's call,
+     * when it is fetched; rendering must not read the file.
+     */
+    public function testAnUnlabelledPartIsSignedForTheRouteToSniff(): void
+    {
+        $message = $this->message([101 => 'image/octet-stream', 102 => 'application/octet-stream']);
+
+        $html = $this->linker->link('<img src="/mail/attachment/101"><img src="/mail/attachment/102">', $message);
+
+        self::assertSame(
+            '<img src="' . $this->signed(101) . '"><img src="' . $this->signed(102) . '">',
+            $html,
+        );
+    }
+
+    /** A click on a link comes from the top-level page, which has the session. */
+    public function testALinkKeepsTheAuthenticatedUrl(): void
+    {
+        $message = $this->message([101 => 'image/png']);
+
+        $html = '<a href="/mail/attachment/101">logo.png</a>';
+
+        self::assertSame($html, $this->linker->link($html, $message));
+    }
+
+    public function testOnlyTheBareImageUrlIsRewritten(): void
+    {
+        $message = $this->message([101 => 'image/png']);
+
+        $html = '<a href="/mail/attachment/101?download=1">a</a><a href="/mail/attachment/101/preview">b</a>';
+
+        self::assertSame($html, $this->linker->link($html, $message));
+    }
+
+    public function testABackgroundImageInAStyleIsSignedToo(): void
+    {
+        $message = $this->message([101 => 'image/gif']);
+        $signed  = $this->signed(101);
+
+        $html = $this->linker->link('<td style="background:url(/mail/attachment/101)"></td>', $message);
+        self::assertStringContainsString('url(' . $signed . ')', (string) $html);
+
+        $html = $this->linker->link('<td style="background:url(&quot;/mail/attachment/101&quot;)"></td>', $message);
+        self::assertStringContainsString('url(&quot;' . $signed . '&quot;)', (string) $html);
+    }
+
+    public function testABodyWithoutInlineImagesComesBackAsItWas(): void
+    {
+        $message = $this->message([101 => 'image/png']);
+
+        self::assertNull($this->linker->link(null, $message));
+        self::assertSame('<p>hi</p>', $this->linker->link('<p>hi</p>', $message));
+    }
+
+    public function testASignatureIsForOnePartAndOneExpiry(): void
+    {
+        $expires = time() + 3600;
+
+        self::assertTrue($this->signer->isValid(101, $expires, $this->signer->sign(101, $expires)));
+        self::assertFalse($this->signer->isValid(102, $expires, $this->signer->sign(101, $expires)), 'not transferable between parts');
+        self::assertFalse($this->signer->isValid(101, $expires + 86400, $this->signer->sign(101, $expires)), 'the expiry cannot be pushed out');
+        self::assertFalse($this->signer->isValid(101, $expires, ''));
+    }
+
+    /**
+     * A link leaked through a forward or a log must stop working (#213). The
+     * expiry is the end of the next UTC day, so every view on one day mints the
+     * same URL and the browser cache keeps answering.
+     */
+    public function testALinkWorksForOneToTwoDaysAndIsTheSameAllDay(): void
+    {
+        $clock  = new MockClock('2026-10-19 00:00:01 UTC');
+        $signer = $this->signerAt($clock);
+
+        $url = $signer->urlFor(101);
+        $clock->modify('2026-10-19 23:59:59 UTC');
+        self::assertSame($url, $signer->urlFor(101), 'the same URL all day, so the browser can cache it');
+
+        parse_str((string) parse_url($url, PHP_URL_QUERY), $query);
+        $expires   = (int) $query['e'];
+        $signature = (string) $query['s'];
+
+        self::assertSame(strtotime('2026-10-21 00:00:00 UTC'), $expires);
+
+        $clock->modify('2026-10-20 23:59:59 UTC');
+        self::assertTrue($signer->isValid(101, $expires, $signature));
+
+        $clock->modify('2026-10-21 00:00:00 UTC');
+        self::assertFalse($signer->isValid(101, $expires, $signature), 'expired');
+    }
+
+    /** The signed URL as it stands in HTML: `&` escaped. */
+    private function signed(int $partId): string
+    {
+        return htmlspecialchars($this->signer->urlFor($partId), ENT_QUOTES | ENT_HTML5);
+    }
+
+    private function signerAt(MockClock $clock): InlineAttachmentSigner
+    {
+        return new InlineAttachmentSigner(
+            static::getContainer()->get(UrlGeneratorInterface::class),
+            $clock,
+            (string) static::getContainer()->getParameter('kernel.secret'),
+        );
+    }
+
+    /**
+     * @param array<int, string> $parts part id => content type
+     */
+    private function message(array $parts): Message
+    {
+        $message = new Message();
+
+        foreach ($parts as $id => $contentType) {
+            $part              = new MessagePart();
+            $part->contentType = $contentType;
+
+            // The id is generated by the database; nothing here needs one.
+            new \ReflectionProperty(MessagePart::class, 'id')->setValue($part, $id);
+
+            $message->addMessagePart($part);
+        }
+
+        return $message;
+    }
+}