From bcd913a4e2eca5256a0946ba2db6bb57dfc27441 Mon Sep 17 00:00:00 2001 From: Elisei Roca Date: Wed, 2 Sep 2026 21:32:33 +0200 Subject: [PATCH] fix(httplib): prevent leaking localhost:3000 in public links (#39217) Co-authored-by: wxiaoguang --- modules/git/url/url_test.go | 6 +++-- modules/httplib/url.go | 31 +++++++++++++++++++--- modules/httplib/url_test.go | 42 ++++++++++++++++++++---------- routers/api/actions/artifacts.go | 13 ++++++--- routers/api/actions/artifactsv4.go | 5 +--- routers/common/middleware.go | 15 ++--------- services/context/api.go | 3 ++- services/context/base.go | 3 +-- services/context/context.go | 2 ++ 9 files changed, 76 insertions(+), 44 deletions(-) diff --git a/modules/git/url/url_test.go b/modules/git/url/url_test.go index 093eb53c19c..4f3280951ef 100644 --- a/modules/git/url/url_test.go +++ b/modules/git/url/url_test.go @@ -4,12 +4,12 @@ package url import ( - "context" "net/http" "net/url" "testing" "gitea.dev/modules/httplib" + "gitea.dev/modules/reqctx" "gitea.dev/modules/setting" "gitea.dev/modules/test" @@ -175,11 +175,13 @@ func TestParseRepositoryURL(t *testing.T) { defer test.MockVariableValue(&setting.AppURL, "https://localhost:3000")() defer test.MockVariableValue(&setting.SSH.Domain, "try.gitea.io")() + ctx := reqctx.NewRequestContextForTest(t) ctxURL, _ := url.Parse("https://gitea") ctxReq := &http.Request{URL: ctxURL, Header: http.Header{}} ctxReq.Host = ctxURL.Host ctxReq.Header.Add("X-Forwarded-Proto", ctxURL.Scheme) - ctx := context.WithValue(t.Context(), httplib.RequestContextKey, ctxReq) + httplib.RequestWithContext(ctxReq, ctx) + httplib.MarkRequestSupportPublicURL(ctx) cases := []struct { input string ownerName, repoName, remaining string diff --git a/modules/httplib/url.go b/modules/httplib/url.go index 0bc996a21ea..f83214966dd 100644 --- a/modules/httplib/url.go +++ b/modules/httplib/url.go @@ -10,13 +10,35 @@ import ( "net/url" "strings" + "gitea.dev/modules/reqctx" "gitea.dev/modules/setting" "gitea.dev/modules/util" ) -type RequestContextKeyStruct struct{} +type contextKeyType string -var RequestContextKey = RequestContextKeyStruct{} +var ( + contextKeyRequest = contextKeyType("request") + contextKeySupportPublicURL = contextKeyType("support-public-url") +) + +// RequestWithContext returns a request with the given context and adds a cleanup function to remove temporary files. +// It also sets the request in the context for later retrieval. +func RequestWithContext(req *http.Request, ctx reqctx.RequestContext) *http.Request { + req = req.WithContext(ctx) + ctx.AddCleanUp(func() { + if req.MultipartForm != nil { + _ = req.MultipartForm.RemoveAll() // remove the temp files buffered to tmp directory + } + }) + ctx.SetContextValue(contextKeyRequest, req) + return req +} + +// MarkRequestSupportPublicURL marks the request context to support public URL detection from request headers. +func MarkRequestSupportPublicURL(ctx reqctx.RequestContext) { + ctx.SetContextValue(contextKeySupportPublicURL, true) +} func urlIsRelative(s string, u *url.URL) bool { // Unfortunately, browsers consider a redirect Location with preceding "//", "\\", "/\" and "\/" as meaning redirect to "http(s)://REST_OF_PATH" @@ -80,7 +102,8 @@ func GuessCurrentAppURL(ctx context.Context) string { // GuessCurrentHostURL tries to guess the current full host URL (no sub-path) by http headers, there is no trailing slash. func GuessCurrentHostURL(ctx context.Context) string { // "never" means always trust ROOT_URL and skip any request header detection. - if setting.PublicURLDetection == setting.PublicURLNever { + detectPublicURL := setting.PublicURLDetection != setting.PublicURLNever && ctx.Value(contextKeySupportPublicURL) == true + if !detectPublicURL { return strings.TrimSuffix(setting.AppURL, setting.AppSubURL+"/") } // Try the best guess to get the current host URL (will be used for public URL) by http headers. @@ -92,7 +115,7 @@ func GuessCurrentHostURL(ctx context.Context) string { // Without more information, Gitea is impossible to distinguish between case 2 and case 3, then case 2 would result in // wrong guess like guessed public URL becomes "http://gitea:3000/" behind a "https" reverse proxy, which is not accessible by end users. // So we introduced "PUBLIC_URL_DETECTION" option, to control the guessing behavior to satisfy different use cases. - req, ok := ctx.Value(RequestContextKey).(*http.Request) + req, ok := ctx.Value(contextKeyRequest).(*http.Request) if !ok { return strings.TrimSuffix(setting.AppURL, setting.AppSubURL+"/") } diff --git a/modules/httplib/url_test.go b/modules/httplib/url_test.go index 8356213724a..0a481ddc719 100644 --- a/modules/httplib/url_test.go +++ b/modules/httplib/url_test.go @@ -9,8 +9,10 @@ import ( "net/http" "testing" + "gitea.dev/modules/reqctx" "gitea.dev/modules/setting" "gitea.dev/modules/test" + "gitea.dev/modules/util" "github.com/stretchr/testify/assert" ) @@ -43,6 +45,15 @@ func TestIsRelativeURL(t *testing.T) { } } +func testCtxWithPublicURL(t *testing.T, req *http.Request, supportPublicURL ...bool) context.Context { + reqCtx := reqctx.NewRequestContextForTest(t) + _ = RequestWithContext(req, reqCtx) + if util.OptionalArg(supportPublicURL, true) { + MarkRequestSupportPublicURL(reqCtx) + } + return reqCtx +} + func TestGuessCurrentHostURL(t *testing.T) { defer test.MockVariableValue(&setting.AppURL, "http://cfg-host/sub/")() defer test.MockVariableValue(&setting.AppSubURL, "/sub")() @@ -55,14 +66,14 @@ func TestGuessCurrentHostURL(t *testing.T) { assert.Equal(t, "http://cfg-host", GuessCurrentHostURL(t.Context())) // legacy: "Host" is not used when there is no "X-Forwarded-Proto" header - ctx := context.WithValue(t.Context(), RequestContextKey, &http.Request{Host: "req-host:3000"}) + ctx := testCtxWithPublicURL(t, &http.Request{Host: "req-host:3000"}) assert.Equal(t, "http://cfg-host", GuessCurrentHostURL(ctx)) // if "X-Forwarded-Proto" exists, then use it and "Host" header - ctx = context.WithValue(t.Context(), RequestContextKey, &http.Request{Host: "req-host:3000", Header: headersWithProto}) + ctx = testCtxWithPublicURL(t, &http.Request{Host: "req-host:3000", Header: headersWithProto}) assert.Equal(t, "https://req-host:3000", GuessCurrentHostURL(ctx)) - ctx = context.WithValue(t.Context(), RequestContextKey, &http.Request{Host: "req-host:3000", Header: maliciousProtoHeaders}) + ctx = testCtxWithPublicURL(t, &http.Request{Host: "req-host:3000", Header: maliciousProtoHeaders}) assert.Equal(t, "http://cfg-host", GuessCurrentHostURL(ctx)) }) @@ -72,17 +83,20 @@ func TestGuessCurrentHostURL(t *testing.T) { assert.Equal(t, "http://cfg-host", GuessCurrentHostURL(t.Context())) // auto: always use "Host" header, the scheme is determined by "X-Forwarded-Proto" header, or TLS config if no "X-Forwarded-Proto" header - ctx := context.WithValue(t.Context(), RequestContextKey, &http.Request{Host: "req-host:3000"}) + ctx := testCtxWithPublicURL(t, &http.Request{Host: "req-host:3000"}) assert.Equal(t, "http://req-host:3000", GuessCurrentHostURL(ctx)) - ctx = context.WithValue(t.Context(), RequestContextKey, &http.Request{Host: "req-host", TLS: &tls.ConnectionState{}}) + ctx = testCtxWithPublicURL(t, &http.Request{Host: "req-host", TLS: &tls.ConnectionState{}}) assert.Equal(t, "https://req-host", GuessCurrentHostURL(ctx)) - ctx = context.WithValue(t.Context(), RequestContextKey, &http.Request{Host: "req-host:3000", Header: headersWithProto}) + ctx = testCtxWithPublicURL(t, &http.Request{Host: "req-host:3000", Header: headersWithProto}) assert.Equal(t, "https://req-host:3000", GuessCurrentHostURL(ctx)) - ctx = context.WithValue(t.Context(), RequestContextKey, &http.Request{Host: "req-host:3000", Header: maliciousProtoHeaders}) + ctx = testCtxWithPublicURL(t, &http.Request{Host: "req-host:3000", Header: maliciousProtoHeaders}) assert.Equal(t, "http://req-host:3000", GuessCurrentHostURL(ctx)) + + ctx = testCtxWithPublicURL(t, &http.Request{Host: "req-host:3000", Header: maliciousProtoHeaders}, false) + assert.Equal(t, "http://cfg-host", GuessCurrentHostURL(ctx)) }) t.Run("Never", func(t *testing.T) { @@ -90,13 +104,13 @@ func TestGuessCurrentHostURL(t *testing.T) { assert.Equal(t, "http://cfg-host", GuessCurrentHostURL(t.Context())) - ctx := context.WithValue(t.Context(), RequestContextKey, &http.Request{Host: "req-host:3000"}) + ctx := testCtxWithPublicURL(t, &http.Request{Host: "req-host:3000"}) assert.Equal(t, "http://cfg-host", GuessCurrentHostURL(ctx)) - ctx = context.WithValue(t.Context(), RequestContextKey, &http.Request{Host: "req-host:3000", TLS: &tls.ConnectionState{}}) + ctx = testCtxWithPublicURL(t, &http.Request{Host: "req-host:3000", TLS: &tls.ConnectionState{}}) assert.Equal(t, "http://cfg-host", GuessCurrentHostURL(ctx)) - ctx = context.WithValue(t.Context(), RequestContextKey, &http.Request{Host: "req-host:3000", Header: headersWithProto}) + ctx = testCtxWithPublicURL(t, &http.Request{Host: "req-host:3000", Header: headersWithProto}) assert.Equal(t, "http://cfg-host", GuessCurrentHostURL(ctx)) }) } @@ -112,12 +126,12 @@ func TestMakeAbsoluteURL(t *testing.T) { assert.Equal(t, "http://cfg-host/foo", MakeAbsoluteURL(ctx, "/foo")) assert.Equal(t, "http://other/foo", MakeAbsoluteURL(ctx, "http://other/foo")) - ctx = context.WithValue(ctx, RequestContextKey, &http.Request{ + ctx = testCtxWithPublicURL(t, &http.Request{ Host: "user-host", }) assert.Equal(t, "http://cfg-host/foo", MakeAbsoluteURL(ctx, "/foo")) - ctx = context.WithValue(ctx, RequestContextKey, &http.Request{ + ctx = testCtxWithPublicURL(t, &http.Request{ Host: "user-host", Header: map[string][]string{ "X-Forwarded-Host": {"forwarded-host"}, @@ -125,7 +139,7 @@ func TestMakeAbsoluteURL(t *testing.T) { }) assert.Equal(t, "http://cfg-host/foo", MakeAbsoluteURL(ctx, "/foo")) - ctx = context.WithValue(ctx, RequestContextKey, &http.Request{ + ctx = testCtxWithPublicURL(t, &http.Request{ Host: "user-host", Header: map[string][]string{ "X-Forwarded-Host": {"forwarded-host"}, @@ -173,7 +187,7 @@ func TestIsCurrentGiteaSiteURL(t *testing.T) { assert.False(t, IsCurrentGiteaSiteURL(ctx, "http://localhost")) assert.True(t, IsCurrentGiteaSiteURL(ctx, "http://localhost:3000?key=val")) - ctx = context.WithValue(ctx, RequestContextKey, &http.Request{ + ctx = testCtxWithPublicURL(t, &http.Request{ Host: "user-host", Header: map[string][]string{ "X-Forwarded-Host": {"forwarded-host"}, diff --git a/routers/api/actions/artifacts.go b/routers/api/actions/artifacts.go index c9890072032..cc9d6cadeb0 100644 --- a/routers/api/actions/artifacts.go +++ b/routers/api/actions/artifacts.go @@ -130,13 +130,18 @@ func ArtifactsRoutes(prefix string) *web.Router { return m } +func newArtifactContext(resp http.ResponseWriter, req *http.Request) *ArtifactContext { + base := context.NewBaseContext(resp, req) + ctx := &ArtifactContext{Base: base} + ctx.SetContextValue(artifactContextKey, ctx) + httplib.MarkRequestSupportPublicURL(ctx) + return ctx +} + func ArtifactContexter() func(next http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { - base := context.NewBaseContext(resp, req) - - ctx := &ArtifactContext{Base: base} - ctx.SetContextValue(artifactContextKey, ctx) + ctx := newArtifactContext(resp, req) // action task call server api with Bearer ACTIONS_RUNTIME_TOKEN // we should verify the ACTIONS_RUNTIME_TOKEN diff --git a/routers/api/actions/artifactsv4.go b/routers/api/actions/artifactsv4.go index 65cc92cfaa5..0ea44ee57d9 100644 --- a/routers/api/actions/artifactsv4.go +++ b/routers/api/actions/artifactsv4.go @@ -114,7 +114,6 @@ import ( "gitea.dev/modules/util" "gitea.dev/modules/web" "gitea.dev/services/actions" - "gitea.dev/services/context" "google.golang.org/protobuf/encoding/protojson" "google.golang.org/protobuf/reflect/protoreflect" @@ -132,9 +131,7 @@ type artifactV4Routes struct { func ArtifactV4Contexter() func(next http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { - base := context.NewBaseContext(resp, req) - ctx := &ArtifactContext{Base: base} - ctx.SetContextValue(artifactContextKey, ctx) + ctx := newArtifactContext(resp, req) next.ServeHTTP(ctx.Resp, ctx.Req) }) } diff --git a/routers/common/middleware.go b/routers/common/middleware.go index a1ebc25c144..dcd9ea3ad63 100644 --- a/routers/common/middleware.go +++ b/routers/common/middleware.go @@ -87,19 +87,8 @@ func RequestContextHandler() func(h http.Handler) http.Handler { } }() - ds := reqctx.GetRequestDataStore(ctx) - req = req.WithContext(cache.WithCacheContext(ctx)) - ds.SetContextValue(httplib.RequestContextKey, req) - ds.AddCleanUp(func() { - // TODO: GOLANG-HTTP-TMPDIR: Golang saves the uploaded files to temp directory (TMPDIR) when parsing multipart-form. - // The "req" might have changed due to the new "req.WithContext" calls - // For example: in NewBaseContext, a new "req" with context is created, and the multipart-form is parsed there. - // So we always use the latest "req" from the data store. - ctxReq := ds.GetContextValue(httplib.RequestContextKey).(*http.Request) //nolint:forcetypeassert // must be valid - if ctxReq.MultipartForm != nil { - _ = ctxReq.MultipartForm.RemoveAll() // remove the temp files buffered to tmp directory - } - }) + ctx = cache.WithCacheContext(req.Context()) + req = httplib.RequestWithContext(req, reqctx.FromContext(ctx)) next.ServeHTTP(respWriter, req) }) } diff --git a/services/context/api.go b/services/context/api.go index 50976de4576..47e1cc43431 100644 --- a/services/context/api.go +++ b/services/context/api.go @@ -19,6 +19,7 @@ import ( "gitea.dev/modules/cache" "gitea.dev/modules/git" "gitea.dev/modules/httpcache" + "gitea.dev/modules/httplib" "gitea.dev/modules/log" "gitea.dev/modules/paginator" "gitea.dev/modules/reqctx" @@ -257,8 +258,8 @@ func APIContexter() func(http.Handler) http.Handler { Repo: &Repository{}, Org: &APIOrganization{}, } - ctx.SetContextValue(apiContextKey, ctx) + httplib.MarkRequestSupportPublicURL(ctx) // FIXME: GLOBAL-PARSE-FORM: see more details in another FIXME comment if ctx.Req.Method == http.MethodPost && strings.Contains(ctx.Req.Header.Get("Content-Type"), "multipart/form-data") { diff --git a/services/context/base.go b/services/context/base.go index bb0798b2423..3bf7d8b319c 100644 --- a/services/context/base.go +++ b/services/context/base.go @@ -218,10 +218,9 @@ func NewBaseContext(resp http.ResponseWriter, req *http.Request) *Base { Locale: middleware.Locale(resp, req), Data: reqCtx.GetData(), } - b.Req = b.Req.WithContext(b) + b.Req = httplib.RequestWithContext(b.Req, reqCtx) reqCtx.SetContextValue(BaseContextKey, b) reqCtx.SetContextValue(translation.ContextKey, b.Locale) - reqCtx.SetContextValue(httplib.RequestContextKey, b.Req) return b } diff --git a/services/context/context.go b/services/context/context.go index a1688111c12..6cc1fe0e984 100644 --- a/services/context/context.go +++ b/services/context/context.go @@ -17,6 +17,7 @@ import ( user_model "gitea.dev/models/user" "gitea.dev/modules/cache" "gitea.dev/modules/httpcache" + "gitea.dev/modules/httplib" "gitea.dev/modules/reqctx" "gitea.dev/modules/session" "gitea.dev/modules/setting" @@ -119,6 +120,7 @@ func NewWebContext(base *Base, render Render, session session.Store) *Context { ctx.TemplateContext = NewTemplateContextForWeb(ctx, ctx.Base.Req, ctx.Base.Locale) ctx.Flash = &middleware.Flash{DataStore: ctx, Values: url.Values{}} ctx.SetContextValue(WebContextKey, ctx) + httplib.MarkRequestSupportPublicURL(ctx) return ctx }