refactor: markup render (#38864) (#38869)

backport #38864

1. add missing CSP header to api & web render endpoints.
2. make jupyter render skip post-processors, nothing to process

modules/markup/external/frontend.go

@@ -21,19 +21,12 @@ type frontendRenderer struct {
 	patterns []string
 }
 
-var (
+var _ markup.ExternalRenderer = (*frontendRenderer)(nil)
-	_ markup.PostProcessRenderer = (*frontendRenderer)(nil)
-	_ markup.ExternalRenderer    = (*frontendRenderer)(nil)
-)
 
 func (p *frontendRenderer) Name() string {
 	return p.name
 }
 
-func (p *frontendRenderer) NeedPostProcess() bool {
-	return false
-}
-
 func (p *frontendRenderer) FileNamePatterns() []string {
 	// TODO: the file extensions are ambiguous, even if the file name matches, it doesn't mean that the file is a 3D model
 	// There are some approaches to make it more accurate, but they are all complicated:

modules/markup/jupyter/jupyter.go

@@ -29,9 +29,8 @@ func init() {
 type renderer struct{}
 
 var (
-	_ markup.Renderer            = (*renderer)(nil)
+	_ markup.Renderer         = (*renderer)(nil)
-	_ markup.PostProcessRenderer = (*renderer)(nil)
+	_ markup.ExternalRenderer = (*renderer)(nil) // FIXME: this is not an external render, need to refactor the framework in the future
-	_ markup.ExternalRenderer    = (*renderer)(nil) // FIXME: this is not an external render, need to refactor the framework in the future
 )
 
 type mimeHandler struct {
@@ -96,8 +95,6 @@ func (renderer) Name() string {
 	return "jupyter-render"
 }
 
-func (renderer) NeedPostProcess() bool { return true }
-
 func (renderer) GetExternalRendererOptions() markup.ExternalRendererOptions {
 	return markup.ExternalRendererOptions{
 		// HINT: no need to let markup render sanitize the output because there are many special CSS class names, inline attributes.

modules/markup/jupyter/jupyter_test.go

@@ -274,7 +274,7 @@ func TestIntegrationAndSanitization(t *testing.T) {
 						"execution_count": 1,
 						"data": {
 							"text/html": [
-								"<div><script>alert('XSS Vector')</script><table class=\"dataframe\"><tr><td>Safe Content</td></tr></table></div>"
+								"<div><script>foo</script><table class=other><tr><td>[[name=no-post-process|link=/link]]</td></tr></table></div>"
 							]
 						},
 						"metadata": {}
@@ -304,7 +304,7 @@ func TestIntegrationAndSanitization(t *testing.T) {
 			<div class="cell-left cell-prompt">Out [1]:</div>
 			<div class="cell-right cell-output">
 				<div class="cell-output-html">
-					<div><table><tbody><tr><td>Safe Content</td></tr></tbody></table></div>
+					<div><table><tr><td>[[name=no-post-process|link=/link]]</td></tr></table></div>
 				</div>
 			</div>
 		</div>

modules/markup/main_test.go

@@ -14,7 +14,6 @@ import (
 func TestMain(m *testing.M) {
 	setting.IsInTesting = true
 	markup.RenderBehaviorForTesting.DisableAdditionalAttributes = true
-	setting.Markdown.FileNamePatterns = []string{"*.md"}
 	markup.RefreshFileNamePatterns()
 	os.Exit(m.Run())
 }

modules/setting/markup.go

@@ -50,7 +50,8 @@ var Markdown = struct {
 	MathCodeBlockDetection []string
 	MathCodeBlockOptions   MarkdownMathCodeBlockOptions `ini:"-"`
 }{
-	EnableMath: true,
+	EnableMath:       true,
+	FileNamePatterns: []string{"*.md"},
 }
 
 // MarkupRenderer defines the external parser configured in ini

routers/api/v1/misc/markup.go

@@ -4,8 +4,9 @@
 package misc
 
 import (
-	"gitea.dev/modules/markup"
+	"io"
-	"gitea.dev/modules/markup/markdown"
+
+	"gitea.dev/modules/setting"
 	api "gitea.dev/modules/structs"
 	"gitea.dev/modules/util"
 	"gitea.dev/modules/web"
@@ -84,9 +85,6 @@ func MarkdownRaw(ctx *context.APIContext) {
 	//     "$ref": "#/responses/MarkdownRender"
 	//   "422":
 	//     "$ref": "#/responses/validationError"
-	defer ctx.Req.Body.Close()
+	textBytes, _ := io.ReadAll(io.LimitReader(ctx.Req.Body, setting.UI.MaxDisplayFileSize))
-	if err := markdown.RenderRaw(markup.NewRenderContext(ctx), ctx.Req.Body, ctx.Resp); err != nil {
+	common.RenderMarkup(ctx.Base, ctx.Repo, "markdown", util.UnsafeBytesToString(textBytes), "", "")
-		ctx.APIErrorInternal(err)
-		return
-	}
 }

routers/api/v1/misc/markup_test.go

@@ -7,19 +7,15 @@ import (
 	go_context "context"
 	"io"
 	"net/http"
-	"os"
 	"path"
 	"strings"
 	"testing"
 
-	repo_model "gitea.dev/models/repo"
-	"gitea.dev/models/unittest"
 	"gitea.dev/modules/markup"
 	"gitea.dev/modules/setting"
 	api "gitea.dev/modules/structs"
 	"gitea.dev/modules/test"
 	"gitea.dev/modules/web"
-	context_service "gitea.dev/services/context"
 	"gitea.dev/services/contexttest"
 
 	"github.com/stretchr/testify/assert"
@@ -27,13 +23,6 @@ import (
 
 const AppURL = "http://localhost:3000/"
 
-func TestMain(m *testing.M) {
-	unittest.MainTest(m, &unittest.TestOptions{
-		FixtureFiles: []string{"repository.yml", "user.yml"},
-	})
-	os.Exit(m.Run())
-}
-
 func testRenderMarkup(t *testing.T, mode string, wiki bool, filePath, text, expectedBody string, expectedCode int) {
 	setting.AppURL = AppURL
 	defer test.MockVariableValue(&markup.RenderBehaviorForTesting.DisableAdditionalAttributes, true)()
@@ -49,13 +38,11 @@ func testRenderMarkup(t *testing.T, mode string, wiki bool, filePath, text, expe
 		FilePath: filePath,
 	}
 	ctx, resp := contexttest.MockAPIContext(t, "POST /api/v1/markup")
-	ctx.Repo = &context_service.Repository{}
-	ctx.Repo.Repository = unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
 	web.SetForm(ctx, &options)
 	Markup(ctx)
 	assert.Equal(t, expectedBody, resp.Body.String())
 	assert.Equal(t, expectedCode, resp.Code)
-	resp.Body.Reset()
+	assert.Contains(t, resp.Header().Get("Content-Security-Policy"), "script-src * 'nonce-")
 }
 
 func testRenderMarkdown(t *testing.T, mode string, wiki bool, text, responseBody string, responseCode int) {
@@ -76,11 +63,10 @@ func testRenderMarkdown(t *testing.T, mode string, wiki bool, text, responseBody
 	Markdown(ctx)
 	assert.Equal(t, responseBody, resp.Body.String())
 	assert.Equal(t, responseCode, resp.Code)
-	resp.Body.Reset()
+	assert.Contains(t, resp.Header().Get("Content-Security-Policy"), "script-src * 'nonce-")
 }
 
 func TestAPI_RenderGFM(t *testing.T) {
-	unittest.PrepareTestEnv(t)
 	markup.Init(&markup.RenderHelperFuncs{
 		IsUsernameMentionable: func(ctx go_context.Context, username string) bool {
 			return username == "r-lyeh"

routers/common/errpage.go

@@ -40,6 +40,7 @@ func renderServerErrorPage(w http.ResponseWriter, req *http.Request, respCode in
 	if acceptsHTML {
 		err := templates.PageRenderer().HTML(outBuf, respCode, tmpl, ctxData, tmplCtx)
 		if err != nil {
+			log.Error("Failed to render error page template %s: %v", tmpl, err)
 			_, _ = w.Write([]byte("Internal server error but failed to render error page template, please collect error logs and report to Gitea issue tracker"))
 			return
 		}

routers/common/markup.go

@@ -31,6 +31,8 @@ func RenderMarkup(ctx *context.Base, ctxRepo *context.Repository, mode, text, ur
 	// for example, when previewing file "/gitea/owner/repo/src/branch/features/feat-123/doc/CHANGE.md", then filePath is "doc/CHANGE.md"
 	// and the urlPathContext is "/gitea/owner/repo/src/branch/features/feat-123/doc"
 
+	ctx.SetHeaderContentSecurityPolicyGeneral()
+
 	if mode == "" || mode == "markdown" {
 		// raw Markdown doesn't do any special handling
 		// TODO: raw markdown doesn't do any link processing, so "urlPathContext" doesn't take effect

routers/web/repo/render.go

@@ -64,6 +64,9 @@ func RenderFile(ctx *context.Context) {
 	extRendererOpts := extRenderer.GetExternalRendererOptions()
 	if extRendererOpts.ContentSandbox != "" {
 		ctx.Resp.Header().Add("Content-Security-Policy", "sandbox "+extRendererOpts.ContentSandbox)
+	} else {
+		// if no sandbox, just apply the same CSP as a general Gitea web page
+		ctx.SetHeaderContentSecurityPolicyGeneral()
 	}
 
 	err = markup.RenderWithRenderer(rctx, renderer, rendererInput, ctx.Resp)

services/context/base.go

@@ -188,6 +188,26 @@ func (b *Base) TrN(cnt any, key1, keyN string, args ...any) template.HTML {
 	return b.Locale.TrN(cnt, key1, keyN, args...)
 }
 
+func CspScriptNonce(ctx reqctx.RequestContext) (ret string) {
+	// Generate a random nonce for each request and cache it in the context to make it usable during the whole rendering process.
+	//
+	// Some "<script>" tags are not in the CSP context, so they don't need nonce,
+	// these tags are written as "<script nonce>" to help developers to know that "no script nonce attribute is missing"
+	// (e.g.: when they grep the codebase for "script" tags)
+	ret, _ = ctx.Value("_cspScriptNonce").(string)
+	if ret == "" {
+		ret = util.FastCryptoRandomHex(32) // 16 bytes / 128 bits entropy
+		ctx.SetContextValue("_cspScriptNonce", ret)
+	}
+	return ret
+}
+
+func (b *Base) SetHeaderContentSecurityPolicyGeneral() {
+	if csp := WebContentSecurityPolicy(CspScriptNonce(b)); csp != "" {
+		b.Resp.Header().Set("Content-Security-Policy", csp)
+	}
+}
+
 func NewBaseContext(resp http.ResponseWriter, req *http.Request) *Base {
 	reqCtx := reqctx.FromContext(req.Context())
 	b := &Base{

services/context/context_template.go

@@ -12,10 +12,11 @@ import (
 	"strings"
 	"time"
 
+	"gitea.dev/modules/htmlutil"
 	"gitea.dev/modules/httplib"
 	"gitea.dev/modules/public"
+	"gitea.dev/modules/reqctx"
 	"gitea.dev/modules/setting"
-	"gitea.dev/modules/util"
 	"gitea.dev/modules/web/middleware"
 	"gitea.dev/services/webtheme"
 )
@@ -24,16 +25,16 @@ type TemplateContext map[string]any
 
 var _ context.Context = TemplateContext(nil)
 
-func NewTemplateContext(ctx context.Context, req *http.Request) TemplateContext {
+func NewTemplateContext(ctx reqctx.RequestContext, req *http.Request) TemplateContext {
 	return TemplateContext{"_ctx": ctx, "_req": req}
 }
 
 func (c TemplateContext) req() *http.Request {
-	return c["_req"].(*http.Request)
+	return c["_req"].(*http.Request) //nolint:forcetypeassert // must exist
 }
 
-func (c TemplateContext) parentContext() context.Context {
+func (c TemplateContext) parentContext() reqctx.RequestContext {
-	return c["_ctx"].(context.Context)
+	return c["_ctx"].(reqctx.RequestContext) //nolint:forcetypeassert // must exist
 }
 
 func (c TemplateContext) Deadline() (deadline time.Time, ok bool) {
@@ -100,21 +101,10 @@ func (c TemplateContext) ScriptImport(path string, typ ...string) template.HTML
 }
 
 func (c TemplateContext) CspScriptNonce() (ret string) {
-	// Generate a random nonce for each request and cache it in the context to make it usable during the whole rendering process.
+	return CspScriptNonce(c.parentContext())
-	//
-	// Some "<script>" tags are not in the CSP context, so they don't need nonce,
-	// these tags are written as "<script nonce>" to help developers to know that "no script nonce attribute is missing"
-	// (e.g.: when they grep the codebase for "script" tags)
-
-	ret, _ = c["_cspScriptNonce"].(string)
-	if ret == "" {
-		ret = util.FastCryptoRandomHex(32) // 16 bytes / 128 bits entropy
-		c["_cspScriptNonce"] = ret
-	}
-	return ret
 }
 
-func (c TemplateContext) HeadMetaContentSecurityPolicy() template.HTML {
+func WebContentSecurityPolicy(scriptNonce string) string {
 	if setting.Security.ContentSecurityPolicyGeneral == "unset" {
 		return "" // if site admin disables the general CSP, then we don't use it
 	}
@@ -130,16 +120,24 @@ func (c TemplateContext) HeadMetaContentSecurityPolicy() template.HTML {
 	//    * Browsers will merge and use the stricter rules between Gitea and reverse proxy
 	// B. Introduce some config options in "app.ini"
 	//    * Maybe this approach should be avoided, don't make the config system too complex, just let users use A
-	return template.HTML(`<meta http-equiv="Content-Security-Policy" content="` +
+
-		// allow all by default (the same as old releases with no CSP)
+	// allow all by default (the same as old releases with no CSP)
-		// * maybe some images or markup (external) renders need "data:", need to investigate
+	// * maybe some images or markup (external) renders need "data:", need to investigate
-		// * avatar upload editor needs "blob:", at least "img-src" and "content-src"
+	// * avatar upload editor needs "blob:", at least "img-src" and "content-src"
-		`default-src * data: blob:;` +
+	return `default-src * data: blob:;` +
 
 		// enforce nonce for all scripts, disallow inline scripts
-		`script-src * 'nonce-` + c.CspScriptNonce() + `';` +
+		`script-src * 'nonce-` + scriptNonce + `';` +
 
 		// it seems that Vue needs the unsafe-inline, and our custom colors (e.g.: label) also need it
-		`style-src * 'unsafe-inline';` +
+		`style-src * 'unsafe-inline';`
-		`">`)
+}
+
+func (c TemplateContext) HeadMetaContentSecurityPolicy() template.HTML {
+	scriptNonce := c.CspScriptNonce()
+	csp := WebContentSecurityPolicy(scriptNonce)
+	if csp == "" {
+		return ""
+	}
+	return htmlutil.HTMLFormat(`<meta http-equiv="Content-Security-Policy" content="%s">`, csp)
 }

services/context/context_test.go

@@ -9,6 +9,7 @@ import (
 	"net/url"
 	"testing"
 
+	"gitea.dev/modules/reqctx"
 	"gitea.dev/modules/setting"
 	"gitea.dev/modules/test"
 
@@ -57,7 +58,7 @@ func TestAppFullLink(t *testing.T) {
 	defer test.MockVariableValue(&setting.PublicURLDetection, setting.PublicURLNever)()
 
 	req := httptest.NewRequest(http.MethodGet, "https://gitea.example.com/sub/", nil)
-	tmplCtx := NewTemplateContext(req.Context(), req)
+	tmplCtx := NewTemplateContext(reqctx.NewRequestContextForTest(req.Context()), req)
 
 	assert.Equal(t, "https://gitea.example.com/sub", string(tmplCtx.AppFullLink()))
 	assert.Equal(t, "https://gitea.example.com/sub/user/repo", string(tmplCtx.AppFullLink("user/repo")))

tests/integration/markup_external_test.go

@@ -127,7 +127,8 @@ func TestExternalMarkupRenderer(t *testing.T) {
 				req = NewRequest(t, "GET", "/user2/repo1/render/branch/master/bin.no-sanitizer")
 				respSub := MakeRequest(t, req, http.StatusOK)
 				assert.Equal(t, binaryContent, respSub.Body.String()) // raw content should keep the raw bytes (including invalid UTF-8 bytes), and no "external-render-iframe" helpers
-				assert.Empty(t, respSub.Header().Get("Content-Security-Policy"), "sandbox is disabled by RENDER_CONTENT_SANDBOX")
+				assert.NotContains(t, respSub.Header().Get("Content-Security-Policy"), "sandbox", "sandbox is disabled by RENDER_CONTENT_SANDBOX")
+				assert.Contains(t, respSub.Header().Get("Content-Security-Policy"), "nonce-", "it should have the general policies as a normal web page")
 			})
 
 			t.Run("HTMLContentWithExternalRenderIframeHelper", func(t *testing.T) {
@@ -139,7 +140,8 @@ func TestExternalMarkupRenderer(t *testing.T) {
 						`<script>foo("raw")</script>`,
 					respSub.Body.String(),
 				)
-				assert.Empty(t, respSub.Header().Get("Content-Security-Policy"))
+				assert.NotContains(t, respSub.Header().Get("Content-Security-Policy"), "sandbox")
+				assert.Contains(t, respSub.Header().Get("Content-Security-Policy"), "nonce-")
 			})
 		})
 	})