From b2f2f8528a0eac71e623293012d51109118c1f98 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 23 Oct 2025 16:07:17 +0800 Subject: [PATCH] Fix external render, make iframe render work (#35727, #35730) (#35731) Backport #35727 and #35730 --------- Signed-off-by: wxiaoguang --- custom/conf/app.example.ini | 14 ++- modules/httplib/serve.go | 1 + modules/markup/external/external.go | 20 ++-- modules/markup/internal/finalprocessor.go | 30 ++++- modules/markup/internal/internal_test.go | 37 +++++- modules/markup/internal/renderinternal.go | 8 +- modules/markup/render.go | 94 ++++++++++------ modules/markup/renderer.go | 12 +- modules/setting/markup.go | 26 +++-- routers/web/repo/render.go | 45 ++++---- routers/web/repo/view.go | 17 ++- tests/integration/markup_external_test.go | 105 +++++++++++++----- tests/sqlite.ini.tmpl | 16 ++- web_src/css/markup/content.css | 5 + .../css/standalone/external-render-iframe.css | 1 + web_src/js/markup/content.ts | 2 + web_src/js/markup/render-iframe.ts | 32 ++++++ .../js/standalone/external-render-iframe.ts | 43 +++++++ webpack.config.ts | 4 + 19 files changed, 391 insertions(+), 121 deletions(-) create mode 100644 web_src/css/standalone/external-render-iframe.css create mode 100644 web_src/js/markup/render-iframe.ts create mode 100644 web_src/js/standalone/external-render-iframe.ts diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 4a6356ec50..5fee78af54 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -2540,7 +2540,19 @@ LEVEL = Info ;; * sanitized: Sanitize the content and render it inside current page, default to only allow a few HTML tags and attributes. Customized sanitizer rules can be defined in [markup.sanitizer.*] . ;; * no-sanitizer: Disable the sanitizer and render the content inside current page. It's **insecure** and may lead to XSS attack if the content contains malicious code. ;; * iframe: Render the content in a separate standalone page and embed it into current page by iframe. The iframe is in sandbox mode with same-origin disabled, and the JS code are safely isolated from parent page. -;RENDER_CONTENT_MODE=sanitized +;RENDER_CONTENT_MODE = sanitized +;; The sandbox applied to the iframe and Content-Security-Policy header when RENDER_CONTENT_MODE is `iframe`. +;; It defaults to a safe set of "allow-*" restrictions (space separated). +;; You can also set it by your requirements or use "disabled" to disable the sandbox completely. +;; When set it, make sure there is no security risk: +;; * PDF-only content: generally safe to use "disabled", and it needs to be "disabled" because PDF only renders with no sandbox. +;; * HTML content with JS: if the "RENDER_COMMAND" can guarantee there is no XSS, then it is safe, otherwise, you need to fine tune the "allow-*" restrictions. +;RENDER_CONTENT_SANDBOX = +;; Whether post-process the rendered HTML content, including: +;; resolve relative links and image sources, recognizing issue/commit references, escaping invisible characters, +;; mentioning users, rendering permlink code blocks, replacing emoji shorthands, etc. +;; By default, this is true when RENDER_CONTENT_MODE is `sanitized`, otherwise false. +;NEED_POST_PROCESS = false ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/modules/httplib/serve.go b/modules/httplib/serve.go index 7c1edf432d..b4c5e7fe1e 100644 --- a/modules/httplib/serve.go +++ b/modules/httplib/serve.go @@ -126,6 +126,7 @@ func setServeHeadersByFile(r *http.Request, w http.ResponseWriter, mineBuf []byt // no sandbox attribute for pdf as it breaks rendering in at least safari. this // should generally be safe as scripts inside PDF can not escape the PDF document // see https://bugs.chromium.org/p/chromium/issues/detail?id=413851 for more discussion + // HINT: PDF-RENDER-SANDBOX: PDF won't render in sandboxed context w.Header().Set("Content-Security-Policy", "default-src 'none'; style-src 'unsafe-inline'") } diff --git a/modules/markup/external/external.go b/modules/markup/external/external.go index 39861ade12..3cbe14b86a 100644 --- a/modules/markup/external/external.go +++ b/modules/markup/external/external.go @@ -15,6 +15,8 @@ import ( "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/process" "code.gitea.io/gitea/modules/setting" + + "github.com/kballard/go-shellquote" ) // RegisterRenderers registers all supported third part renderers according settings @@ -56,14 +58,11 @@ func (p *Renderer) SanitizerRules() []setting.MarkupSanitizerRule { return p.MarkupSanitizerRules } -// SanitizerDisabled disabled sanitize if return true -func (p *Renderer) SanitizerDisabled() bool { - return p.RenderContentMode == setting.RenderContentModeNoSanitizer || p.RenderContentMode == setting.RenderContentModeIframe -} - -// DisplayInIFrame represents whether render the content with an iframe -func (p *Renderer) DisplayInIFrame() bool { - return p.RenderContentMode == setting.RenderContentModeIframe +func (p *Renderer) GetExternalRendererOptions() (ret markup.ExternalRendererOptions) { + ret.SanitizerDisabled = p.RenderContentMode == setting.RenderContentModeNoSanitizer || p.RenderContentMode == setting.RenderContentModeIframe + ret.DisplayInIframe = p.RenderContentMode == setting.RenderContentModeIframe + ret.ContentSandbox = p.RenderContentSandbox + return ret } func envMark(envName string) string { @@ -81,7 +80,10 @@ func (p *Renderer) Render(ctx *markup.RenderContext, input io.Reader, output io. envMark("GITEA_PREFIX_SRC"), baseLinkSrc, envMark("GITEA_PREFIX_RAW"), baseLinkRaw, ).Replace(p.Command) - commands := strings.Fields(command) + commands, err := shellquote.Split(command) + if err != nil || len(commands) == 0 { + return fmt.Errorf("%s invalid command %q: %w", p.Name(), p.Command, err) + } args := commands[1:] if p.IsInputFile { diff --git a/modules/markup/internal/finalprocessor.go b/modules/markup/internal/finalprocessor.go index 14d46a161f..4442afa0c9 100644 --- a/modules/markup/internal/finalprocessor.go +++ b/modules/markup/internal/finalprocessor.go @@ -5,11 +5,13 @@ package internal import ( "bytes" + "html/template" "io" ) type finalProcessor struct { renderInternal *RenderInternal + extraHeadHTML template.HTML output io.Writer buf bytes.Buffer @@ -25,6 +27,32 @@ func (p *finalProcessor) Close() error { // because "postProcess" already does so. In the future we could optimize the code to process data on the fly. buf := p.buf.Bytes() buf = bytes.ReplaceAll(buf, []byte(` data-attr-class="`+p.renderInternal.secureIDPrefix), []byte(` class="`)) - _, err := p.output.Write(buf) + + tmp := bytes.TrimSpace(buf) + isLikelyHTML := len(tmp) != 0 && tmp[0] == '<' && tmp[len(tmp)-1] == '>' && bytes.Index(tmp, []byte(` 0 + if !isLikelyHTML { + // not HTML, write back directly + _, err := p.output.Write(buf) + return err + } + + // add our extra head HTML into output + headBytes := []byte("") + posHead := bytes.Index(buf, headBytes) + var part1, part2 []byte + if posHead >= 0 { + part1, part2 = buf[:posHead+len(headBytes)], buf[posHead+len(headBytes):] + } else { + part1, part2 = nil, buf + } + if len(part1) > 0 { + if _, err := p.output.Write(part1); err != nil { + return err + } + } + if _, err := io.WriteString(p.output, string(p.extraHeadHTML)); err != nil { + return err + } + _, err := p.output.Write(part2) return err } diff --git a/modules/markup/internal/internal_test.go b/modules/markup/internal/internal_test.go index 590bcbb67f..a216d75203 100644 --- a/modules/markup/internal/internal_test.go +++ b/modules/markup/internal/internal_test.go @@ -12,7 +12,7 @@ import ( "github.com/stretchr/testify/assert" ) -func TestRenderInternal(t *testing.T) { +func TestRenderInternalAttrs(t *testing.T) { cases := []struct { input, protected, recovered string }{ @@ -30,7 +30,7 @@ func TestRenderInternal(t *testing.T) { for _, c := range cases { var r RenderInternal out := &bytes.Buffer{} - in := r.init("sec", out) + in := r.init("sec", out, "") protected := r.ProtectSafeAttrs(template.HTML(c.input)) assert.EqualValues(t, c.protected, protected) _, _ = io.WriteString(in, string(protected)) @@ -41,7 +41,7 @@ func TestRenderInternal(t *testing.T) { var r1, r2 RenderInternal protected := r1.ProtectSafeAttrs(`
`) assert.EqualValues(t, `
`, protected, "non-initialized RenderInternal should not protect any attributes") - _ = r1.init("sec", nil) + _ = r1.init("sec", nil, "") protected = r1.ProtectSafeAttrs(`
`) assert.EqualValues(t, `
`, protected) assert.Equal(t, "data-attr-class", r1.SafeAttr("class")) @@ -54,8 +54,37 @@ func TestRenderInternal(t *testing.T) { assert.Empty(t, recovered) out2 := &bytes.Buffer{} - in2 := r2.init("sec-other", out2) + in2 := r2.init("sec-other", out2, "") _, _ = io.WriteString(in2, string(protected)) _ = in2.Close() assert.Equal(t, `
`, out2.String(), "different secureID should not recover the value") } + +func TestRenderInternalExtraHead(t *testing.T) { + t.Run("HeadExists", func(t *testing.T) { + out := &bytes.Buffer{} + var r RenderInternal + in := r.init("sec", out, ``) + _, _ = io.WriteString(in, `any`) + _ = in.Close() + assert.Equal(t, `any`, out.String()) + }) + + t.Run("HeadNotExists", func(t *testing.T) { + out := &bytes.Buffer{} + var r RenderInternal + in := r.init("sec", out, ``) + _, _ = io.WriteString(in, `
`) + _ = in.Close() + assert.Equal(t, `
`, out.String()) + }) + + t.Run("NotHTML", func(t *testing.T) { + out := &bytes.Buffer{} + var r RenderInternal + in := r.init("sec", out, ``) + _, _ = io.WriteString(in, ``) + _ = in.Close() + assert.Equal(t, ``, out.String()) + }) +} diff --git a/modules/markup/internal/renderinternal.go b/modules/markup/internal/renderinternal.go index 7a3e37b120..9fd9a1c0e8 100644 --- a/modules/markup/internal/renderinternal.go +++ b/modules/markup/internal/renderinternal.go @@ -29,19 +29,19 @@ type RenderInternal struct { secureIDPrefix string } -func (r *RenderInternal) Init(output io.Writer) io.WriteCloser { +func (r *RenderInternal) Init(output io.Writer, extraHeadHTML template.HTML) io.WriteCloser { buf := make([]byte, 12) _, err := rand.Read(buf) if err != nil { panic("unable to generate secure id") } - return r.init(base64.URLEncoding.EncodeToString(buf), output) + return r.init(base64.URLEncoding.EncodeToString(buf), output, extraHeadHTML) } -func (r *RenderInternal) init(secID string, output io.Writer) io.WriteCloser { +func (r *RenderInternal) init(secID string, output io.Writer, extraHeadHTML template.HTML) io.WriteCloser { r.secureID = secID r.secureIDPrefix = r.secureID + ":" - return &finalProcessor{renderInternal: r, output: output} + return &finalProcessor{renderInternal: r, output: output, extraHeadHTML: extraHeadHTML} } func (r *RenderInternal) RecoverProtectedValue(v string) (string, bool) { diff --git a/modules/markup/render.go b/modules/markup/render.go index 79f1f473c2..c645749065 100644 --- a/modules/markup/render.go +++ b/modules/markup/render.go @@ -6,12 +6,14 @@ package markup import ( "context" "fmt" + "html/template" "io" "net/url" "strconv" "strings" "time" + "code.gitea.io/gitea/modules/htmlutil" "code.gitea.io/gitea/modules/markup/internal" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" @@ -120,31 +122,38 @@ func (ctx *RenderContext) WithHelper(helper RenderHelper) *RenderContext { return ctx } -// Render renders markup file to HTML with all specific handling stuff. -func Render(ctx *RenderContext, input io.Reader, output io.Writer) error { +// FindRendererByContext finds renderer by RenderContext +// TODO: it should be merged with other similar functions like GetRendererByFileName, DetectMarkupTypeByFileName, etc +func FindRendererByContext(ctx *RenderContext) (Renderer, error) { if ctx.RenderOptions.MarkupType == "" && ctx.RenderOptions.RelativePath != "" { ctx.RenderOptions.MarkupType = DetectMarkupTypeByFileName(ctx.RenderOptions.RelativePath) if ctx.RenderOptions.MarkupType == "" { - return util.NewInvalidArgumentErrorf("unsupported file to render: %q", ctx.RenderOptions.RelativePath) + return nil, util.NewInvalidArgumentErrorf("unsupported file to render: %q", ctx.RenderOptions.RelativePath) } } renderer := renderers[ctx.RenderOptions.MarkupType] if renderer == nil { - return util.NewInvalidArgumentErrorf("unsupported markup type: %q", ctx.RenderOptions.MarkupType) + return nil, util.NewNotExistErrorf("unsupported markup type: %q", ctx.RenderOptions.MarkupType) } - if ctx.RenderOptions.RelativePath != "" { - if externalRender, ok := renderer.(ExternalRenderer); ok && externalRender.DisplayInIFrame() { - if !ctx.RenderOptions.InStandalonePage { - // for an external "DisplayInIFrame" render, it could only output its content in a standalone page - // otherwise, a `, - setting.AppSubURL, +func renderIFrame(ctx *RenderContext, sandbox string, output io.Writer) error { + src := fmt.Sprintf("%s/%s/%s/render/%s/%s", setting.AppSubURL, url.PathEscape(ctx.RenderOptions.Metas["user"]), url.PathEscape(ctx.RenderOptions.Metas["repo"]), - ctx.RenderOptions.Metas["RefTypeNameSubURL"], - url.PathEscape(ctx.RenderOptions.RelativePath), - )) + util.PathEscapeSegments(ctx.RenderOptions.Metas["RefTypeNameSubURL"]), + util.PathEscapeSegments(ctx.RenderOptions.RelativePath), + ) + + var sandboxAttrValue template.HTML + if sandbox != "" { + sandboxAttrValue = htmlutil.HTMLFormat(`sandbox="%s"`, sandbox) + } + iframe := htmlutil.HTMLFormat(``, src, sandboxAttrValue) + _, err := io.WriteString(output, string(iframe)) return err } @@ -185,13 +190,34 @@ func pipes() (io.ReadCloser, io.WriteCloser, func()) { } } -func render(ctx *RenderContext, renderer Renderer, input io.Reader, output io.Writer) error { +func getExternalRendererOptions(renderer Renderer) (ret ExternalRendererOptions, _ bool) { + if externalRender, ok := renderer.(ExternalRenderer); ok { + return externalRender.GetExternalRendererOptions(), true + } + return ret, false +} + +func RenderWithRenderer(ctx *RenderContext, renderer Renderer, input io.Reader, output io.Writer) error { + var extraHeadHTML template.HTML + if extOpts, ok := getExternalRendererOptions(renderer); ok && extOpts.DisplayInIframe { + if !ctx.RenderOptions.InStandalonePage { + // for an external "DisplayInIFrame" render, it could only output its content in a standalone page + // otherwise, a