summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-20 10:50:10 +0200
committerDanilo M. <danix@danix.xyz>2026-08-20 10:50:10 +0200
commite4e2f4af71a6597548d2e35d82a5dec6a4ed3a5c (patch)
treefacb5bfaa35b587ff9abbb12d95d6e88fde0d111
parent3f256acda192fbe2b9fad28d09cd74dca9a69418 (diff)
downloadqtmaildir-e4e2f4af71a6597548d2e35d82a5dec6a4ed3a5c.tar.gz
qtmaildir-e4e2f4af71a6597548d2e35d82a5dec6a4ed3a5c.zip
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YDq53rMd3AQp7QmcZzpuBM
-rw-r--r--CHANGELOG.md10
-rw-r--r--docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md11
-rw-r--r--docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md16
-rw-r--r--src/messageview.cpp20
-rw-r--r--tests/test_messageview.cpp6
5 files changed, 58 insertions, 5 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9131c25..3f0748a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -20,9 +20,13 @@ point at which they are stable.
was discarded. Plain links, as in most text mail, were unaffected and already
worked, which is what made this look like "HTML mail is broken".
- The context menu on a link no longer offers Open in new tab, Open in new
- window or Open in this window. None can work: the pane has no tabs and must
- never open a window or navigate away from the message. Copy link address is
- kept.
+ window, Open in this window or Save link. The first three cannot work: the
+ pane has no tabs and must never open a window or navigate away from the
+ message. Save link is removed for a stronger reason: it would fetch a
+ sender-chosen remote URL through the message pane, which never fetches remote
+ content by design. Copy link address is kept, and attachments are still saved
+ from the attachment bar, which reads what the message already carries rather
+ than the network.
## [0.26.1] - 2026-08-19
diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md
index 4f361bf..a4aae42 100644
--- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md
+++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md
@@ -5286,8 +5286,15 @@ returning `nullptr` discards it before it can be read. Routing it through the
same handler the plain-link path uses is what keeps the two kinds of link from
drifting apart.
-127: three actions added to `removeBrowserActions()`'s list,
-`CopyLinkToClipboard` deliberately left. The order mattered: 126 gives the page
+127: four actions added to `removeBrowserActions()`'s list,
+`CopyLinkToClipboard` deliberately left. The fourth, `DownloadLinkToDisk`, was
+added after a hand test: it had been deferred to item 114 alongside Save image
+on the grounds that both are inert without a `downloadRequested` handler. True,
+and the wrong conclusion. Save image is content the message already carries;
+Save link fetches a sender-chosen remote URL through the one profile that must
+never fetch remote content, so it is removed rather than implemented, and item
+114 now carries the constraint that a handler for the former must not revive the
+latter. The order mattered: 126 gives the page
a real `createWindow()`, so those entries would have stopped being merely dead
and started opening links into a tab that does not exist.
diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md
index 3848a6d..d7783f7 100644
--- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md
+++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md
@@ -730,6 +730,22 @@ for the network to satisfy it.
**Size: S.**
+**Save LINK is no longer part of this item** (2026-08-20, item 127). It was
+deferred here on the grounds that both are inert for want of a
+`downloadRequested` handler, which is true and beside the point: they are not
+the same question.
+
+Save image is content the message already carries, and making it work is what
+this item is about. Save link fetches a REMOTE URL chosen by the sender,
+through the pane's profile, which is the one profile in the application that
+must never fetch remote content. It is removed from the menu rather than
+implemented, and `theLinkMenuDropsTheOpenInWindowActions` asserts its absence.
+
+**That assertion constrains this item.** A `downloadRequested` handler added to
+make Save image work must not make Save link reachable again. The test fails if
+it does, which is the point: the handler is per-profile, so the natural
+implementation would light up both entries at once.
+
## 118. No way to empty the trash from inside the app
**Observed (user, 2026-08-17):** raised while reviewing item 103's spec, as
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) {
diff --git a/tests/test_messageview.cpp b/tests/test_messageview.cpp
index c3e21ed..c0c9bc2 100644
--- a/tests/test_messageview.cpp
+++ b/tests/test_messageview.cpp
@@ -1129,6 +1129,12 @@ void TestMessageView::theLinkMenuDropsTheOpenInWindowActions()
QWebEnginePage::OpenLinkInNewTab,
QWebEnginePage::OpenLinkInNewWindow,
QWebEnginePage::OpenLinkInThisWindow,
+ // Save link. Asserted here rather than left to item 114, because it is
+ // not the same question as Save image: it fetches a sender-chosen
+ // remote URL through the one profile that must never fetch remote
+ // content. A download handler added for Save image must NOT make this
+ // reachable again, and this assertion is what would catch that.
+ QWebEnginePage::DownloadLinkToDisk,
};
const QList<QWebEnginePage::WebAction> wanted = {
QWebEnginePage::CopyLinkToClipboard,