From 47746c7ad33df2e9aa107933b0c52059a282e782 Mon Sep 17 00:00:00 2001 From: oppiliappan Date: Thu, 14 Aug 2025 10:42:26 +0100 Subject: [PATCH] appview/pages: rework sanitizer Change-Id: tpnnpmqzsvzttyswktyxykmpkosxpzpt - initialize sanitizer once, and reuse for life - improve policies, and allow sanitizer to hold multiple policies (this will come in handy, for PR titles, repo description, profiles description etc.) - add general safe items to allow list, most of these are generated by goldmark GFM Signed-off-by: oppiliappan --- appview/pages/markup/markdown.go | 61 ++++++++++++++++++++++++++++---- appview/pages/pages.go | 16 +++++---- 2 files changed, 64 insertions(+), 13 deletions(-) diff --git a/appview/pages/markup/markdown.go b/appview/pages/markup/markdown.go index df4afa5e..759ddba7 100644 --- a/appview/pages/markup/markdown.go +++ b/appview/pages/markup/markdown.go @@ -7,6 +7,7 @@ import ( "io" "net/url" "path" + "regexp" "strings" "github.com/microcosm-cc/bluemonday" @@ -40,6 +41,11 @@ type RenderContext struct { repoinfo.RepoInfo IsDev bool RendererType RendererType + Sanitizer Sanitizer +} + +type Sanitizer struct { + defaultPolicy *bluemonday.Policy } func (rctx *RenderContext) RenderMarkdown(source string) string { @@ -145,14 +151,56 @@ func visitNode(ctx *RenderContext, node *htmlparse.Node) { } } -func (rctx *RenderContext) Sanitize(html string) string { +func (rctx *RenderContext) SanitizeDefault(html string) string { + return rctx.Sanitizer.defaultPolicy.Sanitize(html) +} + +func NewSanitizer() Sanitizer { + return Sanitizer{ + defaultPolicy: defaultPolicy(), + } +} +func defaultPolicy() *bluemonday.Policy { policy := bluemonday.UGCPolicy() + // Allow generally safe attributes + generalSafeAttrs := []string{ + "abbr", "accept", "accept-charset", + "accesskey", "action", "align", "alt", + "aria-describedby", "aria-hidden", "aria-label", "aria-labelledby", + "axis", "border", "cellpadding", "cellspacing", "char", + "charoff", "charset", "checked", + "clear", "cols", "colspan", "color", + "compact", "coords", "datetime", "dir", + "disabled", "enctype", "for", "frame", + "headers", "height", "hreflang", + "hspace", "ismap", "label", "lang", + "maxlength", "media", "method", + "multiple", "name", "nohref", "noshade", + "nowrap", "open", "prompt", "readonly", "rel", "rev", + "rows", "rowspan", "rules", "scope", + "selected", "shape", "size", "span", + "start", "summary", "tabindex", "target", + "title", "type", "usemap", "valign", "value", + "vspace", "width", "itemprop", + } + + generalSafeElements := []string{ + "h1", "h2", "h3", "h4", "h5", "h6", "h7", "h8", "br", "b", "i", "strong", "em", "a", "pre", "code", "img", "tt", + "div", "ins", "del", "sup", "sub", "p", "ol", "ul", "table", "thead", "tbody", "tfoot", "blockquote", "label", + "dl", "dt", "dd", "kbd", "q", "samp", "var", "hr", "ruby", "rt", "rp", "li", "tr", "td", "th", "s", "strike", "summary", + "details", "caption", "figure", "figcaption", + "abbr", "bdo", "cite", "dfn", "mark", "small", "span", "time", "video", "wbr", + } + + policy.AllowAttrs(generalSafeAttrs...).OnElements(generalSafeElements...) + // video - policy.AllowElements("video") - policy.AllowAttrs("controls").OnElements("video") - policy.AllowElements("source") - policy.AllowAttrs("src", "type").OnElements("source") + policy.AllowAttrs("src", "autoplay", "controls").OnElements("video") + + // checkboxes + policy.AllowAttrs("type").Matching(regexp.MustCompile(`^checkbox$`)).OnElements("input") + policy.AllowAttrs("checked", "disabled", "data-source-position").OnElements("input") // centering content policy.AllowElements("center") @@ -173,7 +221,8 @@ func (rctx *RenderContext) Sanitize(html string) string { "margin-top", "margin-bottom", ) - return policy.Sanitize(html) + + return policy } type MarkdownTransformer struct { diff --git a/appview/pages/pages.go b/appview/pages/pages.go index 0cd9b402..0e740b2d 100644 --- a/appview/pages/pages.go +++ b/appview/pages/pages.go @@ -57,6 +57,7 @@ func NewPages(config *config.Config) *Pages { IsDev: config.Core.Dev, CamoUrl: config.Camo.Host, CamoSecret: config.Camo.SharedSecret, + Sanitizer: markup.NewSanitizer(), } p := &Pages{ @@ -522,14 +523,13 @@ func (p *Pages) RepoIndexPage(w io.Writer, params RepoIndexParams) error { p.rctx.RendererType = markup.RendererTypeRepoMarkdown if params.ReadmeFileName != "" { - var htmlString string ext := filepath.Ext(params.ReadmeFileName) switch ext { case ".md", ".markdown", ".mdown", ".mkdn", ".mkd": - htmlString = p.rctx.Sanitize(htmlString) - htmlString = p.rctx.RenderMarkdown(params.Readme) params.Raw = false - params.HTMLReadme = template.HTML(htmlString) + htmlString := p.rctx.RenderMarkdown(params.Readme) + sanitized := p.rctx.SanitizeDefault(htmlString) + params.HTMLReadme = template.HTML(sanitized) default: params.Raw = true } @@ -668,7 +668,8 @@ func (p *Pages) RepoBlob(w io.Writer, params RepoBlobParams) error { p.rctx.RepoInfo = params.RepoInfo p.rctx.RendererType = markup.RendererTypeRepoMarkdown htmlString := p.rctx.RenderMarkdown(params.Contents) - params.RenderedContents = template.HTML(p.rctx.Sanitize(htmlString)) + sanitized := p.rctx.SanitizeDefault(htmlString) + params.RenderedContents = template.HTML(sanitized) } } @@ -1182,9 +1183,10 @@ func (p *Pages) SingleString(w io.Writer, params SingleStringParams) error { if params.ShowRendered { switch markup.GetFormat(params.String.Filename) { case markup.FormatMarkdown: - p.rctx.RendererType = markup.RendererTypeDefault + p.rctx.RendererType = markup.RendererTypeRepoMarkdown htmlString := p.rctx.RenderMarkdown(params.String.Contents) - params.RenderedContents = template.HTML(p.rctx.Sanitize(htmlString)) + sanitized := p.rctx.SanitizeDefault(htmlString) + params.RenderedContents = template.HTML(sanitized) } } -- 2.43.0