Merge remote-tracking branch 'upstream/main' into limit-repo-size

modules/context/api.go

@@ -20,6 +20,8 @@ import (
 	"code.gitea.io/gitea/modules/httpcache"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/web"
+	web_types "code.gitea.io/gitea/modules/web/types"
 
 	"gitea.com/go-chi/cache"
 )
@@ -41,6 +43,12 @@ type APIContext struct {
 	Package *Package
 }
 
+func init() {
+	web.RegisterResponseStatusProvider[*APIContext](func(req *http.Request) web_types.ResponseStatusProvider {
+		return req.Context().Value(apiContextKey).(*APIContext)
+	})
+}
+
 // Currently, we have the following common fields in error response:
 // * message: the message for end users (it shouldn't be used for error type detection)
 //            if we need to indicate some errors, we should introduce some new fields like ErrorCode or ErrorType

modules/context/base.go

@@ -96,7 +96,11 @@ func (b *Base) SetTotalCountHeader(total int64) {
 
 // Written returns true if there are something sent to web browser
 func (b *Base) Written() bool {
-	return b.Resp.Status() > 0
+	return b.Resp.WrittenStatus() != 0
+}
+
+func (b *Base) WrittenStatus() int {
+	return b.Resp.WrittenStatus()
 }
 
 // Status writes status code
@@ -136,6 +140,10 @@ func (b *Base) JSONRedirect(redirect string) {
 	b.JSON(http.StatusOK, map[string]any{"redirect": redirect})
 }
 
+func (b *Base) JSONError(msg string) {
+	b.JSON(http.StatusBadRequest, map[string]any{"errorMessage": msg})
+}
+
 // RemoteAddr returns the client machine ip address
 func (b *Base) RemoteAddr() string {
 	return b.Req.RemoteAddr

modules/context/context.go

@@ -21,7 +21,9 @@ import (
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/templates"
 	"code.gitea.io/gitea/modules/translation"
+	"code.gitea.io/gitea/modules/web"
 	"code.gitea.io/gitea/modules/web/middleware"
+	web_types "code.gitea.io/gitea/modules/web/types"
 
 	"gitea.com/go-chi/cache"
 	"gitea.com/go-chi/session"
@@ -58,6 +60,12 @@ type Context struct {
 	Package *Package
 }
 
+func init() {
+	web.RegisterResponseStatusProvider[*Context](func(req *http.Request) web_types.ResponseStatusProvider {
+		return req.Context().Value(WebContextKey).(*Context)
+	})
+}
+
 // TrHTMLEscapeArgs runs ".Locale.Tr()" but pre-escapes all arguments with html.EscapeString.
 // This is useful if the locale message is intended to only produce HTML content.
 func (ctx *Context) TrHTMLEscapeArgs(msg string, args ...string) string {

modules/context/context_response.go

@@ -16,6 +16,7 @@ import (
 
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/base"
+	"code.gitea.io/gitea/modules/httplib"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/templates"
@@ -49,14 +50,7 @@ func (ctx *Context) RedirectToFirst(location ...string) {
 			continue
 		}
 
-		// Unfortunately browsers consider a redirect Location with preceding "//", "\\" and "/\" as meaning redirect to "http(s)://REST_OF_PATH"
-		// Therefore we should ignore these redirect locations to prevent open redirects
-		if len(loc) > 1 && (loc[0] == '/' || loc[0] == '\\') && (loc[1] == '/' || loc[1] == '\\') {
-			continue
-		}
-
-		u, err := url.Parse(loc)
-		if err != nil || ((u.Scheme != "" || u.Host != "") && !strings.HasPrefix(strings.ToLower(loc), strings.ToLower(setting.AppURL))) {
+		if httplib.IsRiskyRedirectURL(loc) {
 			continue
 		}
 

modules/context/private.go

@@ -11,6 +11,8 @@ import (
 
 	"code.gitea.io/gitea/modules/graceful"
 	"code.gitea.io/gitea/modules/process"
+	"code.gitea.io/gitea/modules/web"
+	web_types "code.gitea.io/gitea/modules/web/types"
 )
 
 // PrivateContext represents a context for private routes
@@ -21,6 +23,12 @@ type PrivateContext struct {
 	Repo *Repository
 }
 
+func init() {
+	web.RegisterResponseStatusProvider[*PrivateContext](func(req *http.Request) web_types.ResponseStatusProvider {
+		return req.Context().Value(privateContextKey).(*PrivateContext)
+	})
+}
+
 // Deadline is part of the interface for context.Context and we pass this to the request context
 func (ctx *PrivateContext) Deadline() (deadline time.Time, ok bool) {
 	if ctx.Override != nil {

modules/context/response.go

@@ -5,15 +5,20 @@ package context
 
 import (
 	"net/http"
+
+	web_types "code.gitea.io/gitea/modules/web/types"
 )
 
 // ResponseWriter represents a response writer for HTTP
 type ResponseWriter interface {
 	http.ResponseWriter
 	http.Flusher
-	Status() int
+	web_types.ResponseStatusProvider
+
 	Before(func(ResponseWriter))
-	Size() int // used by access logger template
+
+	Status() int // used by access logger template
+	Size() int   // used by access logger template
 }
 
 var _ ResponseWriter = &Response{}
@@ -46,6 +51,10 @@ func (r *Response) Write(bs []byte) (int, error) {
 	return size, nil
 }
 
+func (r *Response) Status() int {
+	return r.status
+}
+
 func (r *Response) Size() int {
 	return r.written
 }
@@ -71,8 +80,8 @@ func (r *Response) Flush() {
 	}
 }
 
-// Status returned status code written
-func (r *Response) Status() int {
+// WrittenStatus returned status code written
+func (r *Response) WrittenStatus() int {
 	return r.status
 }
 

modules/git/sha1.go

@@ -28,6 +28,14 @@ func IsValidSHAPattern(sha string) bool {
 	return shaPattern.MatchString(sha)
 }
 
+type ErrInvalidSHA struct {
+	SHA string
+}
+
+func (err ErrInvalidSHA) Error() string {
+	return fmt.Sprintf("invalid sha: %s", err.SHA)
+}
+
 // MustID always creates a new SHA1 from a [20]byte array with no validation of input.
 func MustID(b []byte) SHA1 {
 	var id SHA1

modules/httplib/url.go

@@ -0,0 +1,27 @@
+// Copyright 2023 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package httplib
+
+import (
+	"net/url"
+	"strings"
+
+	"code.gitea.io/gitea/modules/setting"
+)
+
+// IsRiskyRedirectURL returns true if the URL is considered risky for redirects
+func IsRiskyRedirectURL(s string) bool {
+	// Unfortunately browsers consider a redirect Location with preceding "//", "\\", "/\" and "\/" as meaning redirect to "http(s)://REST_OF_PATH"
+	// Therefore we should ignore these redirect locations to prevent open redirects
+	if len(s) > 1 && (s[0] == '/' || s[0] == '\\') && (s[1] == '/' || s[1] == '\\') {
+		return true
+	}
+
+	u, err := url.Parse(s)
+	if err != nil || ((u.Scheme != "" || u.Host != "") && !strings.HasPrefix(strings.ToLower(s), strings.ToLower(setting.AppURL))) {
+		return true
+	}
+
+	return false
+}

modules/httplib/url_test.go

@@ -0,0 +1,38 @@
+// Copyright 2023 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package httplib
+
+import (
+	"testing"
+
+	"code.gitea.io/gitea/modules/setting"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestIsRiskyRedirectURL(t *testing.T) {
+	setting.AppURL = "http://localhost:3000/"
+	tests := []struct {
+		input string
+		want  bool
+	}{
+		{"", false},
+		{"foo", false},
+		{"/", false},
+		{"/foo?k=%20#abc", false},
+
+		{"//", true},
+		{"\\\\", true},
+		{"/\\", true},
+		{"\\/", true},
+		{"mail:a@b.com", true},
+		{"https://test.com", true},
+		{setting.AppURL + "/foo", false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.input, func(t *testing.T) {
+			assert.Equal(t, tt.want, IsRiskyRedirectURL(tt.input))
+		})
+	}
+}

modules/setting/mirror.go

@@ -30,7 +30,7 @@ func loadMirrorFrom(rootCfg ConfigProvider) {
 	// DEPRECATED should not be removed because users maybe upgrade from lower version to the latest version
 	// if these are removed, the warning will not be shown
 	deprecatedSetting(rootCfg, "repository", "DISABLE_MIRRORS", "mirror", "ENABLED", "v1.19.0")
-	if rootCfg.Section("repository").Key("DISABLE_MIRRORS").MustBool(false) {
+	if ConfigSectionKeyBool(rootCfg.Section("repository"), "DISABLE_MIRRORS") {
 		Mirror.DisableNewPull = true
 	}
 

modules/setting/oauth2.go

@@ -120,18 +120,20 @@ func loadOAuth2From(rootCfg ConfigProvider) {
 		OAuth2.JWTSigningPrivateKeyFile = filepath.Join(AppDataPath, OAuth2.JWTSigningPrivateKeyFile)
 	}
 
-	key := make([]byte, 32)
-	n, err := base64.RawURLEncoding.Decode(key, []byte(OAuth2.JWTSecretBase64))
-	if err != nil || n != 32 {
-		key, err = generate.NewJwtSecret()
-		if err != nil {
-			log.Fatal("error generating JWT secret: %v", err)
-		}
+	if InstallLock {
+		key := make([]byte, 32)
+		n, err := base64.RawURLEncoding.Decode(key, []byte(OAuth2.JWTSecretBase64))
+		if err != nil || n != 32 {
+			key, err = generate.NewJwtSecret()
+			if err != nil {
+				log.Fatal("error generating JWT secret: %v", err)
+			}
 
-		secretBase64 := base64.RawURLEncoding.EncodeToString(key)
-		rootCfg.Section("oauth2").Key("JWT_SECRET").SetValue(secretBase64)
-		if err := rootCfg.Save(); err != nil {
-			log.Fatal("save oauth2.JWT_SECRET failed: %v", err)
+			secretBase64 := base64.RawURLEncoding.EncodeToString(key)
+			rootCfg.Section("oauth2").Key("JWT_SECRET").SetValue(secretBase64)
+			if err := rootCfg.Save(); err != nil {
+				log.Fatal("save oauth2.JWT_SECRET failed: %v", err)
+			}
 		}
 	}
 }

modules/setting/setting.go

@@ -262,7 +262,7 @@ func loadRunModeFrom(rootCfg ConfigProvider) {
 	RunUser = rootSec.Key("RUN_USER").MustString(user.CurrentUsername())
 	// The following is a purposefully undocumented option. Please do not run Gitea as root. It will only cause future headaches.
 	// Please don't use root as a bandaid to "fix" something that is broken, instead the broken thing should instead be fixed properly.
-	unsafeAllowRunAsRoot := rootSec.Key("I_AM_BEING_UNSAFE_RUNNING_AS_ROOT").MustBool(false)
+	unsafeAllowRunAsRoot := ConfigSectionKeyBool(rootSec, "I_AM_BEING_UNSAFE_RUNNING_AS_ROOT")
 	RunMode = os.Getenv("GITEA_RUN_MODE")
 	if RunMode == "" {
 		RunMode = rootSec.Key("RUN_MODE").MustString("prod")

modules/test/context_tests.go

@@ -9,6 +9,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"net/url"
+	"strings"
 	"testing"
 
 	access_model "code.gitea.io/gitea/models/perm/access"
@@ -25,19 +26,26 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-// MockContext mock context for unit tests
-// TODO: move this function to other packages, because it depends on "models" package
-func MockContext(t *testing.T, path string) *context.Context {
-	resp := httptest.NewRecorder()
+func mockRequest(t *testing.T, reqPath string) *http.Request {
+	method, path, found := strings.Cut(reqPath, " ")
+	if !found {
+		method = "GET"
+		path = reqPath
+	}
 	requestURL, err := url.Parse(path)
 	assert.NoError(t, err)
-	req := &http.Request{
-		URL:  requestURL,
-		Form: url.Values{},
-	}
+	req := &http.Request{Method: method, URL: requestURL, Form: url.Values{}}
+	req = req.WithContext(middleware.WithContextData(req.Context()))
+	return req
+}
 
+// MockContext mock context for unit tests
+// TODO: move this function to other packages, because it depends on "models" package
+func MockContext(t *testing.T, reqPath string) (*context.Context, *httptest.ResponseRecorder) {
+	resp := httptest.NewRecorder()
+	req := mockRequest(t, reqPath)
 	base, baseCleanUp := context.NewBaseContext(resp, req)
-	base.Data = middleware.ContextData{}
+	base.Data = middleware.GetContextData(req.Context())
 	base.Locale = &translation.MockLocale{}
 	ctx := &context.Context{
 		Base:   base,
@@ -48,29 +56,23 @@ func MockContext(t *testing.T, path string) *context.Context {
 
 	chiCtx := chi.NewRouteContext()
 	ctx.Base.AppendContextValue(chi.RouteCtxKey, chiCtx)
-	return ctx
+	return ctx, resp
 }
 
 // MockAPIContext mock context for unit tests
 // TODO: move this function to other packages, because it depends on "models" package
-func MockAPIContext(t *testing.T, path string) *context.APIContext {
+func MockAPIContext(t *testing.T, reqPath string) (*context.APIContext, *httptest.ResponseRecorder) {
 	resp := httptest.NewRecorder()
-	requestURL, err := url.Parse(path)
-	assert.NoError(t, err)
-	req := &http.Request{
-		URL:  requestURL,
-		Form: url.Values{},
-	}
-
+	req := mockRequest(t, reqPath)
 	base, baseCleanUp := context.NewBaseContext(resp, req)
-	base.Data = middleware.ContextData{}
+	base.Data = middleware.GetContextData(req.Context())
 	base.Locale = &translation.MockLocale{}
 	ctx := &context.APIContext{Base: base}
 	_ = baseCleanUp // during test, it doesn't need to do clean up. TODO: this can be improved later
 
 	chiCtx := chi.NewRouteContext()
 	ctx.Base.AppendContextValue(chi.RouteCtxKey, chiCtx)
-	return ctx
+	return ctx, resp
 }
 
 // LoadRepo load a repo into a test context.

modules/test/utils.go

@@ -5,12 +5,29 @@ package test
 
 import (
 	"net/http"
+	"net/http/httptest"
 	"strings"
+
+	"code.gitea.io/gitea/modules/json"
 )
 
 // RedirectURL returns the redirect URL of a http response.
+// It also works for JSONRedirect: `{"redirect": "..."}`
 func RedirectURL(resp http.ResponseWriter) string {
-	return resp.Header().Get("Location")
+	loc := resp.Header().Get("Location")
+	if loc != "" {
+		return loc
+	}
+	if r, ok := resp.(*httptest.ResponseRecorder); ok {
+		m := map[string]any{}
+		err := json.Unmarshal(r.Body.Bytes(), &m)
+		if err == nil {
+			if loc, ok := m["redirect"].(string); ok {
+				return loc
+			}
+		}
+	}
+	return ""
 }
 
 func IsNormalPageCompleted(s string) bool {

modules/web/handler.go

@@ -9,25 +9,15 @@ import (
 	"net/http"
 	"reflect"
 
-	"code.gitea.io/gitea/modules/context"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/web/routing"
+	"code.gitea.io/gitea/modules/web/types"
 )
 
-// ResponseStatusProvider is an interface to check whether the response has been written by the handler
-type ResponseStatusProvider interface {
-	Written() bool
-}
+var responseStatusProviders = map[reflect.Type]func(req *http.Request) types.ResponseStatusProvider{}
 
-// TODO: decouple this from the context package, let the context package register these providers
-var argTypeProvider = map[reflect.Type]func(req *http.Request) ResponseStatusProvider{
-	reflect.TypeOf(&context.APIContext{}):     func(req *http.Request) ResponseStatusProvider { return context.GetAPIContext(req) },
-	reflect.TypeOf(&context.Context{}):        func(req *http.Request) ResponseStatusProvider { return context.GetWebContext(req) },
-	reflect.TypeOf(&context.PrivateContext{}): func(req *http.Request) ResponseStatusProvider { return context.GetPrivateContext(req) },
-}
-
-func RegisterHandleTypeProvider[T any](fn func(req *http.Request) ResponseStatusProvider) {
-	argTypeProvider[reflect.TypeOf((*T)(nil)).Elem()] = fn
+func RegisterResponseStatusProvider[T any](fn func(req *http.Request) types.ResponseStatusProvider) {
+	responseStatusProviders[reflect.TypeOf((*T)(nil)).Elem()] = fn
 }
 
 // responseWriter is a wrapper of http.ResponseWriter, to check whether the response has been written
@@ -36,10 +26,10 @@ type responseWriter struct {
 	status     int
 }
 
-var _ ResponseStatusProvider = (*responseWriter)(nil)
+var _ types.ResponseStatusProvider = (*responseWriter)(nil)
 
-func (r *responseWriter) Written() bool {
-	return r.status > 0
+func (r *responseWriter) WrittenStatus() int {
+	return r.status
 }
 
 func (r *responseWriter) Header() http.Header {
@@ -68,7 +58,7 @@ var (
 func preCheckHandler(fn reflect.Value, argsIn []reflect.Value) {
 	hasStatusProvider := false
 	for _, argIn := range argsIn {
-		if _, hasStatusProvider = argIn.Interface().(ResponseStatusProvider); hasStatusProvider {
+		if _, hasStatusProvider = argIn.Interface().(types.ResponseStatusProvider); hasStatusProvider {
 			break
 		}
 	}
@@ -101,7 +91,7 @@ func prepareHandleArgsIn(resp http.ResponseWriter, req *http.Request, fn reflect
 		case httpReqType:
 			argsIn[i] = reflect.ValueOf(req)
 		default:
-			if argFn, ok := argTypeProvider[argTyp]; ok {
+			if argFn, ok := responseStatusProviders[argTyp]; ok {
 				if isPreCheck {
 					argsIn[i] = reflect.ValueOf(&responseWriter{})
 				} else {
@@ -129,8 +119,8 @@ func handleResponse(fn reflect.Value, ret []reflect.Value) goctx.CancelFunc {
 
 func hasResponseBeenWritten(argsIn []reflect.Value) bool {
 	for _, argIn := range argsIn {
-		if statusProvider, ok := argIn.Interface().(ResponseStatusProvider); ok {
-			if statusProvider.Written() {
+		if statusProvider, ok := argIn.Interface().(types.ResponseStatusProvider); ok {
+			if statusProvider.WrittenStatus() != 0 {
 				return true
 			}
 		}
@@ -161,7 +151,7 @@ func toHandlerProvider(handler any) func(next http.Handler) http.Handler {
 		return http.HandlerFunc(func(respOrig http.ResponseWriter, req *http.Request) {
 			// wrap the response writer to check whether the response has been written
 			resp := respOrig
-			if _, ok := resp.(ResponseStatusProvider); !ok {
+			if _, ok := resp.(types.ResponseStatusProvider); !ok {
 				resp = &responseWriter{respWriter: resp}
 			}
 

modules/web/middleware/data.go

@@ -17,7 +17,7 @@ type ContextDataStore interface {
 
 type ContextData map[string]any
 
-func (ds ContextData) GetData() map[string]any {
+func (ds ContextData) GetData() ContextData {
 	return ds
 }
 

modules/web/route.go

@@ -7,31 +7,31 @@ import (
 	"net/http"
 	"strings"
 
-	"code.gitea.io/gitea/modules/context"
 	"code.gitea.io/gitea/modules/web/middleware"
 
 	"gitea.com/go-chi/binding"
-	chi "github.com/go-chi/chi/v5"
+	"github.com/go-chi/chi/v5"
 )
 
-// Bind binding an obj to a handler
-func Bind[T any](_ T) any {
-	return func(ctx *context.Context) {
+// Bind binding an obj to a handler's context data
+func Bind[T any](_ T) http.HandlerFunc {
+	return func(resp http.ResponseWriter, req *http.Request) {
 		theObj := new(T) // create a new form obj for every request but not use obj directly
-		binding.Bind(ctx.Req, theObj)
-		SetForm(ctx, theObj)
-		middleware.AssignForm(theObj, ctx.Data)
+		data := middleware.GetContextData(req.Context())
+		binding.Bind(req, theObj)
+		SetForm(data, theObj)
+		middleware.AssignForm(theObj, data)
 	}
 }
 
 // SetForm set the form object
-func SetForm(data middleware.ContextDataStore, obj interface{}) {
-	data.GetData()["__form"] = obj
+func SetForm(dataStore middleware.ContextDataStore, obj interface{}) {
+	dataStore.GetData()["__form"] = obj
 }
 
 // GetForm returns the validate form information
-func GetForm(data middleware.ContextDataStore) interface{} {
-	return data.GetData()["__form"]
+func GetForm(dataStore middleware.ContextDataStore) interface{} {
+	return dataStore.GetData()["__form"]
 }
 
 // Route defines a route based on chi's router

modules/web/routing/logger.go

@@ -8,8 +8,8 @@ import (
 	"strings"
 	"time"
 
-	"code.gitea.io/gitea/modules/context"
 	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/web/types"
 )
 
 // NewLoggerHandler is a handler that will log routing to the router log taking account of
@@ -86,8 +86,8 @@ func logPrinter(logger log.Logger) func(trigger Event, record *requestRecord) {
 		}
 
 		var status int
-		if v, ok := record.responseWriter.(context.ResponseWriter); ok {
-			status = v.Status()
+		if v, ok := record.responseWriter.(types.ResponseStatusProvider); ok {
+			status = v.WrittenStatus()
 		}
 		logf := log.Info
 		if strings.HasPrefix(req.RequestURI, "/assets/") {

modules/web/types/response.go

@@ -0,0 +1,10 @@
+// Copyright 2023 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package types
+
+// ResponseStatusProvider is an interface to get the written status in the response
+// Many packages need this interface, so put it in the separate package to avoid import cycle
+type ResponseStatusProvider interface {
+	WrittenStatus() int
+}