Sanitize Twice: Why One Pass Isn't Enough for Rich-Text Email

TL;DR — I sanitize rich-text email bodies when they're saved. Today I added a second pass at render time, and it isn't belt-and-braces paranoia — it's an ordering bug. {{variable}} merge tags get interpolated after the save-time sanitization ran, which means contact-supplied text lands inside HTML that was already declared clean. Sanitize on save, and again on render.

---

The setup

A campaign builder. A rich-text editor (Trix, in this case) produces HTML. That HTML gets stored, then rendered into an email with merge tags swapped for real values:

Hi {{first_name}}, here's your update.

Standard stuff. And the standard advice is "sanitize on input" — so that's what was there:

$campaign->body = $this->sanitizer->sanitize($request->input('body'));
$campaign->save();

Clean HTML in the database. Job done.

Why one pass isn't enough

Look at the order of operations at send time:

1. Load the stored HTML — already sanitized. ✅
2. Replace {{first_name}} with the contact's actual first name. ⬅️ new bytes
3. Render and send.

Step 2 splices data into the document after the only sanitization pass ran. And where does first_name come from? A web form. An import. An API. Anywhere a contact can put text.

That's the whole bug. It's not that the sanitizer is weak — it's that it ran before the document was finished. Sanitizing "on input" is only sufficient when the thing you sanitized is the thing you output. The moment you have a templating step in between, "input" and "output" are two different documents.

So: sanitize when it's saved, and sanitize again just before it goes out.

/**
 * Sanitizes rich-text email bodies before they are persisted and again at
 * render time (defence in depth — {{variable}} interpolation can splice
 * contact-supplied text into the HTML after save-time sanitization).
 */

Worth noting why this stays safe: {{variable}} tokens live in text nodes, so running the sanitizer over the document doesn't disturb them. If your merge tags lived in attributes, you'd have a much less pleasant conversation with yourself.

The allow-list is smaller than you think

Symfony's HtmlSanitizer is allow-list based, which is the only sane default — a deny-list is a list of things you thought of.

$config = (new HtmlSanitizerConfig)
    ->allowElement('div')
    ->allowElement('p')
    ->allowElement('br')
    ->allowElement('strong')
    ->allowElement('em')
    ->allowElement('h1')
    ->allowElement('h2')
    ->allowElement('h3')
    ->allowElement('blockquote')
    ->allowElement('ul')
    ->allowElement('ol')
    ->allowElement('li')
    ->allowElement('figure', ['class'])
    ->allowElement('figcaption', ['class'])
    ->allowElement('a', ['href', 'title'])
    ->allowElement('img', ['src', 'alt', 'width', 'height'])
    ->allowLinkSchemes(['http', 'https', 'mailto'])
    ->allowMediaSchemes(['http', 'https'])
    ->withMaxInputLength(200_000);

Two things I like about how this landed:

The allow-list mirrors what the editor can actually produce. Not "all the HTML I can think of" — exactly the subset the editor emits. Anything outside that set couldn't have come from the editor, so there's no legitimate case to allow it.

That subset happens to be the same one email clients render reliably. Outlook is the strictest allow-list you'll ever meet. Constraining for security and constraining for deliverability point the same direction here, which almost never happens, so enjoy it.

And withMaxInputLength — set it. A sanitizer parses a DOM; an unbounded input is an unbounded parse.

The bug the sanitizer created

Here's the part I didn't see coming.

A Trix attachment whose upload hasn't finished (or has failed) serialises as an <img> with no src at all. And the allow-list, doing its job, strips blob: and data: srcs — producing exactly the same shape.

Email clients render a src-less <img> as a big empty frame. So the sanitizer was technically correct and visually terrible: recipients got a campaign with a grey rectangle where the image should be.

The fix is a post-pass:

public function sanitize(string $html): string
{
    return $this->stripBrokenAttachments(
        $this->sanitizer->sanitizeFor('div', $html)
    );
}

/**
 * Drop images that can never render, and any attachment <figure> left
 * holding no image at all.
 */
private function stripBrokenAttachments(string $html): string
{
    if (stripos($html, '<img') === false && stripos($html, '<figure') === false) {
        return $html;
    }

    // ... remove <img> with no src, then remove now-empty <figure> wrappers
}

The generalisable lesson: a sanitizer that leaves a husk is worse than one that removes the whole element. Stripping a dangerous attribute off a node often leaves a node that has no reason to exist. Decide what the element means without its stripped attribute — and if the answer is "nothing", take the element too. Same for its wrapper.

That early stripos guard is not premature optimisation either. Most campaign bodies are text; skipping a DOM pass on them is free.

Testing it

The tests I'd want here are less about "does it strip <script>" (Symfony's suite covers that) and more about my two claims: that interpolated text is sanitized, and that husks don't survive.

it('sanitizes interpolated values, not just the stored body', function () {
    $campaign = EmailCampaign::factory()->create([
        'body' => '<p>Hi {{first_name}}, welcome.</p>',
    ]);

    $contact = Contact::factory()->create([
        'first_name' => '<img src=x onerror=alert(1)>',
    ]);

    $html = app(CampaignRenderer::class)->render($campaign, $contact);

    expect($html)->not->toContain('onerror');
});

it('drops an image that can never render, and its empty figure', function () {
    $html = app(EmailHtmlSanitizer::class)
        ->sanitize('<figure class="attachment"><img alt="pending"></figure>');

    expect($html)->not->toContain('<img')
        ->and($html)->not->toContain('<figure');
});

it('leaves a plain text body untouched', function () {
    $body = '<p>Hi {{first_name}}, welcome.</p>';

    expect(app(EmailHtmlSanitizer::class)->sanitize($body))->toBe($body);
});

That third one matters more than it looks. A sanitizer that mangles ordinary content gets switched off by the next developer who hits it.

While I was in there

Two smaller things from the same day, both in the "the failure that looks like success" family:

Serving email images through a route instead of a storage symlink. A missing public/storage symlink is a deploy-shaped failure that shows up as broken images in someone's inbox — long after the deploy went green. Serving the asset through an ordinary controller route removes the symlink from the list of things that can be missing. One less environmental precondition is usually worth one more route.

A stale route cache. Production had booted from a cached routes file that the deploy pipeline never refreshed. New routes existed in code but not at runtime — and not in artisan processes either, which meant a deploy operation that depended on the new route failed too. The fix was a route:clear operation, timestamped to run before the one that needed it. The lesson: cached artefacts are part of your deploy contract. If nothing in the pipeline is responsible for invalidating them, they will eventually be wrong, and the symptom will point anywhere but at the cache.

Takeaway

"Sanitize on input" is shorthand for "sanitize the document you're about to trust". If a templating step runs after your sanitizer, the document you trusted isn't the document you sent.

Three rules I'm keeping:

1. Sanitize on save and on render. Save-time keeps the database clean; render-time is the one that's actually load-bearing.
2. Allow-list what your editor produces, not what HTML permits.
3. Don't leave husks. If stripping an attribute leaves an element with no meaning, remove the element — and whatever was wrapping it.