Skip to content

fix: use HMAC for proxied HTML iframe content#12735

Merged
ChristophWurst merged 1 commit intomainfrom
fix/iframe-hmac
Apr 15, 2026
Merged

fix: use HMAC for proxied HTML iframe content#12735
ChristophWurst merged 1 commit intomainfrom
fix/iframe-hmac

Conversation

@ChristophWurst
Copy link
Copy Markdown
Member

@ChristophWurst ChristophWurst commented Apr 15, 2026

This scopes the proxy to a specific email and URL.

Most changes are about injecting service and data. The main changes are trivial.

AI-Assisted: OpenCode (Claude Haiku 4.5)

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the /proxy endpoint used for loading external content from HTML emails by binding proxied URLs to a specific message id via an HMAC, and ensuring the current user actually owns the referenced message.

Changes:

  • Introduce ProxyHmacGenerator and integrate it into HTML sanitization so proxied URLs include id, hmac, and src.
  • Update TransformURLScheme to generate proxied URLs with HMAC instead of relying on request tokens.
  • Update ProxyController::proxy() to validate message ownership and HMAC before proxying, and expand/adjust unit tests accordingly.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
lib/Html/ProxyHmacGenerator.php New helper to generate per-message HMACs for proxied URLs.
lib/Service/Html.php Injects HMAC generator and passes message id into URL rewriting filter.
lib/Service/HtmlPurify/TransformURLScheme.php Rewrites external URLs to /proxy with id/hmac/src query params and updates CID attachment URL building.
lib/Controller/ProxyController.php Validates id/hmac and message ownership before proxying; returns 400/401 on invalid requests.
lib/Model/IMAPMessage.php Updates call site to new sanitizeHtmlMailBody(..., int $id, ...) signature.
tests/Unit/Html/ProxyHmacGeneratorTest.php Adds unit tests for HMAC generation behavior.
tests/Unit/Controller/ProxyControllerTest.php Updates controller construction/signature and adds tests for invalid/missing HMAC and ownership checks.
tests/Unit/Service/HtmlPurify/TransformURLSchemeTest.php Updates expectations to include id/hmac/src in proxied URLs and new constructor signature.
tests/Unit/Service/HtmlTest.php Updates Html service construction to include the new dependency.
tests/Unit/Model/IMAPMessageTest.php Updates Html service construction to include the new dependency.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/Controller/ProxyController.php Outdated
Comment thread tests/Unit/Html/ProxyHmacGeneratorTest.php Outdated
AI-Assisted: OpenCode (Claude Haiku 4.5)
Signed-off-by: Christoph Wurst <1374172+ChristophWurst@users.noreply.github.com>
@ChristophWurst
Copy link
Copy Markdown
Member Author

/backport to stable5.7

@ChristophWurst
Copy link
Copy Markdown
Member Author

/backport to stable5.6

@ChristophWurst ChristophWurst merged commit fffd17f into main Apr 15, 2026
56 of 58 checks passed
@ChristophWurst ChristophWurst deleted the fix/iframe-hmac branch April 15, 2026 09:36
@backportbot

This comment was marked as resolved.

@backportbot

This comment was marked as resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants