From 7fb2c8f38c28b1e1c4df21674d1541054ba1de1d Mon Sep 17 00:00:00 2001 From: Philippe Delodder <7259755+phdelodder@users.noreply.github.com> Date: Mon, 19 Oct 2026 14:03:04 +0200 Subject: [PATCH] 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 Co-authored-by: MontyWei <62734446+montywei@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 --- CHANGELOG.md | 10 + config/packages/security.yaml | 6 + src/Controller/Mail/AttachmentController.php | 18 +- .../Mail/InlineAttachmentController.php | 89 +++++++ src/Service/Mail/AttachmentResolver.php | 32 +++ src/Service/Mail/InlineAttachmentLinker.php | 85 +++++++ src/Service/Mail/InlineAttachmentSigner.php | 97 ++++++++ src/Service/Mail/MessageRenderer.php | 3 +- .../Mail/InlineAttachmentAccessTest.php | 220 ++++++++++++++++++ .../Mail/InlineAttachmentLinkerTest.php | 200 ++++++++++++++++ 10 files changed, 746 insertions(+), 14 deletions(-) create mode 100644 src/Controller/Mail/InlineAttachmentController.php create mode 100644 src/Service/Mail/InlineAttachmentLinker.php create mode 100644 src/Service/Mail/InlineAttachmentSigner.php create mode 100644 tests/Controller/Mail/InlineAttachmentAccessTest.php create mode 100644 tests/Service/Mail/InlineAttachmentLinkerTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 4648b450..159c4819 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/config/packages/security.yaml b/config/packages/security.yaml index f6d6f5f2..069f8c11 100644 --- a/config/packages/security.yaml +++ b/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: diff --git a/src/Controller/Mail/AttachmentController.php b/src/Controller/Mail/AttachmentController.php index 6d2d2d11..1eba5d5b 100644 --- a/src/Controller/Mail/AttachmentController.php +++ b/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 diff --git a/src/Controller/Mail/InlineAttachmentController.php b/src/Controller/Mail/InlineAttachmentController.php new file mode 100644 index 00000000..9857571e --- /dev/null +++ b/src/Controller/Mail/InlineAttachmentController.php @@ -0,0 +1,89 @@ + '\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; + } +} diff --git a/src/Service/Mail/AttachmentResolver.php b/src/Service/Mail/AttachmentResolver.php index 4b074bbe..f39344b1 100644 --- a/src/Service/Mail/AttachmentResolver.php +++ b/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 diff --git a/src/Service/Mail/InlineAttachmentLinker.php b/src/Service/Mail/InlineAttachmentLinker.php new file mode 100644 index 00000000..4df3c0ef --- /dev/null +++ b/src/Service/Mail/InlineAttachmentLinker.php @@ -0,0 +1,85 @@ +` 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 `"` 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*(?:["\']|"|&\#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, + ); + } +} diff --git a/src/Service/Mail/InlineAttachmentSigner.php b/src/Service/Mail/InlineAttachmentSigner.php new file mode 100644 index 00000000..aa25336b --- /dev/null +++ b/src/Service/Mail/InlineAttachmentSigner.php @@ -0,0 +1,97 @@ +` 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, + ); + } +} diff --git a/src/Service/Mail/MessageRenderer.php b/src/Service/Mail/MessageRenderer.php index 799d1639..0afda64b 100644 --- a/src/Service/Mail/MessageRenderer.php +++ b/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, diff --git a/tests/Controller/Mail/InlineAttachmentAccessTest.php b/tests/Controller/Mail/InlineAttachmentAccessTest.php new file mode 100644 index 00000000..41124512 --- /dev/null +++ b/tests/Controller/Mail/InlineAttachmentAccessTest.php @@ -0,0 +1,220 @@ +` 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', ''); + + $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', ''); + $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; + } +} diff --git a/tests/Service/Mail/InlineAttachmentLinkerTest.php b/tests/Service/Mail/InlineAttachmentLinkerTest.php new file mode 100644 index 00000000..212b3d6e --- /dev/null +++ b/tests/Service/Mail/InlineAttachmentLinkerTest.php @@ -0,0 +1,200 @@ +` 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('', $message); + + self::assertSame( + '', + $html, + ); + self::assertStringStartsWith(''; + + 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 = ''; + + 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('', $message); + + self::assertSame( + '', + $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 = 'logo.png'; + + self::assertSame($html, $this->linker->link($html, $message)); + } + + public function testOnlyTheBareImageUrlIsRewritten(): void + { + $message = $this->message([101 => 'image/png']); + + $html = 'ab'; + + 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('', $message); + self::assertStringContainsString('url(' . $signed . ')', (string) $html); + + $html = $this->linker->link('', $message); + self::assertStringContainsString('url("' . $signed . '")', (string) $html); + } + + public function testABodyWithoutInlineImagesComesBackAsItWas(): void + { + $message = $this->message([101 => 'image/png']); + + self::assertNull($this->linker->link(null, $message)); + self::assertSame('

hi

', $this->linker->link('

hi

', $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 $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; + } +}