1 files changed, 21 insertions(+), 7 deletions(-)

M doc/review.go

@@ 18,14 18,28 @@ import (
 // SECURITY, AND WHY IT DIFFERS FROM NewRenderer. The read plane turns goldmark's
 // unsafe mode on, and the reason it states — "documents here are first-party and
 // reviewed" — is exactly inverted on the review page: the source is
-// agent-authored and approving it is what the page is for. So this renderer
-// leaves unsafe OFF. Do not "align it with the read renderer": a <script> in a
-// proposed document would then run in the reviewer's session, with the
-// reviewer's cookie, on the page whose whole purpose is to decide whether that
-// document is acceptable. The tests in review_test.go pin it.
+// agent-authored and approving it is what the page is for. A <script> reaching
+// this page runs in the reviewer's session, with the reviewer's cookie, while
+// they decide whether that document is acceptable.
 //
-// Unsafe off costs one thing this plane cannot afford, which is why
-// reviewHTMLRenderer exists below: goldmark's own answer to raw HTML with unsafe
+// THREE THINGS HOLD THAT, and it is worth knowing which does what, because two
+// of them are load-bearing and the famous one is not:
+//
+//  1. reviewHTMLRenderer, below, escapes raw HTML instead of emitting it. This is
+//     the mechanism for the <script> case. Removing it turns six tests red.
+//  2. reviewResolver refuses a dangerous destination, which is the mechanism for
+//     [[javascript:…|click]]: wikilink.go writes its own anchor and takes the
+//     href from the Target without checking it. Removing it turns three tests
+//     red, one of them exactly that wikilink.
+//  3. Unsafe stays OFF. Measured by mutation: putting html.WithUnsafe() back
+//     turns NOTHING red today, because (1) intercepts the node kinds it governs
+//     and (2) strips the destinations before goldmark's own check would. It stays
+//     off anyway — it is the backstop for a goldmark version that renders
+//     something neither (1) nor (2) covers, and for the day someone simplifies
+//     one of them away. Do not read the green suite as licence to turn it on.
+//
+// Unsafe off costs one thing this plane cannot afford, which is why (1) is not
+// simply "leave it to goldmark": goldmark's own answer to raw HTML with unsafe
 // off is to REPLACE it with "<!-- raw HTML omitted -->", and a review page that
 // silently drops part of the document under review is worse than one that shows
 // markup. Here raw HTML is escaped and shown as the text it is.