feat(classes): allow the Blog Editor's own markup in the HTML policy
- figure data-embed ^youtube$ and data-id ^[A-Za-z0-9_-]{11}$ (the embed
placeholder), img alt with any text, ol start ^[0-9]+$; nothing else
- TestEditorOutputPassesPolicy checks every editor-corpus.json entry byte
for byte with an empty report, plus the negative cases;
TestSanitizeHTMLReport pins the removal report
- CheckHTML documented as a predicate no write path refuses with; README
lists the four additions and the report API
This commit is contained in:
253
classes/editor_policy_test.go
Normal file
253
classes/editor_policy_test.go
Normal file
@@ -0,0 +1,253 @@
|
||||
package classes
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"os"
|
||||
"slices"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
xhtml "golang.org/x/net/html"
|
||||
)
|
||||
|
||||
// editorPlaceholder is the Blog Editor's YouTube embed placeholder.
|
||||
const editorPlaceholder = `<figure data-embed="youtube" data-id="dQw4w9WgXcQ"></figure>`
|
||||
|
||||
// editorCorpus reads plan 02's editor-corpus.json, the editor's own output.
|
||||
// A missing or empty file fails the test; it never skips.
|
||||
func editorCorpus(t *testing.T) []struct{ Name, HTML string } {
|
||||
t.Helper()
|
||||
raw, err := os.ReadFile("../tests/blogeditor/fixtures/editor-corpus.json")
|
||||
if err != nil {
|
||||
t.Fatalf("editor corpus: %v", err)
|
||||
}
|
||||
var entries []struct {
|
||||
Name string `json:"name"`
|
||||
HTML string `json:"html"`
|
||||
}
|
||||
if err := json.Unmarshal(raw, &entries); err != nil {
|
||||
t.Fatalf("editor corpus: %v", err)
|
||||
}
|
||||
if len(entries) == 0 {
|
||||
t.Fatal("editor corpus is empty")
|
||||
}
|
||||
out := make([]struct{ Name, HTML string }, 0, len(entries))
|
||||
for _, e := range entries {
|
||||
if e.Name == "" || e.HTML == "" {
|
||||
t.Fatalf("editor corpus entry %+v has no name or html", e)
|
||||
}
|
||||
out = append(out, struct{ Name, HTML string }{e.Name, e.HTML})
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// tagAttrs returns the attribute names of every start tag named tag in doc,
|
||||
// one slice per tag, repeats included.
|
||||
func tagAttrs(doc, tag string) [][]string {
|
||||
var out [][]string
|
||||
z := xhtml.NewTokenizer(strings.NewReader(doc))
|
||||
for {
|
||||
tt := z.Next()
|
||||
if tt == xhtml.ErrorToken {
|
||||
return out
|
||||
}
|
||||
if tt != xhtml.StartTagToken && tt != xhtml.SelfClosingTagToken {
|
||||
continue
|
||||
}
|
||||
raw := rawAttrNames(z.Raw())
|
||||
if z.Token().Data == tag {
|
||||
out = append(out, raw)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestEditorOutputPassesPolicy pins Phase 16.2 D-05 (amended): the HTML
|
||||
// policy keeps every piece of markup the Blog Editor writes, byte for byte
|
||||
// and with an empty removal report, and its four editor-output additions
|
||||
// (figure data-embed and data-id, img alt, ol start) admit nothing else.
|
||||
func TestEditorOutputPassesPolicy(t *testing.T) {
|
||||
for _, e := range editorCorpus(t) {
|
||||
t.Run("corpus "+e.Name, func(t *testing.T) {
|
||||
if got := SanitizeHTML(e.HTML); got != e.HTML {
|
||||
t.Fatalf("SanitizeHTML changed the editor's output:\n got %q\nwant %q", got, e.HTML)
|
||||
}
|
||||
if _, report := SanitizeHTMLReport(e.HTML); !report.Empty() {
|
||||
t.Fatalf("report = %+v, want empty", report)
|
||||
}
|
||||
if err := CheckHTML(e.HTML); err != nil {
|
||||
t.Fatalf("CheckHTML = %v", err)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
t.Run("placeholder is kept", func(t *testing.T) {
|
||||
if got := SanitizeHTML(editorPlaceholder); got != editorPlaceholder {
|
||||
t.Fatalf("SanitizeHTML(placeholder) = %q", got)
|
||||
}
|
||||
out, err := FormatHTML("Intro\n\n" + editorPlaceholder + "\n\nOutro")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if !strings.Contains(out, editorPlaceholder) {
|
||||
t.Fatalf("FormatHTML dropped the placeholder: %q", out)
|
||||
}
|
||||
})
|
||||
|
||||
stripped := []struct {
|
||||
name, in, want string
|
||||
}{
|
||||
{"other provider", `<figure data-embed="vimeo" data-id="dQw4w9WgXcQ"></figure>`, `<figure data-id="dQw4w9WgXcQ"></figure>`},
|
||||
{"provider with a suffix", `<figure data-embed="youtube2" data-id="dQw4w9WgXcQ"></figure>`, `<figure data-id="dQw4w9WgXcQ"></figure>`},
|
||||
{"id of 10 characters", `<figure data-embed="youtube" data-id="dQw4w9WgXc"></figure>`, `<figure data-embed="youtube"></figure>`},
|
||||
{"id of 12 characters", `<figure data-embed="youtube" data-id="dQw4w9WgXcQQ"></figure>`, `<figure data-embed="youtube"></figure>`},
|
||||
{"id with a quote", `<figure data-embed="youtube" data-id="dQw4w9"gXcQ"></figure>`, `<figure data-embed="youtube"></figure>`},
|
||||
{"id with a space", `<figure data-embed="youtube" data-id="dQw4w9 gXcQ"></figure>`, `<figure data-embed="youtube"></figure>`},
|
||||
{"id with an angle bracket", `<figure data-embed="youtube" data-id="dQw4w9<gXcQ"></figure>`, `<figure data-embed="youtube"></figure>`},
|
||||
{"embed attributes on div", `<div data-embed="youtube" data-id="dQw4w9WgXcQ">x</div>`, `<div>x</div>`},
|
||||
{"embed attributes on img", `<img src="/a.jpg" data-embed="youtube" data-id="dQw4w9WgXcQ">`, `<img src="/a.jpg">`},
|
||||
{"embed attributes on p", `<p data-embed="youtube" data-id="dQw4w9WgXcQ">x</p>`, `<p>x</p>`},
|
||||
{"other data attribute on figure", `<figure data-embed="youtube" data-id="dQw4w9WgXcQ" data-src="https://evil.test/"></figure>`, editorPlaceholder},
|
||||
{"data attribute alone on figure", `<figure data-url="https://evil.test/"></figure>`, `<figure></figure>`},
|
||||
{"onload on the placeholder", `<figure data-embed="youtube" data-id="dQw4w9WgXcQ" onload="alert(1)"></figure>`, editorPlaceholder},
|
||||
{"iframe next to the placeholder", editorPlaceholder + `<iframe src="https://www.youtube.com/embed/dQw4w9WgXcQ"></iframe>`, editorPlaceholder},
|
||||
{"start that is not a number", `<ol start="x"><li>a</li></ol>`, `<ol><li>a</li></ol>`},
|
||||
{"negative start", `<ol start="-1"><li>a</li></ol>`, `<ol><li>a</li></ol>`},
|
||||
{"start on ul", `<ul start="3"><li>a</li></ul>`, `<ul><li>a</li></ul>`},
|
||||
{"alt on p", `<p alt="x">y</p>`, `<p>y</p>`},
|
||||
}
|
||||
for _, tc := range stripped {
|
||||
t.Run("strips "+tc.name, func(t *testing.T) {
|
||||
got, report := SanitizeHTMLReport(tc.in)
|
||||
if got != tc.want {
|
||||
t.Fatalf("SanitizeHTMLReport(%q) = %q, want %q", tc.in, got, tc.want)
|
||||
}
|
||||
if report.Empty() {
|
||||
t.Fatalf("report is empty for a stripped input %q", tc.in)
|
||||
}
|
||||
if CheckHTML(tc.in) == nil {
|
||||
t.Fatalf("CheckHTML(%q) = nil, want an error", tc.in)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
t.Run("alt keeps any text as one attribute", func(t *testing.T) {
|
||||
for _, in := range []string{
|
||||
`<img src="/a.jpg" alt="Turkey Tail: czy to "grzyb odporności"?">`,
|
||||
`<img src="/a.jpg" alt="x" onerror="alert(1)">`,
|
||||
`<img src="/a.jpg" alt='x" onerror="alert(1)'>`,
|
||||
`<img src="/a.jpg" alt="<script>alert(1)</script>">`,
|
||||
} {
|
||||
got, report := SanitizeHTMLReport(in)
|
||||
if got != in || !report.Empty() {
|
||||
t.Fatalf("SanitizeHTMLReport(%q) = %q, %+v, want it unchanged", in, got, report)
|
||||
}
|
||||
out := htmlPolicy.Sanitize(in)
|
||||
if attrs := tagAttrs(out, "img"); len(attrs) != 1 || !slices.Equal(attrs[0], []string{"src", "alt"}) {
|
||||
t.Fatalf("policy output %q has img attributes %v, want [src alt]", out, attrs)
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("ol start keeps a number", func(t *testing.T) {
|
||||
const in = `<ol start="3"><li>c</li></ol>`
|
||||
if got := SanitizeHTML(in); got != in {
|
||||
t.Fatalf("SanitizeHTML(%q) = %q", in, got)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// TestSanitizeHTMLReport pins the removal report the Blog Editor turns into
|
||||
// admin notices (D-10 amended).
|
||||
func TestSanitizeHTMLReport(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
in, out string
|
||||
elements []string
|
||||
elementCount int
|
||||
attributes []string
|
||||
attributeCount int
|
||||
comments int
|
||||
other bool
|
||||
}{
|
||||
{name: "clean", in: `<p>Plain <a href="https://example.test/">link</a></p>`, out: `<p>Plain <a href="https://example.test/">link</a></p>`},
|
||||
{name: "style element", in: `<p>a</p><style>p { color: red }</style>`, out: `<p>a</p>`, elements: []string{"style"}, elementCount: 1},
|
||||
{name: "onclick attribute", in: `<p onclick="steal()">x</p>`, out: `<p>x</p>`, attributes: []string{"onclick"}, attributeCount: 1},
|
||||
{name: "comment", in: `<p>a</p><!-- hidden --><!-- two -->`, out: `<p>a</p>`, comments: 2},
|
||||
{name: "repeated attribute", in: `<p class="a" class="b">x</p>`, out: `<p class="a">x</p>`, attributes: []string{"class"}, attributeCount: 1},
|
||||
{name: "style loses a property", in: `<p style="color: red; position: fixed">x</p>`, out: `<p style="color: red">x</p>`, attributes: []string{"style"}, attributeCount: 1},
|
||||
{name: "javascript link", in: `<a href="javascript:alert(1)">x</a><a href="/ok">y</a>`, out: `x<a href="/ok">y</a>`, elements: []string{"a"}, elementCount: 1},
|
||||
{
|
||||
name: "more than ten elements",
|
||||
in: `<script></script><iframe></iframe><object></object><embed><form></form><input><button></button><select></select><textarea></textarea><base><meta><link><script></script>`,
|
||||
out: ``,
|
||||
elements: []string{"script", "iframe", "object", "embed", "form", "input", "button", "select", "textarea", "base"},
|
||||
elementCount: 13,
|
||||
},
|
||||
{
|
||||
name: "more than ten attributes",
|
||||
in: `<p onclick="a" onload="a" onerror="a" onmouseover="a" onfocus="a" onblur="a" onkeyup="a" onkeydown="a" onchange="a" onsubmit="a" oninput="a" data-x="a">x</p>`,
|
||||
out: `<p>x</p>`,
|
||||
attributes: []string{"onclick", "onload", "onerror", "onmouseover", "onfocus", "onblur", "onkeyup", "onkeydown", "onchange", "onsubmit"},
|
||||
attributeCount: 12,
|
||||
},
|
||||
{name: "doctype", in: "<!DOCTYPE html><p>x</p>", out: "<p>x</p>", other: true},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
got, r := SanitizeHTMLReport(tc.in)
|
||||
if got != tc.out {
|
||||
t.Fatalf("output = %q, want %q", got, tc.out)
|
||||
}
|
||||
if !slices.Equal(r.Elements, tc.elements) || r.ElementCount != tc.elementCount ||
|
||||
!slices.Equal(r.Attributes, tc.attributes) || r.AttributeCount != tc.attributeCount ||
|
||||
r.Comments != tc.comments || r.Other != tc.other {
|
||||
t.Fatalf("report = %+v", r)
|
||||
}
|
||||
if r.Empty() != (got == tc.in) {
|
||||
t.Fatalf("Empty() = %v but output changed = %v", r.Empty(), got != tc.in)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
t.Run("cut lists end the notice names with an ellipsis", func(t *testing.T) {
|
||||
_, r := SanitizeHTMLReport(cases[7].in)
|
||||
notices := removalNotices(r)
|
||||
if len(notices) != 1 || notices[0].Count != 13 || !strings.HasSuffix(notices[0].Params["names"], ", …") ||
|
||||
!strings.HasPrefix(notices[0].Params["names"], "script, iframe, ") {
|
||||
t.Fatalf("notices = %#v", notices)
|
||||
}
|
||||
_, r = SanitizeHTMLReport(`<p onclick="a">x</p><!-- c -->`)
|
||||
notices = removalNotices(r)
|
||||
if len(notices) != 2 || notices[0].Params["names"] != "onclick" || notices[1].Message != editorRemovedComments || notices[1].Count != 1 {
|
||||
t.Fatalf("notices = %#v", notices)
|
||||
}
|
||||
_, r = SanitizeHTMLReport("<!DOCTYPE html><p>x</p>")
|
||||
if notices = removalNotices(r); len(notices) != 1 || notices[0].Message != editorMarkupChanged || notices[0].Count != 0 {
|
||||
t.Fatalf("notices = %#v", notices)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("report is empty exactly when the output equals the input", func(t *testing.T) {
|
||||
inputs := []string{}
|
||||
for _, tc := range unsafePayloads {
|
||||
inputs = append(inputs, tc.src)
|
||||
}
|
||||
fixture, err := os.ReadFile("../tests/blogeditor/fixtures/imported-post.html")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
inputs = append(inputs, string(fixture))
|
||||
for _, e := range editorCorpus(t) {
|
||||
inputs = append(inputs, e.HTML)
|
||||
}
|
||||
for _, in := range inputs {
|
||||
got, r := SanitizeHTMLReport(in)
|
||||
if r.Empty() != (got == in) {
|
||||
t.Fatalf("SanitizeHTMLReport(%q): Empty() = %v, output %q", in, r.Empty(), got)
|
||||
}
|
||||
if got != SanitizeHTML(in) {
|
||||
t.Fatalf("SanitizeHTMLReport and SanitizeHTML disagree on %q", in)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
@@ -54,6 +54,22 @@ func newHTMLPolicy() *bluemonday.Policy {
|
||||
p.AllowAttrs("dir").Matching(regexp.MustCompile(`(?i)^(ltr|rtl|auto)$`)).Globally()
|
||||
p.AllowAttrs("data-message-author-role", "data-message-id").
|
||||
Matching(regexp.MustCompile(`^[A-Za-z0-9_-]+$`)).OnElements("div")
|
||||
// Blog Editor output (Phase 16.2 D-05), exactly what the editor writes
|
||||
// and nothing more; plan 02's editor-corpus.json pins it
|
||||
// (TestEditorOutputPassesPolicy) and the site layer mirrors these rules.
|
||||
// The embed placeholder <figure data-embed="youtube" data-id="..."></figure>:
|
||||
// the provider name and the 11-character video id, on figure only. The
|
||||
// layer expands it; no iframe is ever stored.
|
||||
p.AllowAttrs("data-embed").Matching(regexp.MustCompile(`^youtube$`)).OnElements("figure")
|
||||
p.AllowAttrs("data-id").Matching(regexp.MustCompile(`^[A-Za-z0-9_-]{11}$`)).OnElements("figure")
|
||||
// The editor's alt field takes free text, which UGC's Paragraph pattern
|
||||
// cuts at ':', '?', '"' and more. bluemonday accepts an attribute when
|
||||
// any of its rules matches and escapes the value on output; alt carries
|
||||
// no URL.
|
||||
p.AllowAttrs("alt").OnElements("img")
|
||||
// TipTap's ordered-list input rule writes start when a list does not
|
||||
// begin at 1.
|
||||
p.AllowAttrs("start").Matching(regexp.MustCompile(`^[0-9]+$`)).OnElements("ol")
|
||||
return p
|
||||
}
|
||||
|
||||
@@ -310,10 +326,12 @@ func textOf(v any) string {
|
||||
}
|
||||
}
|
||||
|
||||
// CheckHTML gates HTML that is stored as is, such as category content. It
|
||||
// uses the FormatHTML allow-list but refuses instead of stripping: doc is
|
||||
// refused when the policy would remove or change any element, attribute or
|
||||
// text in it.
|
||||
// CheckHTML reports whether the FormatHTML allow-list would remove or
|
||||
// change any element, attribute or text in doc: it returns an error when it
|
||||
// would, nil when the policy keeps doc whole. Since Phase 16.2 no admin or
|
||||
// API write path refuses with it (admin saves strip and report through
|
||||
// SanitizeHTMLReport, D-10 amended); it is the predicate counterpart of the
|
||||
// site layer's isSafeJournalHtml, and the policy cross-check tests use it.
|
||||
func CheckHTML(doc string) error {
|
||||
if _, changed := sanitizeHTML(doc); changed {
|
||||
return errors.New("journal: HTML contains elements, attributes or URLs that are not allowed")
|
||||
|
||||
Reference in New Issue
Block a user