From e4e2f4af71a6597548d2e35d82a5dec6a4ed3a5c Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Thu, 20 Aug 2026 10:50:10 +0200 Subject: fix(pane): drop Save link from a link's context menu Reported by hand after the item 127 fix: right-clicking a link still offered Save link. It had been deferred to item 114 alongside Save image, on the grounds that both are inert without a downloadRequested handler. That is true and it was the wrong conclusion, because the two are not the same question. Save image is content the message already carries, and item 114 is about making it work. Save link fetches a remote URL chosen by the sender, through the pane's profile, which is the one profile in this application that must never fetch remote content: that is what m_allowRemote and the interceptor exist to prevent. Answering it with a download handler would put a network fetch of attacker-controlled content behind one context-menu entry. Saving what the user actually wants already has a path that never touches the network: saveAttachment(), which writes a MIME part already parsed into memory and sanitises the filename. So it is removed rather than implemented, and the test asserts its absence. Item 114 now carries the constraint that follows: a downloadRequested handler added to make Save image work must not make Save link reachable again, which the natural per-profile implementation would do by default. Mutation checked: dropping the entry from the filter fails the test with "a link action survived: Save link". Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01YDq53rMd3AQp7QmcZzpuBM --- src/messageview.cpp | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) (limited to 'src') diff --git a/src/messageview.cpp b/src/messageview.cpp index 3dbd0f1..469d148 100644 --- a/src/messageview.cpp +++ b/src/messageview.cpp @@ -825,6 +825,26 @@ void MessageView::removeBrowserActions(QMenu *menu, QWebEnginePage *page) QWebEnginePage::OpenLinkInNewTab, QWebEnginePage::OpenLinkInNewWindow, QWebEnginePage::OpenLinkInThisWindow, + // Save link, and this one is a SECURITY decision rather than tidying. + // + // It is inert today, since no downloadRequested handler exists + // anywhere, which is why it was first deferred to item 114 alongside + // Save image. That was wrong: the two are not the same question. + // + // Save image is content the message already carries, and item 114 is + // about making it work. Save link fetches a REMOTE URL chosen by the + // sender, through this pane's profile, which is the one profile in the + // application that must never fetch remote content: that is what + // m_allowRemote and the whole interceptor exist to prevent. Answering + // it with a download handler would put a network fetch of + // attacker-controlled content behind a single context-menu entry, and + // the request would carry whatever the profile holds. + // + // Saving what the user actually wants already has a path that does not + // touch the network: saveAttachment(), which writes a MIME part + // already parsed into memory and sanitises the filename. Do not + // "restore" this entry by implementing downloadRequested for it. + QWebEnginePage::DownloadLinkToDisk, }; for (const QWebEnginePage::WebAction which : kUnwanted) { -- cgit v1.2.3