From 2baf2d4e2c1d8d059a0795bf46596c64c28e01e5 Mon Sep 17 00:00:00 2001
From: "Danilo M." <danix@danix.xyz>
Date: Thu, 20 Aug 2026 18:19:31 +0200
Subject: fix(compose): document what actually suppresses raw HTML, item 123

CMARK_OPT_SAFE has had no effect since cmark-gfm made safe mode the
default; the flag is retained for API compatibility and the real
protection is that CMARK_OPT_UNSAFE is never set. Measured against
0.29.0.gfm.13: rendering with OPT_DEFAULT alone, with OPT_SAFE, and with
OPT_UNSAFE shows the first two suppress a script element and a
javascript: link while the third leaks both.

The comment credited the flag, which would have sent the next reader to
the wrong place, and the test could not tell the two apart: it would
have passed just as well with the flag deleted. What it has to guard
against is OPT_UNSAFE being introduced, so it now also asserts that
unsafe links are stripped, which is a protection this gets for free and
previously asserted nothing about.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015muoUo2GdxmBDSp5vjYcbE
---
 src/markdownrenderer.cpp        | 21 +++++++++++++++++----
 tests/test_markdownrenderer.cpp | 29 ++++++++++++++++++++++++++---
 2 files changed, 43 insertions(+), 7 deletions(-)

diff --git a/src/markdownrenderer.cpp b/src/markdownrenderer.cpp
index a9b8177..ccb3309 100644
--- a/src/markdownrenderer.cpp
+++ b/src/markdownrenderer.cpp
@@ -34,8 +34,8 @@ namespace {
 ///
 /// `table` is absent deliberately, not by oversight: tables render badly
 /// across mail clients regardless of who generates them. `tagfilter` is absent
-/// because CMARK_OPT_SAFE already suppresses raw HTML wholesale, which is the
-/// stronger measure.
+/// because safe mode (see below) already suppresses raw HTML wholesale, which
+/// is the stronger measure.
 const char *const kExtensions[] = { "autolink", "strikethrough", "tasklist" };
 
 }  // namespace
@@ -51,8 +51,21 @@ QString MarkdownRenderer::toHtml(const QString &markdown)
     // after the first call.
     cmark_gfm_core_extensions_ensure_registered();
 
-    // SAFE suppresses raw HTML in the INPUT. It does not escape the output,
-    // which is markup by definition.
+    // CMARK_OPT_DEFAULT is 0, and CMARK_OPT_SAFE is a NO-OP in cmark-gfm 0.29:
+    // safe mode has been the default since that release, and the flag is kept
+    // only for API compatibility with code written against older versions.
+    // The real requirement is that CMARK_OPT_UNSAFE must never be set. Under
+    // safe mode a raw <script> block is replaced with an HTML comment
+    // placeholder, and a link whose scheme is not in the allowed set
+    // (javascript:, vbscript:, file:, and data: except a few safe image
+    // types) is replaced with an empty href. Measured against
+    // cmark-gfm-0.29.0.gfm.13 on 2026-08-20: rendering the same script tag and
+    // a javascript: link under OPT_DEFAULT alone, under OPT_DEFAULT|OPT_SAFE,
+    // and under OPT_UNSAFE shows the first two behave identically and
+    // suppress both, while OPT_UNSAFE leaks both verbatim into the output.
+    // OPT_SAFE is kept anyway, both as a statement of intent and in case a
+    // future cmark-gfm release makes it meaningful again; do not read its
+    // presence as the mechanism actually doing the suppressing.
     const int options = CMARK_OPT_DEFAULT | CMARK_OPT_SAFE;
 
     cmark_parser *parser = cmark_parser_new(options);
diff --git a/tests/test_markdownrenderer.cpp b/tests/test_markdownrenderer.cpp
index 44f3ccb..0c35b6c 100644
--- a/tests/test_markdownrenderer.cpp
+++ b/tests/test_markdownrenderer.cpp
@@ -35,6 +35,7 @@ private slots:
     void tasklistRenders();
     void tablesAreNotEnabled();
     void rawHtmlIsSuppressed();
+    void unsafeLinksAreStripped();
     void accentedTextSurvivesAsUtf8();
     void emptyInputProducesEmptyOutput();
 };
@@ -90,15 +91,37 @@ void TestMarkdownRenderer::tablesAreNotEnabled()
 
 void TestMarkdownRenderer::rawHtmlIsSuppressed()
 {
-    // CMARK_OPT_SAFE. The body is the user's own text, but a body that can
-    // inject markup into its own generated HTML part is a sharp edge with no
-    // upside.
+    // Safe mode (the cmark-gfm 0.29 default, not CMARK_OPT_SAFE, which is a
+    // no-op in this version, see markdownrenderer.cpp). The body is the
+    // user's own text, but a body that can inject markup into its own
+    // generated HTML part is a sharp edge with no upside.
+    //
+    // Asserted on the actual placeholder rather than only "no <script>",
+    // because the weaker assertion would still pass with CMARK_OPT_UNSAFE
+    // set by mistake, as long as something ELSE in the string also matched
+    // "not <script>" and "contains after" (measured: it does not distinguish
+    // safe from unsafe mode on its own). "raw HTML omitted" is what safe mode
+    // actually emits in place of the tag.
     const QString html = MarkdownRenderer::toHtml(
         QStringLiteral("<script>alert(1)</script>\n\nafter"));
     QVERIFY2(!html.contains(QStringLiteral("<script>")), qPrintable(html));
+    QVERIFY2(html.contains(QStringLiteral("raw HTML omitted")), qPrintable(html));
     QVERIFY2(html.contains(QStringLiteral("after")), qPrintable(html));
 }
 
+void TestMarkdownRenderer::unsafeLinksAreStripped()
+{
+    // A protection this gets for free from safe mode, and previously
+    // asserted nothing about: a javascript: link is replaced with an empty
+    // href rather than passed through. The body is the user's own text, but
+    // it is rendered into an HTML part sent to other people, so a
+    // javascript: link surviving into that part would be a real defect, not
+    // a cosmetic one.
+    const QString html = MarkdownRenderer::toHtml(
+        QStringLiteral("[click](javascript:alert(1))"));
+    QVERIFY2(!html.contains(QStringLiteral("javascript:")), qPrintable(html));
+}
+
 void TestMarkdownRenderer::accentedTextSurvivesAsUtf8()
 {
     // This user writes Italian, so accented text is every message rather
-- 
cgit v1.2.3