aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-03 09:09:43 +0200
committerDanilo M. <danix@danix.xyz>2026-08-03 09:09:43 +0200
commite7dceee1f449cafd07a86dfd11ff77f35897bdcf (patch)
treeb990c85490e8eea3b5f269ff8afdf7a7c3495da4
parent255662101a4370197da6a1e64a7a446fd9a80187 (diff)
downloadqtmaildir-e7dceee1f449cafd07a86dfd11ff77f35897bdcf.tar.gz
qtmaildir-e7dceee1f449cafd07a86dfd11ff77f35897bdcf.zip
feat: add MessageView with locked-down web engine profile
Off-the-record profile, JavaScript off, deny-by-default interceptor, and a page subclass that hands link clicks to the system browser so a message can never navigate the pane. Honours the obligation task 5 recorded: the interceptor trusts exactly one qtmaildir: URL and fails closed otherwise, so setHtml() and setDocumentUrl() must agree or the pane renders nothing. Rather than pairing those calls at each site, every load goes through one setDocument() and the URL comes from a single documentUrl() accessor. Verified against the real interceptor that this URL is allowed while siblings, subpaths, remote and file: are not. Three fixes against the drafted version: - showError() called setHtml() with a base URL but never setDocumentUrl(), so an error card would have rendered blank. Now impossible to repeat. - clear() and showError() left the previous thread's inline parts in the scheme handler and its cids in the interceptor. Both now empty the policy, so no thread's parts outlive it. - MessagePage trusted the whole qtmaildir: scheme for typed navigations, which is the same blanket-trust mistake task 5 removed from the interceptor. It now matches the exact document URL. The parts-flattening is extracted into buildThreadCidMap() so it can be tested without a live profile, and a cidPrefix containing '!' is sanitized rather than trusted, since Q_ASSERT is compiled out in release and this map decides which bytes a message can name. The sanitizer escapes '_' before replacing '!', because a plain replace would map "m0!x" and "m0_x" onto one key and merge two messages, which is the very collision the namespacing exists to prevent. Mutation-verified: the naive replace fails the distinctness test, and dropping the sanitizer trips the assert. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
-rw-r--r--src/CMakeLists.txt2
-rw-r--r--src/messageview.cpp227
-rw-r--r--src/messageview.h66
-rw-r--r--src/threadcidmap.cpp51
-rw-r--r--src/threadcidmap.h29
-rw-r--r--tests/CMakeLists.txt1
-rw-r--r--tests/test_threadcidmap.cpp148
7 files changed, 524 insertions, 0 deletions
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 6a2b18e..0073870 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -8,6 +8,8 @@ add_library(qtmaildir_lib STATIC
notmuchworker.cpp
threadlistmodel.cpp
mailsync.cpp
+ threadcidmap.cpp
+ messageview.cpp
)
target_include_directories(qtmaildir_lib
diff --git a/src/messageview.cpp b/src/messageview.cpp
new file mode 100644
index 0000000..4e7586e
--- /dev/null
+++ b/src/messageview.cpp
@@ -0,0 +1,227 @@
+#include "messageview.h"
+
+#include <QDesktopServices>
+#include <QHBoxLayout>
+#include <QLabel>
+#include <QPushButton>
+#include <QTimer>
+#include <QVBoxLayout>
+#include <QWebEnginePage>
+#include <QWebEngineProfile>
+#include <QWebEngineSettings>
+#include <QWebEngineView>
+
+#include <algorithm>
+
+#include "cidschemehandler.h"
+#include "htmlbuilder.h"
+#include "requestinterceptor.h"
+#include "threadcidmap.h"
+
+namespace {
+
+/// Intercepts link clicks so a message can never navigate the pane.
+class MessagePage : public QWebEnginePage
+{
+public:
+ MessagePage(QWebEngineProfile *profile, QObject *parent)
+ : QWebEnginePage(profile, parent) {}
+
+protected:
+ bool acceptNavigationRequest(const QUrl &url, NavigationType type,
+ bool isMainFrame) override
+ {
+ // setHtml() arrives as a typed navigation to our own base URL. Matching
+ // the exact URL rather than the scheme keeps this consistent with the
+ // interceptor, which deliberately refuses to trust qtmaildir: wholesale.
+ if (type == NavigationTypeTyped && url == MessageView::documentUrl())
+ return true;
+
+ if (type == NavigationTypeLinkClicked) {
+ QDesktopServices::openUrl(url);
+ return false;
+ }
+
+ // Subframe loads are still subject to the interceptor; a main-frame
+ // navigation would replace the pane, which no message may do.
+ return !isMainFrame;
+ }
+};
+
+} // namespace
+
+MessageView::MessageView(QWidget *parent)
+ : QWidget(parent)
+{
+ // Off-the-record profile: no cookies, no cache, nothing persisted.
+ m_profile = new QWebEngineProfile(this);
+ m_profile->setHttpCacheType(QWebEngineProfile::NoCache);
+ m_profile->setPersistentCookiesPolicy(QWebEngineProfile::NoPersistentCookies);
+
+ m_interceptor = new RequestInterceptor(this);
+ m_profile->setUrlRequestInterceptor(m_interceptor);
+
+ m_cidHandler = new CidSchemeHandler(this);
+ m_profile->installUrlSchemeHandler(QByteArrayLiteral("cid"), m_cidHandler);
+
+ m_view = new QWebEngineView(this);
+ m_view->setPage(new MessagePage(m_profile, m_view));
+
+ QWebEngineSettings *settings = m_view->settings();
+ settings->setAttribute(QWebEngineSettings::JavascriptEnabled, false);
+ settings->setAttribute(QWebEngineSettings::LocalContentCanAccessRemoteUrls, false);
+ settings->setAttribute(QWebEngineSettings::LocalContentCanAccessFileUrls, false);
+ settings->setAttribute(QWebEngineSettings::PluginsEnabled, false);
+ settings->setAttribute(QWebEngineSettings::FullScreenSupportEnabled, false);
+
+ m_headerLabel = new QLabel(this);
+ m_headerLabel->setTextFormat(Qt::RichText);
+ m_headerLabel->setWordWrap(true);
+ m_headerLabel->setTextInteractionFlags(Qt::TextSelectableByMouse);
+
+ m_blockedLabel = new QLabel(tr("Remote content blocked"), this);
+ m_loadRemoteButton = new QPushButton(tr("Load remote content"), this);
+ connect(m_loadRemoteButton, &QPushButton::clicked,
+ this, &MessageView::loadRemoteContent);
+
+ auto *blockedRow = new QHBoxLayout;
+ blockedRow->addWidget(m_blockedLabel);
+ blockedRow->addWidget(m_loadRemoteButton);
+ blockedRow->addStretch();
+
+ m_attachmentBar = new QWidget(this);
+ new QHBoxLayout(m_attachmentBar);
+
+ auto *layout = new QVBoxLayout(this);
+ layout->addWidget(m_headerLabel);
+ layout->addLayout(blockedRow);
+ layout->addWidget(m_view, 1);
+ layout->addWidget(m_attachmentBar);
+
+ clear();
+}
+
+MessageView::~MessageView() = default;
+
+/// The single place that loads a document into the view.
+///
+/// RequestInterceptor trusts exactly one qtmaildir: URL and denies every other
+/// URL on that scheme, so the base URL given to setHtml() and the one given to
+/// setDocumentUrl() must be identical. Routing every load through here is what
+/// makes that true by construction rather than by remembering to pair two calls
+/// at each site.
+void MessageView::setDocument(const QString &html)
+{
+ m_interceptor->setDocumentUrl(documentUrl());
+ m_view->setHtml(html, documentUrl());
+}
+
+void MessageView::clear()
+{
+ m_items.clear();
+
+ // No thread is displayed, so nothing may be served or allowed. Without
+ // this, the previous thread's parts would stay reachable.
+ m_cidHandler->setParts({});
+ m_interceptor->setAllowedCids({});
+ m_interceptor->resetForNewMessage();
+
+ setDocument(QString());
+ m_headerLabel->clear();
+ m_blockedLabel->hide();
+ m_loadRemoteButton->hide();
+}
+
+void MessageView::showThread(const QList<ThreadRenderItem> &items)
+{
+ m_items = items;
+ m_preferHtml = true;
+
+ // Every thread starts from a clean policy: no remote grant carries over.
+ m_interceptor->resetForNewMessage();
+
+ // Two messages in one thread commonly share a Content-ID, and the thread is
+ // one document, so the parts are namespaced per message.
+ const ThreadCidMap cidMap = buildThreadCidMap(m_items);
+ m_interceptor->setAllowedCids(cidMap.allowedCids);
+ m_cidHandler->setParts(cidMap.parts);
+
+ updateHeader();
+ render();
+}
+
+void MessageView::showError(const QString &text, const QString &filePath)
+{
+ m_items.clear();
+
+ // An error card references nothing, so the policy is emptied rather than
+ // left holding the previous thread's parts.
+ m_cidHandler->setParts({});
+ m_interceptor->setAllowedCids({});
+ m_interceptor->resetForNewMessage();
+
+ m_headerLabel->setText(tr("<b>Cannot display message</b>"));
+ m_blockedLabel->hide();
+ m_loadRemoteButton->hide();
+
+ const QString html = QStringLiteral(
+ "<html><body><p>%1</p><p><code>%2</code></p></body></html>")
+ .arg(text.toHtmlEscaped(), filePath.toHtmlEscaped());
+ setDocument(html);
+}
+
+void MessageView::updateHeader()
+{
+ if (m_items.isEmpty()) {
+ m_headerLabel->clear();
+ return;
+ }
+
+ // The thread's subject comes from its first message; later replies carry
+ // Re: prefixes that add nothing.
+ const QString subject = m_items.first().message.subject;
+
+ m_headerLabel->setText(
+ QStringLiteral("<b>%1</b><br><small>%2</small>")
+ .arg(subject.toHtmlEscaped(),
+ tr("%n message(s) in thread", "", m_items.size())));
+}
+
+void MessageView::render()
+{
+ const HtmlBuilder::Mode mode =
+ m_preferHtml ? HtmlBuilder::PreferHtml : HtmlBuilder::ForcePlain;
+
+ setDocument(HtmlBuilder::buildThread(m_items, mode));
+
+ // Blocking is discovered during load, so check shortly afterwards.
+ QTimer::singleShot(300, this, [this]() {
+ const bool blocked = m_interceptor->blockedAnything()
+ && !m_interceptor->allowRemote();
+ m_blockedLabel->setVisible(blocked);
+ m_loadRemoteButton->setVisible(blocked);
+ });
+}
+
+void MessageView::toggleHtml()
+{
+ const bool anyHtml = std::any_of(
+ m_items.cbegin(), m_items.cend(),
+ [](const ThreadRenderItem &item) { return item.message.hasHtml(); });
+
+ if (!anyHtml) {
+ emit statusMessage(tr("No message in this thread has an HTML part"));
+ return;
+ }
+ m_preferHtml = !m_preferHtml;
+ render();
+}
+
+void MessageView::loadRemoteContent()
+{
+ // Applies to this thread only and is cleared by the next showThread().
+ m_interceptor->setAllowRemote(true);
+ m_blockedLabel->hide();
+ m_loadRemoteButton->hide();
+ render();
+}
diff --git a/src/messageview.h b/src/messageview.h
new file mode 100644
index 0000000..9f2edb3
--- /dev/null
+++ b/src/messageview.h
@@ -0,0 +1,66 @@
+#pragma once
+
+#include <QList>
+#include <QUrl>
+#include <QWidget>
+
+#include "htmlbuilder.h"
+#include "mimeparser.h"
+
+class QLabel;
+class QPushButton;
+class QWebEngineView;
+class QWebEngineProfile;
+class CidSchemeHandler;
+class RequestInterceptor;
+
+/// The message pane: thread header, body, attachment bar.
+///
+/// A whole thread renders into one web view. A newsletter thread can hold
+/// dozens of messages, and one view per message would spawn one Chromium
+/// render process per message.
+class MessageView : public QWidget
+{
+ Q_OBJECT
+public:
+ explicit MessageView(QWidget *parent = nullptr);
+ ~MessageView() override;
+
+ /// The base URL every document in this pane is loaded with, and the only
+ /// qtmaildir: URL the interceptor trusts. Defined once so setHtml() and
+ /// setDocumentUrl() cannot drift apart: if they ever disagree, the
+ /// interceptor fails closed and the pane renders nothing at all.
+ static QUrl documentUrl() { return QUrl(QStringLiteral("qtmaildir://message")); }
+
+ /// Renders a whole thread, oldest first. Items whose expanded flag is
+ /// false collapse to a one-line stub.
+ void showThread(const QList<ThreadRenderItem> &items);
+
+ void showError(const QString &text, const QString &filePath);
+ void clear();
+
+public slots:
+ void toggleHtml();
+ void loadRemoteContent();
+
+signals:
+ void statusMessage(const QString &text);
+
+private:
+ void render();
+ void updateHeader();
+ void setDocument(const QString &html);
+
+ QList<ThreadRenderItem> m_items;
+ bool m_preferHtml = true;
+
+ QWebEngineProfile *m_profile = nullptr;
+ QWebEngineView *m_view = nullptr;
+ RequestInterceptor *m_interceptor = nullptr;
+ CidSchemeHandler *m_cidHandler = nullptr;
+
+ QLabel *m_headerLabel = nullptr;
+ QLabel *m_blockedLabel = nullptr;
+ QPushButton *m_loadRemoteButton = nullptr;
+ QWidget *m_attachmentBar = nullptr;
+};
diff --git a/src/threadcidmap.cpp b/src/threadcidmap.cpp
new file mode 100644
index 0000000..c6f1cc7
--- /dev/null
+++ b/src/threadcidmap.cpp
@@ -0,0 +1,51 @@
+#include "threadcidmap.h"
+
+#include "cidschemehandler.h"
+
+namespace {
+
+/// Removes '!' from a prefix without ever mapping two distinct prefixes onto
+/// one another.
+///
+/// A plain replace of '!' with '_' is NOT safe here: "m0!x" and "m0_x" would
+/// both become "m0_x", merging two messages into one key space, which is
+/// precisely the collision the namespacing exists to prevent. Escaping the
+/// escape character first keeps the transform injective: '_' doubles, and '!'
+/// becomes "_x", so no output is reachable from two different inputs.
+QString sanitizePrefix(const QString &prefix)
+{
+ if (!prefix.contains(QLatin1Char('!')) && !prefix.contains(QLatin1Char('_')))
+ return prefix;
+
+ QString result;
+ result.reserve(prefix.size() + 4);
+ for (const QChar c : prefix) {
+ if (c == QLatin1Char('_'))
+ result += QLatin1String("__");
+ else if (c == QLatin1Char('!'))
+ result += QLatin1String("_x");
+ else
+ result += c;
+ }
+ return result;
+}
+
+} // namespace
+
+ThreadCidMap buildThreadCidMap(const QList<ThreadRenderItem> &items)
+{
+ ThreadCidMap map;
+
+ for (const ThreadRenderItem &item : items) {
+ const QString prefix = sanitizePrefix(item.cidPrefix);
+
+ for (auto it = item.message.inlineParts.cbegin();
+ it != item.message.inlineParts.cend(); ++it) {
+ const QString key = CidSchemeHandler::namespacedKey(prefix, it.key());
+ map.parts.insert(key, it.value());
+ map.allowedCids.insert(key);
+ }
+ }
+
+ return map;
+}
diff --git a/src/threadcidmap.h b/src/threadcidmap.h
new file mode 100644
index 0000000..76bd8b9
--- /dev/null
+++ b/src/threadcidmap.h
@@ -0,0 +1,29 @@
+#pragma once
+
+#include <QHash>
+#include <QList>
+#include <QSet>
+#include <QString>
+
+#include "htmlbuilder.h"
+#include "mimeparser.h"
+
+/// The inline parts of a whole thread, keyed by their namespaced cid.
+struct ThreadCidMap
+{
+ QHash<QString, InlinePart> parts; ///< For the scheme handler.
+ QSet<QString> allowedCids; ///< For the interceptor. Same keys.
+};
+
+/// Flattens every message's inline parts into one namespaced map.
+///
+/// Two messages in a thread commonly share a Content-ID (cid:logo@example.org
+/// from the same sender's newsletter template), and the thread renders as one
+/// document, so the raw ids would collide and one message would show another's
+/// image. Keys are "<prefix>!<content-id>".
+///
+/// A cidPrefix containing '!' would break the split that keeps those apart, so
+/// it is sanitized here rather than trusted: Q_ASSERT fires in debug builds but
+/// is compiled out in release, and this map decides which bytes a message can
+/// name. Sanitizing is injective, so two distinct prefixes stay distinct.
+ThreadCidMap buildThreadCidMap(const QList<ThreadRenderItem> &items);
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index c2bfc8a..d92bc6a 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -15,3 +15,4 @@ add_qtmaildir_test(htmlbuilder)
add_qtmaildir_test(notmuchworker)
add_qtmaildir_test(threadlistmodel)
add_qtmaildir_test(mailsync)
+add_qtmaildir_test(threadcidmap)
diff --git a/tests/test_threadcidmap.cpp b/tests/test_threadcidmap.cpp
new file mode 100644
index 0000000..6087e5d
--- /dev/null
+++ b/tests/test_threadcidmap.cpp
@@ -0,0 +1,148 @@
+#include <QtTest>
+
+#include "threadcidmap.h"
+
+/// The one piece of new security-relevant logic in the message pane: flattening
+/// every message's inline parts into a single namespaced map. Two messages in a
+/// thread commonly share a Content-ID, and getting this wrong shows one
+/// message's image inside another.
+class TestThreadCidMap : public QObject
+{
+ Q_OBJECT
+private slots:
+ void emptyThreadYieldsEmptyMap();
+ void singleMessagePartsAreNamespaced();
+ void sharedContentIdsDoNotCollide();
+ void allowedCidsMatchMapKeys();
+ void prefixWithBangIsSanitized();
+ void sanitizedPrefixesStayDistinct();
+ void hostileContentIdCannotForgeAnotherPrefix();
+ void messagesWithoutInlinePartsAreSkipped();
+};
+
+static ThreadRenderItem makeItem(const QString &prefix,
+ const QStringList &contentIds)
+{
+ ThreadRenderItem item;
+ item.cidPrefix = prefix;
+ for (const QString &id : contentIds) {
+ InlinePart part;
+ part.mimeType = QStringLiteral("image/png");
+ part.data = id.toUtf8(); // Stand-in payload, unique per id.
+ item.message.inlineParts.insert(id, part);
+ }
+ return item;
+}
+
+void TestThreadCidMap::emptyThreadYieldsEmptyMap()
+{
+ const ThreadCidMap map = buildThreadCidMap({});
+ QVERIFY(map.parts.isEmpty());
+ QVERIFY(map.allowedCids.isEmpty());
+}
+
+void TestThreadCidMap::singleMessagePartsAreNamespaced()
+{
+ const ThreadCidMap map = buildThreadCidMap(
+ { makeItem(QStringLiteral("m0"), { QStringLiteral("logo@example.org") }) });
+
+ QCOMPARE(map.parts.size(), 1);
+ QVERIFY(map.parts.contains(QStringLiteral("m0!logo@example.org")));
+ // The raw, un-namespaced id must not be servable: HtmlBuilder rewrites
+ // every reference, so a request for the bare id is a forged one.
+ QVERIFY(!map.parts.contains(QStringLiteral("logo@example.org")));
+}
+
+void TestThreadCidMap::sharedContentIdsDoNotCollide()
+{
+ // The case the namespacing exists for: two newsletters using cid:logo.
+ const ThreadCidMap map = buildThreadCidMap({
+ makeItem(QStringLiteral("m0"), { QStringLiteral("logo@example.org") }),
+ makeItem(QStringLiteral("m1"), { QStringLiteral("logo@example.org") }),
+ });
+
+ QCOMPARE(map.parts.size(), 2);
+ const InlinePart first = map.parts.value(QStringLiteral("m0!logo@example.org"));
+ const InlinePart second = map.parts.value(QStringLiteral("m1!logo@example.org"));
+ QCOMPARE(first.data, second.data); // Same stand-in payload by construction,
+ QVERIFY(map.parts.contains(QStringLiteral("m0!logo@example.org")));
+ QVERIFY(map.parts.contains(QStringLiteral("m1!logo@example.org")));
+}
+
+void TestThreadCidMap::allowedCidsMatchMapKeys()
+{
+ // The interceptor allows a set of cids; the handler serves a map. If those
+ // disagree, either an image 404s or one is servable that policy never
+ // approved.
+ const ThreadCidMap map = buildThreadCidMap({
+ makeItem(QStringLiteral("m0"), { QStringLiteral("a@x"), QStringLiteral("b@x") }),
+ makeItem(QStringLiteral("m1"), { QStringLiteral("a@x") }),
+ });
+
+ QCOMPARE(map.allowedCids.size(), map.parts.size());
+ for (const QString &key : map.parts.keys())
+ QVERIFY(map.allowedCids.contains(key));
+}
+
+void TestThreadCidMap::prefixWithBangIsSanitized()
+{
+ // Q_ASSERT is compiled out in release, so a malformed prefix from the
+ // caller must degrade safely rather than corrupt the key space.
+ const ThreadCidMap map = buildThreadCidMap(
+ { makeItem(QStringLiteral("m0!evil"), { QStringLiteral("logo@x") }) });
+
+ QCOMPARE(map.parts.size(), 1);
+ const QString key = map.parts.keys().first();
+
+ // Whatever the sanitizer produces, the invariant is that the key splits at
+ // its FIRST '!' back to a prefix that itself contains no '!'.
+ const int separator = key.indexOf(QLatin1Char('!'));
+ QVERIFY(separator > 0);
+ QVERIFY(!key.left(separator).contains(QLatin1Char('!')));
+}
+
+void TestThreadCidMap::sanitizedPrefixesStayDistinct()
+{
+ // Sanitizing must not merge two different messages into one key space: if
+ // "a!b" and "a_b" both became "a_b", one message's image would resolve for
+ // the other, which is the exact bug namespacing prevents.
+ const ThreadCidMap map = buildThreadCidMap({
+ makeItem(QStringLiteral("m0!x"), { QStringLiteral("logo@x") }),
+ makeItem(QStringLiteral("m0_x"), { QStringLiteral("logo@x") }),
+ });
+
+ QCOMPARE(map.parts.size(), 2);
+}
+
+void TestThreadCidMap::hostileContentIdCannotForgeAnotherPrefix()
+{
+ // The id half is attacker-controlled and may contain '!'. A message with
+ // prefix m0 must not be able to name a key belonging to m1.
+ const ThreadCidMap map = buildThreadCidMap({
+ makeItem(QStringLiteral("m0"), { QStringLiteral("m1!logo@x") }),
+ makeItem(QStringLiteral("m1"), { QStringLiteral("logo@x") }),
+ });
+
+ QCOMPARE(map.parts.size(), 2);
+ QVERIFY(map.parts.contains(QStringLiteral("m0!m1!logo@x")));
+ QVERIFY(map.parts.contains(QStringLiteral("m1!logo@x")));
+
+ // Splitting at the first '!' is what keeps these apart.
+ const QString forged = QStringLiteral("m0!m1!logo@x");
+ QCOMPARE(forged.left(forged.indexOf(QLatin1Char('!'))), QStringLiteral("m0"));
+}
+
+void TestThreadCidMap::messagesWithoutInlinePartsAreSkipped()
+{
+ const ThreadCidMap map = buildThreadCidMap({
+ makeItem(QStringLiteral("m0"), {}),
+ makeItem(QStringLiteral("m1"), { QStringLiteral("logo@x") }),
+ makeItem(QStringLiteral("m2"), {}),
+ });
+
+ QCOMPARE(map.parts.size(), 1);
+ QVERIFY(map.parts.contains(QStringLiteral("m1!logo@x")));
+}
+
+QTEST_MAIN(TestThreadCidMap)
+#include "test_threadcidmap.moc"