diff --git a/README.md b/README.md
index 9fd0f7f..dee4119 100644
--- a/README.md
+++ b/README.md
@@ -18,7 +18,7 @@ WinterCMS theme components such as `journalPost` and `journalPosts` are a later
## Features
-- **Posts.** `models.Post` maps `golem15_journal_posts`. Translatable fields are title, content, content_html, excerpt, metadata, and slug (indexed). Fillable does not include `redactor_id`, `content_html`, or `user_id`. Attachments `featured_images` and `content_images` use `system_files` under the same MorphName. YAML admin uses `mltext`/`mlmarkdown`. `classes.FormatHTML` regenerates `content_html` on admin save. It passes raw HTML in `content` through unchanged (Winter parity), so imported HTML bodies survive a save, and its gate still refuses script, iframe, object and embed tags, event handlers and javascript, vbscript and data URLs on every write path (admin save, `journal:import-posts`, Journal API writes). The API response shapes are unchanged.
+- **Posts.** `models.Post` maps `golem15_journal_posts`. Translatable fields are title, content, content_html, excerpt, metadata, and slug (indexed). Fillable does not include `redactor_id`, `content_html`, or `user_id`. Attachments `featured_images` and `content_images` use `system_files` under the same MorphName. YAML admin uses `mltext`/`mlmarkdown`. `classes.FormatHTML` regenerates `content_html` on every write path (admin save, `journal:import-posts`, Journal API writes). It renders markdown and passes raw HTML in `content` through the renderer (Winter parity), then sanitizes the result with a `bluemonday` allow-list: bluemonday's UGC policy plus `footer`, `class`, schema.org microdata, the `color`, `font-size`, `font-weight` and `text-align` styles, goldmark's footnote roles, `dir="auto"` and two pasted-transcript `data-message-*` attributes. Like Winter's `Html::clean`, it strips what the policy does not allow instead of refusing the save: script, style, iframe, object, embed, form, base and meta elements, comments, event handler attributes, and URLs other than http, https, mailto and relative ones. HTML the policy keeps whole is stored byte for byte, so imported bodies survive a save. `content` itself is stored as submitted. The API response shapes are unchanged.
- **Categories and tags.** `models.Category` is translatable (name, description, slug). It also stores `content`, an HTML host column that is not translatable, edited in the admin as a textarea. Fillable is name, slug, code, description, content and parent_id; nest_* columns exist in DDL and are not fillable. `models.Tag` is not translatable. Fillable is name, slug, description. Pivots are `golem15_journal_posts_categories` and `golem15_journal_posts_tags`.
- **Settings.** Singleton row ID=1 on `golem15_journal_settings`, not PHP `system_settings`. `search_use_typesense` defaults to false. A fresh save never contacts Typesense.
- **Public HTTP.** Anonymous GET `/_journal/api/v1/posts`, `/posts/{slug}`, `/categories`, `/tags`, `/rss`, `/authors/{author}`, and `/authors/{author}/posts`. Writes under the same prefix require a cabana backend JWT (`aud=backend`).
@@ -177,7 +177,7 @@ func (Post) MorphName() string { return `Golem15\Journal\Models\Post
| `models.Settings` | Singleton settings row; `TableName` is `golem15_journal_settings`. |
| `models.All` | Models registered by this plugin. |
| `updates.All` | gormigrate steps registered by this plugin. |
-| `classes.CheckHTML` | The `FormatHTML` safety gate for HTML stored as is, such as category content. |
+| `classes.CheckHTML` | Gate for HTML stored as is, such as category content: refuses HTML that the `FormatHTML` allow-list would strip or change, instead of stripping it. |
## CLI commands
@@ -198,7 +198,7 @@ Both commands write JSON files (one per post slug), matching the PHP `journal:ex
- SummerCMS modules: backpack, beachcomber, bonfire, cabana, lagoon/attach, pact, party, surf.
- Compiled plugin: `golem15.translate`.
-- Third-party: `gorm.io/gorm`, `github.com/go-gormigrate/gormigrate/v2`, `github.com/yuin/goldmark`, `gocloud.dev`. Test-only: `github.com/testcontainers/testcontainers-go`.
+- Third-party: `gorm.io/gorm`, `github.com/go-gormigrate/gormigrate/v2`, `github.com/yuin/goldmark`, `github.com/microcosm-cc/bluemonday`, `golang.org/x/net/html`, `gocloud.dev`. Test-only: `github.com/testcontainers/testcontainers-go`.
## Testing
diff --git a/classes/check_html_test.go b/classes/check_html_test.go
index cc6f2fd..059c3b5 100644
--- a/classes/check_html_test.go
+++ b/classes/check_html_test.go
@@ -11,14 +11,29 @@ func TestCheckHTML(t *testing.T) {
{"empty", "", true},
{"heading and link", `
Cordyceps Plain link
`, true},
{"table", ``, true},
+ {"polish date label", `Data: 12 maja 2024
`, true},
+ {"metadata text", `Some metadata: here
`, true},
+ {"quotes, apostrophes and nbsp", `Co to jest Lion's Mane? "superior herb" & inne
`, true},
+ {"pasted chat transcript", ``, true},
{"script", ``, false},
{"iframe", ``, false},
{"object", ``, false},
{"embed", ``, false},
{"event handler", ` `, false},
+ {"event handler without a space", `
`, false},
+ {"slash-separated svg onload", "\n\n
", false},
{"javascript url", ``, false},
+ {"numeric entity colon", ` x `, false},
+ {"named entity colon", `x `, false},
+ {"entity inside the scheme", `x `, false},
+ {"tab inside the scheme", "x ", false},
{"vbscript url mixed case", ``, false},
{"data url", ` `, false},
+ {"base", ` `, false},
+ {"meta refresh", ` `, false},
+ {"form", ``, false},
+ {"style element", ``, false},
+ {"comment", `a
`, false},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
@@ -31,4 +46,16 @@ func TestCheckHTML(t *testing.T) {
}
})
}
+
+ // Every stored-XSS payload FormatHTML strips is refused by CheckHTML.
+ for _, tc := range unsafePayloads {
+ if tc.src[0] != '<' {
+ continue // markdown, not HTML
+ }
+ t.Run("payload "+tc.name, func(t *testing.T) {
+ if err := CheckHTML(tc.src); err == nil {
+ t.Fatalf("CheckHTML(%q) = nil, want an error", tc.src)
+ }
+ })
+ }
}
diff --git a/classes/format_html.go b/classes/format_html.go
index f5ce7d4..513f583 100644
--- a/classes/format_html.go
+++ b/classes/format_html.go
@@ -2,14 +2,19 @@ package classes
import (
"bytes"
+ "errors"
"fmt"
+ "io"
"regexp"
+ "slices"
"strings"
+ "github.com/microcosm-cc/bluemonday"
"github.com/yuin/goldmark"
"github.com/yuin/goldmark/extension"
"github.com/yuin/goldmark/parser"
"github.com/yuin/goldmark/renderer/html"
+ xhtml "golang.org/x/net/html"
)
var (
@@ -17,49 +22,105 @@ var (
goldmark.WithExtensions(extension.Footnote, extension.Table),
goldmark.WithParserOptions(parser.WithAttribute()),
// Winter's Markdown::parse passes raw HTML through, so imported HTML
- // bodies round-trip unchanged (D-03, PHP parity); rejectUnsafe gates
- // the rendered result on every write path.
+ // bodies round-trip (D-03, PHP parity); htmlPolicy then sanitizes the
+ // rendered result on every write path.
goldmark.WithRendererOptions(html.WithUnsafe()),
)
- markdownUnsafeTag = regexp.MustCompile(`(?i)<(?:script|iframe|object|embed)\b`)
- markdownEventHandler = regexp.MustCompile(`(?i)\son[a-z]+\s*=`)
- markdownDangerousURL = regexp.MustCompile(`(?i)(?:javascript|vbscript|data):`)
+ // htmlPolicy is the allow-list for stored journal HTML.
+ htmlPolicy = newHTMLPolicy()
)
+// newHTMLPolicy extends bluemonday's UGC policy only with what goldmark
+// renders and what the imported posts and categories use.
+func newHTMLPolicy() *bluemonday.Policy {
+ p := bluemonday.UGCPolicy()
+ // Winter neither adds nor requires rel="nofollow", so links keep their
+ // stored form.
+ p.RequireNoFollowOnLinks(false)
+ // Imported author footers: .
+ p.AllowElements("footer")
+ p.AllowAttrs("class").Matching(bluemonday.SpaceSeparatedTokens).Globally()
+ p.AllowAttrs("itemscope").Matching(regexp.MustCompile(`^(|itemscope)$`)).Globally()
+ p.AllowAttrs("itemtype").Matching(regexp.MustCompile(`^https?://schema\.org/[A-Za-z]+$`)).Globally()
+ p.AllowAttrs("itemprop").Matching(bluemonday.SpaceSeparatedTokens).Globally()
+ // Inline styles in imported footers and goldmark's table cell alignment.
+ // bluemonday validates each property value.
+ p.AllowStyles("color", "font-size", "font-weight", "text-align").Globally()
+ // goldmark's footnote markup.
+ p.AllowAttrs("role").Matching(regexp.MustCompile(`^doc-(noteref|endnotes|backlink)$`)).OnElements("a", "div")
+ // Pasted chat transcripts in imported category bodies: dir="auto" (the
+ // UGC policy allows only ltr and rtl) and two data attributes.
+ 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")
+ return p
+}
+
// FormatHTML converts journal post markdown to stored HTML using goldmark
// footnote, table, and attribute extensions. Raw HTML in the source passes
-// through unchanged; the result then goes through the same rejectUnsafe gate
-// as cabana.RenderMarkdown. It does not call cabana.RenderMarkdown, whose
-// admin preview stays without unsafe HTML.
+// through the renderer, as it does in Winter's Markdown::parse. The result is
+// then sanitized with a bluemonday allow-list that strips, as Winter's
+// Html::clean does, what it does not allow: script, style, iframe, object,
+// embed, form, base and meta elements, event handler attributes, and URLs
+// other than http, https, mailto and relative ones. HTML that the policy
+// keeps whole is returned byte for byte, so imported bodies round-trip. It
+// does not call cabana.RenderMarkdown, whose admin preview stays without
+// unsafe HTML.
func FormatHTML(src string) (string, error) {
var buf bytes.Buffer
if err := markdownEngine.Convert([]byte(strings.TrimSpace(src)), &buf); err != nil {
return "", fmt.Errorf("journal: markdown: %w", err)
}
- out := buf.String()
- if err := rejectUnsafe(out); err != nil {
- return "", err
- }
+ out, _ := sanitizeHTML(buf.String())
return out, nil
}
-// CheckHTML applies the FormatHTML safety gate to HTML that is stored as is,
-// such as category content. It refuses raw script, iframe, object and embed
-// tags, event handler attributes and javascript, vbscript or data URL schemes.
+// 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.
func CheckHTML(doc string) error {
- return rejectUnsafe(doc)
-}
-
-func rejectUnsafe(doc string) error {
- if markdownUnsafeTag.MatchString(doc) {
- return fmt.Errorf("journal: rendered HTML contains raw unsafe tags")
- }
- if markdownEventHandler.MatchString(doc) {
- return fmt.Errorf("journal: rendered HTML contains event handlers")
- }
- if markdownDangerousURL.MatchString(doc) {
- return fmt.Errorf("journal: rendered HTML contains a dangerous URL scheme")
+ if _, changed := sanitizeHTML(doc); changed {
+ return errors.New("journal: HTML contains elements, attributes or URLs that are not allowed")
}
return nil
}
+
+// sanitizeHTML returns doc unchanged when htmlPolicy keeps all of it, and the
+// policy's output with changed set otherwise. Keeping all of it means doc and
+// the policy output tokenize to the same HTML tokens, so an output that only
+// re-escapes text or attribute values is not a change.
+func sanitizeHTML(doc string) (string, bool) {
+ clean := htmlPolicy.Sanitize(doc)
+ if clean == doc || sameTokens(doc, clean) {
+ return doc, false
+ }
+ return clean, true
+}
+
+// sameTokens reports whether a and b tokenize to the same sequence of HTML
+// tokens: type, tag name or decoded text, and the set of decoded attributes.
+func sameTokens(a, b string) bool {
+ ta, oka := htmlTokens(a)
+ tb, okb := htmlTokens(b)
+ return oka && okb && slices.Equal(ta, tb)
+}
+
+func htmlTokens(doc string) ([]string, bool) {
+ var out []string
+ z := xhtml.NewTokenizer(strings.NewReader(doc))
+ for {
+ tt := z.Next()
+ if tt == xhtml.ErrorToken {
+ return out, errors.Is(z.Err(), io.EOF)
+ }
+ t := z.Token()
+ attrs := make([]string, 0, len(t.Attr))
+ for _, a := range t.Attr {
+ attrs = append(attrs, a.Namespace+":"+a.Key+"="+a.Val)
+ }
+ slices.Sort(attrs)
+ out = append(out, fmt.Sprintf("%d|%s|%q", tt, t.Data, attrs))
+ }
+}
diff --git a/classes/format_html_test.go b/classes/format_html_test.go
index d8beba2..93d92a7 100644
--- a/classes/format_html_test.go
+++ b/classes/format_html_test.go
@@ -3,76 +3,192 @@ package classes
import (
"strings"
"testing"
+
+ xhtml "golang.org/x/net/html"
)
-// TestFormatHTMLRejectsUnsafeHTML substitutes JOURNAL-003/004 Twig XSS
-// templates (Phase 16). Script, iframe, event handlers, and javascript /
-// vbscript / data schemes must fail the reject gate. Footnotes and tables
-// from goldmark extensions still pass.
-func TestFormatHTMLRejectsUnsafeHTML(t *testing.T) {
+// unsafePayloads are the stored-XSS inputs FormatHTML and CheckHTML must
+// neutralize: the JOURNAL-003/004 Twig XSS templates (Phase 16) and the
+// regex-gate bypasses from the Phase 16.1 code review (CR-01).
+var unsafePayloads = []struct {
+ name string
+ src string
+}{
+ {"script", ""},
+ {"mixed-case script", ""},
+ {"script inside an HTML block", ""},
+ {"script after markdown", "# Title\n\nText.\n\n"},
+ {"iframe", ``},
+ {"upper-case iframe", ``},
+ {"object", ` `},
+ {"embed", ``},
+ {"event", ` `},
+ {"svg onload", ` `},
+ {"mixed-case event handler", `Click
`},
+ {"event handler in an inline element", "Text with hover ."},
+ {"event handler without a space", `
`},
+ {"slash-separated svg onload", "\n\n
"},
+ {"markdown attribute event handler", "# Title {onclick=\"alert(1)\"}"},
+ {"javascript", "[x](javascript:alert(1))"},
+ {"mixed-case javascript href", `x `},
+ {"numeric entity colon", `x `},
+ {"named entity colon", `x `},
+ {"entity inside the scheme", `x `},
+ {"tab inside the scheme", "x "},
+ {"newline inside the scheme", "x
"},
+ {"leading space and control characters", "x
"},
+ {"mathml xlink href", "\nx \n
"},
+ {"vbscript", "[x](vbscript:msgbox(1))"},
+ {"raw vbscript href", `x `},
+ {"data", "[x](data:text/html;base64,PHNjcmlwdD5hbGVydCgxKTwvc2NyaXB0Pg==)"},
+ {"raw data URL attribute", ` `},
+ {"markdown image with a data URL", ""},
+ {"base", ` `},
+ {"meta refresh", ` `},
+ {"form", ``},
+ {"style element", ""},
+ {"link stylesheet", ` `},
+ {"html comment", "a
"},
+}
+
+// forbiddenElements must never appear in sanitized journal HTML.
+var forbiddenElements = map[string]bool{
+ "script": true, "iframe": true, "object": true, "embed": true, "base": true,
+ "meta": true, "form": true, "input": true, "button": true, "style": true,
+ "link": true, "svg": true, "math": true, "maction": true, "frame": true,
+ "frameset": true, "applet": true, "textarea": true, "select": true,
+}
+
+// assertNoActiveContent tokenizes doc and fails on forbidden elements, event
+// handler attributes, any style other than the allowed properties, and
+// javascript, vbscript or data URLs after entity decoding and after removing
+// ASCII whitespace and control characters, as a browser's URL parser does.
+func assertNoActiveContent(t *testing.T, doc string) {
+ t.Helper()
+ z := xhtml.NewTokenizer(strings.NewReader(doc))
+ for {
+ tt := z.Next()
+ if tt == xhtml.ErrorToken {
+ return
+ }
+ if tt == xhtml.CommentToken {
+ t.Fatalf("comment survived: %q", doc)
+ }
+ if tt != xhtml.StartTagToken && tt != xhtml.SelfClosingTagToken && tt != xhtml.EndTagToken {
+ continue
+ }
+ tok := z.Token()
+ if forbiddenElements[tok.Data] {
+ t.Fatalf("element <%s> survived: %q", tok.Data, doc)
+ }
+ for _, a := range tok.Attr {
+ key := strings.ToLower(a.Key)
+ if strings.HasPrefix(key, "on") {
+ t.Fatalf("event handler %s survived: %q", a.Key, doc)
+ }
+ if key == "action" || key == "formaction" || key == "srcdoc" || key == "http-equiv" {
+ t.Fatalf("attribute %s survived: %q", a.Key, doc)
+ }
+ var b strings.Builder
+ for _, r := range strings.ToLower(a.Val) {
+ if r > 0x20 {
+ b.WriteRune(r)
+ }
+ }
+ v := b.String()
+ for _, scheme := range []string{"javascript:", "vbscript:", "data:"} {
+ if strings.Contains(v, scheme) {
+ t.Fatalf("%s URL survived in %s: %q", scheme, a.Key, doc)
+ }
+ }
+ }
+ }
+}
+
+// TestFormatHTMLSanitizesUnsafeHTML pins CR-01: every payload is rendered
+// without an error and with all active content stripped, as Winter's
+// Html::clean strips rather than refuses. Footnotes and tables from goldmark
+// extensions still render.
+func TestFormatHTMLSanitizesUnsafeHTML(t *testing.T) {
html, err := FormatHTML("# Hello")
if err != nil || !strings.Contains(html, "") {
t.Fatalf("safe heading: html=%s err=%v", html, err)
}
- table, err := FormatHTML("| a | b |\n| - | - |\n| 1 | 2 |")
+ table, err := FormatHTML("| a | b |\n|:- | -:|\n| 1 | 2 |")
if err != nil {
t.Fatalf("table markdown: %v", err)
}
- if !strings.Contains(table, " `, `href="#fn:1"`, `role="doc-noteref"`, `role="doc-endnotes"`, `role="doc-backlink"`} {
+ if !strings.Contains(fn, want) {
+ t.Fatalf("footnote lacks %s: %s", want, fn)
+ }
}
- // With unsafe rendering the gate is the only defence, so every case
- // must be refused with an error, not merely rendered harmless.
+ attrs, err := FormatHTML("# Title {#intro .lead}")
+ if err != nil || !strings.Contains(attrs, `Title `) {
+ t.Fatalf("markdown attributes: html=%s err=%v", attrs, err)
+ }
+
+ for _, tc := range unsafePayloads {
+ t.Run(tc.name, func(t *testing.T) {
+ out, err := FormatHTML(tc.src)
+ if err != nil {
+ t.Fatalf("FormatHTML(%q) = %v, want sanitized output", tc.src, err)
+ }
+ assertNoActiveContent(t, out)
+ })
+ }
+
+ t.Run("safe parts of a payload survive", func(t *testing.T) {
+ out, err := FormatHTML(`Zdjęcie:
`)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if want := `Zdjęcie:
`; out != want {
+ t.Fatalf("got %q, want %q", out, want)
+ }
+ })
+}
+
+// TestFormatHTMLKeepsSchemeLikeText pins WR-01: text that merely contains
+// "data:", "javascript:" or "vbscript:" is not a URL and passes unchanged.
+func TestFormatHTMLKeepsSchemeLikeText(t *testing.T) {
cases := []struct {
- name string
- src string
+ name, src, want string
}{
- {"script", ""},
- {"mixed-case script", ""},
- {"script inside an HTML block", ""},
- {"script after markdown", "# Title\n\nText.\n\n"},
- {"iframe", ``},
- {"upper-case iframe", ``},
- {"object", ` `},
- {"embed", ``},
- {"event", ` `},
- {"svg onload", ` `},
- {"mixed-case event handler", `Click
`},
- {"event handler in an inline element", "Text with hover ."},
- {"javascript", "[x](javascript:alert(1))"},
- {"mixed-case javascript href", `x `},
- {"vbscript", "[x](vbscript:msgbox(1))"},
- {"raw vbscript href", `x `},
- {"data", "[x](data:text/html;base64,PHNjcmlwdD5hbGVydCgxKTwvc2NyaXB0Pg==)"},
- {"raw data URL attribute", ` `},
- {"markdown image with a data URL", ""},
+ {"polish date label", "Data: 12 maja 2024
", "Data: 12 maja 2024
"},
+ {"markdown date label", "Data publikacji: 12 maja 2024", "Data publikacji: 12 maja 2024
\n"},
+ {"metadata", "Some metadata: here", "Some metadata: here
\n"},
+ {"big data in html", "Big data: trends
", "Big data: trends
"},
+ {"scheme names in prose", "Pisze się javascript: i vbscript: w tekście.
", "Pisze się javascript: i vbscript: w tekście.
"},
+ {"quotes and apostrophes", "Lion's Mane, \"superior herb\" & inne
", "Lion's Mane, \"superior herb\" & inne
"},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
out, err := FormatHTML(tc.src)
- if err == nil {
- t.Fatalf("FormatHTML accepted unsafe input %q: %s", tc.src, out)
+ if err != nil {
+ t.Fatalf("FormatHTML(%q): %v", tc.src, err)
}
- if out != "" {
- t.Fatalf("FormatHTML returned HTML with its error: %s", out)
+ if out != tc.want {
+ t.Fatalf("got %q, want %q", out, tc.want)
}
})
}
}
// TestFormatHTMLKeepsRawHTML pins D-03: raw HTML in content passes through
-// unchanged, as Winter's Markdown::parse does, so an imported HTML body
-// survives an admin save, an import or an API write.
+// the renderer, as Winter's Markdown::parse does, and HTML the policy allows
+// is stored byte for byte, so an imported HTML body survives an admin save,
+// an import or an API write.
func TestFormatHTMLKeepsRawHTML(t *testing.T) {
const paragraphs = "Grzyby rosną w lesie.
\nDruga część .
"
out, err := FormatHTML(paragraphs)
@@ -98,10 +214,13 @@ func TestFormatHTMLKeepsRawHTML(t *testing.T) {
}{
{"paragraphs", "Pierwszy akapit.
\nDrugi akapit z kursywą .
\nTrzeci.
"},
{"class attributes", ``},
- {"https link", `Zobacz źródło .
`},
+ {"https link", `Zobacz źródło .
`},
+ {"relative and mailto links", `blog przypisy mail
`},
{"https image", `
`},
- {"entity in an attribute", `link
`},
+ {"entity in an attribute", `link
`},
{"table", ""},
+ {"headings", "Co to jest Lion's Mane? \nWłaściwości "},
+ {"pasted chat transcript", ``},
{"surrounding whitespace", "\n\n Trimmed.
\n"},
}
for _, tc := range bodies {
@@ -132,16 +251,53 @@ func TestFormatHTMLKeepsRawHTML(t *testing.T) {
"pogrubieniem ",
`linkiem `,
footer,
- style,
} {
if !strings.Contains(out, want) {
t.Fatalf("output lacks %q:\n%s", want, out)
}
}
+ // The style element is stripped with its content, as Html::clean
+ // strips