diff --git a/.golangci.yml b/.golangci.yml index 2ad39fbae2..483843bc55 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -153,6 +153,7 @@ linters: text: '(?i)exitAfterDefer:' paths: - node_modules + - .venv - public - web_src - third_party$ @@ -172,6 +173,7 @@ formatters: generated: lax paths: - node_modules + - .venv - public - web_src - third_party$ diff --git a/Makefile b/Makefile index fc507367e7..e81dab7f6c 100644 --- a/Makefile +++ b/Makefile @@ -31,11 +31,11 @@ XGO_VERSION := go-1.25.x AIR_PACKAGE ?= github.com/air-verse/air@v1 EDITORCONFIG_CHECKER_PACKAGE ?= github.com/editorconfig-checker/editorconfig-checker/v3/cmd/editorconfig-checker@v3 -GOFUMPT_PACKAGE ?= mvdan.cc/gofumpt@v0.9.1 -GOLANGCI_LINT_PACKAGE ?= github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.4.0 +GOFUMPT_PACKAGE ?= mvdan.cc/gofumpt@v0.9.2 +GOLANGCI_LINT_PACKAGE ?= github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.5.0 GXZ_PACKAGE ?= github.com/ulikunitz/xz/cmd/gxz@v0.5.15 MISSPELL_PACKAGE ?= github.com/golangci/misspell/cmd/misspell@v0.7.0 -SWAGGER_PACKAGE ?= github.com/go-swagger/go-swagger/cmd/swagger@717e3cb29becaaf00e56953556c6d80f8a01b286 +SWAGGER_PACKAGE ?= github.com/go-swagger/go-swagger/cmd/swagger@v0.33.1 XGO_PACKAGE ?= src.techknowlogick.com/xgo@latest GO_LICENSES_PACKAGE ?= github.com/google/go-licenses@v1 GOVULNCHECK_PACKAGE ?= golang.org/x/vuln/cmd/govulncheck@v1 @@ -258,7 +258,7 @@ clean: ## delete backend and integration files .PHONY: fmt fmt: ## format the Go and template code - @GOFUMPT_PACKAGE=$(GOFUMPT_PACKAGE) $(GO) run build/code-batch-process.go gitea-fmt -w '{file-list}' + @GOFUMPT_PACKAGE=$(GOFUMPT_PACKAGE) $(GO) run tools/code-batch-process.go gitea-fmt -w '{file-list}' $(eval TEMPLATES := $(shell find templates -type f -name '*.tmpl')) @# strip whitespace after '{{' or '(' and before '}}' or ')' unless there is only @# whitespace before it @@ -472,7 +472,7 @@ test\#%: coverage: grep '^\(mode: .*\)\|\(.*:[0-9]\+\.[0-9]\+,[0-9]\+\.[0-9]\+ [0-9]\+ [0-9]\+\)$$' coverage.out > coverage-bodged.out grep '^\(mode: .*\)\|\(.*:[0-9]\+\.[0-9]\+,[0-9]\+\.[0-9]\+ [0-9]\+ [0-9]\+\)$$' integration.coverage.out > integration.coverage-bodged.out - $(GO) run build/gocovmerge.go integration.coverage-bodged.out coverage-bodged.out > coverage.all + $(GO) run tools/gocovmerge.go integration.coverage-bodged.out coverage-bodged.out > coverage.all .PHONY: unit-test-coverage unit-test-coverage: 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/eslint.config.ts b/eslint.config.ts index 02aacefca2..d9c4bcae3a 100644 --- a/eslint.config.ts +++ b/eslint.config.ts @@ -49,6 +49,7 @@ export default defineConfig([ }, linterOptions: { reportUnusedDisableDirectives: 2, + reportUnusedInlineConfigs: 2, }, plugins: { '@eslint-community/eslint-comments': comments, diff --git a/modules/git/notes_test.go b/modules/git/notes_test.go index 7db2dbc0b9..5abb68b102 100644 --- a/modules/git/notes_test.go +++ b/modules/git/notes_test.go @@ -47,5 +47,5 @@ func TestGetNonExistentNotes(t *testing.T) { note := Note{} err = GetNote(t.Context(), bareRepo1, "non_existent_sha", ¬e) assert.Error(t, err) - assert.IsType(t, ErrNotExist{}, err) + assert.ErrorAs(t, err, &ErrNotExist{}) } 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