From 580e21dd2e9dfb3a3f86f51c4eb188c1bbfa8b11 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 12 Nov 2024 10:38:22 +0800 Subject: [PATCH] Refactor LFS SSH and internal routers (#32473) Gitea instance keeps reporting a lot of errors like "LFS SSH transfer connection denied, pure SSH protocol is disabled". When starting debugging the problem, there are more problems found. Try to address most of them: * avoid unnecessary server side error logs (change `fail()` to not log them) * figure out the broken tests/user2/lfs.git (added comments) * avoid `migratePushMirrors` failure when a repository doesn't exist (ignore them) * avoid "Authorization" (internal&lfs) header conflicts, remove the tricky "swapAuth" and use "X-Gitea-Internal-Auth" * make internal token comparing constant time (it wasn't a serous problem because in a real world it's nearly impossible to timing-attack the token, but good to fix and backport) * avoid duplicate routers (introduce AddOwnerRepoGitLFSRoutes) * avoid "internal (private)" routes using session/web context (they should use private context) * fix incorrect "path" usages (use "filepath") * fix incorrect mocked route point handling (need to check func nil correctly) * split some tests from "git general tests" to "git misc tests" (to keep "git_general_test.go" simple) Still no correct result for Git LFS SSH tests. So the code is kept there (`tests/integration/git_lfs_ssh_test.go`) and a FIXME explains the details. --- cmd/serv.go | 14 +- models/fixtures/lfs_meta_object.yml | 18 +- models/migrations/v1_21/v276.go | 5 +- modules/git/batch_reader.go | 5 +- modules/lfstransfer/backend/backend.go | 48 ++--- modules/lfstransfer/backend/lock.go | 38 ++-- modules/lfstransfer/backend/util.go | 10 +- modules/private/internal.go | 2 +- modules/web/route.go | 13 +- routers/common/lfs.go | 29 +++ routers/private/internal.go | 55 ++---- routers/web/web.go | 18 +- tests/integration/api_repo_file_get_test.go | 2 +- .../{git_test.go => git_general_test.go} | 173 ++++-------------- tests/integration/git_lfs_ssh_test.go | 61 ++++++ tests/integration/git_misc_test.go | 138 ++++++++++++++ tests/test_utils.go | 11 +- 17 files changed, 376 insertions(+), 264 deletions(-) create mode 100644 routers/common/lfs.go rename tests/integration/{git_test.go => git_general_test.go} (85%) create mode 100644 tests/integration/git_lfs_ssh_test.go create mode 100644 tests/integration/git_misc_test.go diff --git a/cmd/serv.go b/cmd/serv.go index 2d2df8aa23..d2271b68d2 100644 --- a/cmd/serv.go +++ b/cmd/serv.go @@ -111,12 +111,10 @@ func fail(ctx context.Context, userMessage, logMsgFmt string, args ...any) error if !setting.IsProd { _, _ = fmt.Fprintln(os.Stderr, "Gitea:", logMsg) } - if userMessage != "" { - if unicode.IsPunct(rune(userMessage[len(userMessage)-1])) { - logMsg = userMessage + " " + logMsg - } else { - logMsg = userMessage + ". " + logMsg - } + if unicode.IsPunct(rune(userMessage[len(userMessage)-1])) { + logMsg = userMessage + " " + logMsg + } else { + logMsg = userMessage + ". " + logMsg } _ = private.SSHLog(ctx, true, logMsg) } @@ -288,10 +286,10 @@ func runServ(c *cli.Context) error { if allowedCommands.Contains(verb) { if allowedCommandsLfs.Contains(verb) { if !setting.LFS.StartServer { - return fail(ctx, "Unknown git command", "LFS authentication request over SSH denied, LFS support is disabled") + return fail(ctx, "LFS Server is not enabled", "") } if verb == verbLfsTransfer && !setting.LFS.AllowPureSSH { - return fail(ctx, "Unknown git command", "LFS SSH transfer connection denied, pure SSH protocol is disabled") + return fail(ctx, "LFS SSH transfer is not enabled", "") } if len(words) > 2 { lfsVerb = words[2] diff --git a/models/fixtures/lfs_meta_object.yml b/models/fixtures/lfs_meta_object.yml index 1c29e02d44..5430506d70 100644 --- a/models/fixtures/lfs_meta_object.yml +++ b/models/fixtures/lfs_meta_object.yml @@ -1,4 +1,11 @@ # These are the LFS objects in user2/lfs.git +# user2/lfs is an INVALID repository +# +# commit e9c32647bab825977942598c0efa415de300304b (HEAD -> master) +# Author: Rowan Bohde +# Date: Thu Aug 1 14:38:23 2024 -0500 +# +# add invalid lfs file - id: 1 @@ -11,7 +18,7 @@ id: 2 oid: 2eccdb43825d2a49d99d542daa20075cff1d97d9d2349a8977efe9c03661737c - size: 107 + size: 107 # real size is 2048 repository_id: 54 created_unix: 1671607299 @@ -30,3 +37,12 @@ size: 25 repository_id: 54 created_unix: 1671607299 + +# this file is missing +# - +# +# id: 5 +# oid: 9d178b5f15046343fd32f451df93acc2bdd9e6373be478b968e4cad6b6647351 +# size: 25 +# repository_id: 54 +# created_unix: 1671607299 diff --git a/models/migrations/v1_21/v276.go b/models/migrations/v1_21/v276.go index ed1bc3bda5..15177bf040 100644 --- a/models/migrations/v1_21/v276.go +++ b/models/migrations/v1_21/v276.go @@ -12,6 +12,7 @@ import ( "code.gitea.io/gitea/modules/git" giturl "code.gitea.io/gitea/modules/git/url" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" "xorm.io/xorm" ) @@ -163,7 +164,9 @@ func migratePushMirrors(x *xorm.Engine) error { func getRemoteAddress(ownerName, repoName, remoteName string) (string, error) { repoPath := filepath.Join(setting.RepoRootPath, strings.ToLower(ownerName), strings.ToLower(repoName)+".git") - + if exist, _ := util.IsExist(repoPath); !exist { + return "", nil + } remoteURL, err := git.GetRemoteAddress(context.Background(), repoPath, remoteName) if err != nil { return "", fmt.Errorf("get remote %s's address of %s/%s failed: %v", remoteName, ownerName, repoName, err) diff --git a/modules/git/batch_reader.go b/modules/git/batch_reader.go index 3b1a466b2e..7dfda72155 100644 --- a/modules/git/batch_reader.go +++ b/modules/git/batch_reader.go @@ -146,9 +146,8 @@ func catFileBatch(ctx context.Context, repoPath string) (WriteCloserError, *bufi } // ReadBatchLine reads the header line from cat-file --batch -// We expect: -// SP SP LF -// sha is a hex encoded here +// We expect: SP SP LF +// then leaving the rest of the stream " LF" to be read func ReadBatchLine(rd *bufio.Reader) (sha []byte, typ string, size int64, err error) { typ, err = rd.ReadString('\n') if err != nil { diff --git a/modules/lfstransfer/backend/backend.go b/modules/lfstransfer/backend/backend.go index d4523e1abf..2b1fe49fda 100644 --- a/modules/lfstransfer/backend/backend.go +++ b/modules/lfstransfer/backend/backend.go @@ -33,12 +33,12 @@ var _ transfer.Backend = &GiteaBackend{} // GiteaBackend is an adapter between git-lfs-transfer library and Gitea's internal LFS API type GiteaBackend struct { - ctx context.Context - server *url.URL - op string - token string - itoken string - logger transfer.Logger + ctx context.Context + server *url.URL + op string + authToken string + internalAuth string + logger transfer.Logger } func New(ctx context.Context, repo, op, token string, logger transfer.Logger) (transfer.Backend, error) { @@ -48,7 +48,7 @@ func New(ctx context.Context, repo, op, token string, logger transfer.Logger) (t return nil, err } server = server.JoinPath("api/internal/repo", repo, "info/lfs") - return &GiteaBackend{ctx: ctx, server: server, op: op, token: token, itoken: fmt.Sprintf("Bearer %s", setting.InternalToken), logger: logger}, nil + return &GiteaBackend{ctx: ctx, server: server, op: op, authToken: token, internalAuth: fmt.Sprintf("Bearer %s", setting.InternalToken), logger: logger}, nil } // Batch implements transfer.Backend @@ -73,10 +73,10 @@ func (g *GiteaBackend) Batch(_ string, pointers []transfer.BatchItem, args trans } url := g.server.JoinPath("objects/batch").String() headers := map[string]string{ - headerAuthorisation: g.itoken, - headerAuthX: g.token, - headerAccept: mimeGitLFS, - headerContentType: mimeGitLFS, + headerAuthorization: g.authToken, + headerGiteaInternalAuth: g.internalAuth, + headerAccept: mimeGitLFS, + headerContentType: mimeGitLFS, } req := newInternalRequest(g.ctx, url, http.MethodPost, headers, bodyBytes) resp, err := req.Response() @@ -119,7 +119,7 @@ func (g *GiteaBackend) Batch(_ string, pointers []transfer.BatchItem, args trans } idMapStr := base64.StdEncoding.EncodeToString(idMapBytes) item.Args[argID] = idMapStr - if authHeader, ok := action.Header[headerAuthorisation]; ok { + if authHeader, ok := action.Header[headerAuthorization]; ok { authHeaderB64 := base64.StdEncoding.EncodeToString([]byte(authHeader)) item.Args[argToken] = authHeaderB64 } @@ -142,7 +142,7 @@ func (g *GiteaBackend) Batch(_ string, pointers []transfer.BatchItem, args trans } idMapStr := base64.StdEncoding.EncodeToString(idMapBytes) item.Args[argID] = idMapStr - if authHeader, ok := action.Header[headerAuthorisation]; ok { + if authHeader, ok := action.Header[headerAuthorization]; ok { authHeaderB64 := base64.StdEncoding.EncodeToString([]byte(authHeader)) item.Args[argToken] = authHeaderB64 } @@ -183,9 +183,9 @@ func (g *GiteaBackend) Download(oid string, args transfer.Args) (io.ReadCloser, } url := action.Href headers := map[string]string{ - headerAuthorisation: g.itoken, - headerAuthX: g.token, - headerAccept: mimeOctetStream, + headerAuthorization: g.authToken, + headerGiteaInternalAuth: g.internalAuth, + headerAccept: mimeOctetStream, } req := newInternalRequest(g.ctx, url, http.MethodGet, headers, nil) resp, err := req.Response() @@ -229,10 +229,10 @@ func (g *GiteaBackend) Upload(oid string, size int64, r io.Reader, args transfer } url := action.Href headers := map[string]string{ - headerAuthorisation: g.itoken, - headerAuthX: g.token, - headerContentType: mimeOctetStream, - headerContentLength: strconv.FormatInt(size, 10), + headerAuthorization: g.authToken, + headerGiteaInternalAuth: g.internalAuth, + headerContentType: mimeOctetStream, + headerContentLength: strconv.FormatInt(size, 10), } reqBytes, err := io.ReadAll(r) if err != nil { @@ -279,10 +279,10 @@ func (g *GiteaBackend) Verify(oid string, size int64, args transfer.Args) (trans } url := action.Href headers := map[string]string{ - headerAuthorisation: g.itoken, - headerAuthX: g.token, - headerAccept: mimeGitLFS, - headerContentType: mimeGitLFS, + headerAuthorization: g.authToken, + headerGiteaInternalAuth: g.internalAuth, + headerAccept: mimeGitLFS, + headerContentType: mimeGitLFS, } req := newInternalRequest(g.ctx, url, http.MethodPost, headers, bodyBytes) resp, err := req.Response() diff --git a/modules/lfstransfer/backend/lock.go b/modules/lfstransfer/backend/lock.go index f72ffd5b6f..f094cce1db 100644 --- a/modules/lfstransfer/backend/lock.go +++ b/modules/lfstransfer/backend/lock.go @@ -21,17 +21,17 @@ import ( var _ transfer.LockBackend = &giteaLockBackend{} type giteaLockBackend struct { - ctx context.Context - g *GiteaBackend - server *url.URL - token string - itoken string - logger transfer.Logger + ctx context.Context + g *GiteaBackend + server *url.URL + authToken string + internalAuth string + logger transfer.Logger } func newGiteaLockBackend(g *GiteaBackend) transfer.LockBackend { server := g.server.JoinPath("locks") - return &giteaLockBackend{ctx: g.ctx, g: g, server: server, token: g.token, itoken: g.itoken, logger: g.logger} + return &giteaLockBackend{ctx: g.ctx, g: g, server: server, authToken: g.authToken, internalAuth: g.internalAuth, logger: g.logger} } // Create implements transfer.LockBackend @@ -45,10 +45,10 @@ func (g *giteaLockBackend) Create(path, refname string) (transfer.Lock, error) { } url := g.server.String() headers := map[string]string{ - headerAuthorisation: g.itoken, - headerAuthX: g.token, - headerAccept: mimeGitLFS, - headerContentType: mimeGitLFS, + headerAuthorization: g.authToken, + headerGiteaInternalAuth: g.internalAuth, + headerAccept: mimeGitLFS, + headerContentType: mimeGitLFS, } req := newInternalRequest(g.ctx, url, http.MethodPost, headers, bodyBytes) resp, err := req.Response() @@ -97,10 +97,10 @@ func (g *giteaLockBackend) Unlock(lock transfer.Lock) error { } url := g.server.JoinPath(lock.ID(), "unlock").String() headers := map[string]string{ - headerAuthorisation: g.itoken, - headerAuthX: g.token, - headerAccept: mimeGitLFS, - headerContentType: mimeGitLFS, + headerAuthorization: g.authToken, + headerGiteaInternalAuth: g.internalAuth, + headerAccept: mimeGitLFS, + headerContentType: mimeGitLFS, } req := newInternalRequest(g.ctx, url, http.MethodPost, headers, bodyBytes) resp, err := req.Response() @@ -180,10 +180,10 @@ func (g *giteaLockBackend) queryLocks(v url.Values) ([]transfer.Lock, string, er urlq.RawQuery = v.Encode() url := urlq.String() headers := map[string]string{ - headerAuthorisation: g.itoken, - headerAuthX: g.token, - headerAccept: mimeGitLFS, - headerContentType: mimeGitLFS, + headerAuthorization: g.authToken, + headerGiteaInternalAuth: g.internalAuth, + headerAccept: mimeGitLFS, + headerContentType: mimeGitLFS, } req := newInternalRequest(g.ctx, url, http.MethodGet, headers, nil) resp, err := req.Response() diff --git a/modules/lfstransfer/backend/util.go b/modules/lfstransfer/backend/util.go index 126ac00175..cffefef375 100644 --- a/modules/lfstransfer/backend/util.go +++ b/modules/lfstransfer/backend/util.go @@ -20,11 +20,11 @@ import ( // HTTP headers const ( - headerAccept = "Accept" - headerAuthorisation = "Authorization" - headerAuthX = "X-Auth" - headerContentType = "Content-Type" - headerContentLength = "Content-Length" + headerAccept = "Accept" + headerAuthorization = "Authorization" + headerGiteaInternalAuth = "X-Gitea-Internal-Auth" + headerContentType = "Content-Type" + headerContentLength = "Content-Length" ) // MIME types diff --git a/modules/private/internal.go b/modules/private/internal.go index 9c330a24a8..c7e7773524 100644 --- a/modules/private/internal.go +++ b/modules/private/internal.go @@ -43,7 +43,7 @@ Ensure you are running in the correct environment or set the correct configurati req := httplib.NewRequest(url, method). SetContext(ctx). Header("X-Real-IP", getClientIP()). - Header("Authorization", fmt.Sprintf("Bearer %s", setting.InternalToken)). + Header("X-Gitea-Internal-Auth", fmt.Sprintf("Bearer %s", setting.InternalToken)). SetTLSClientConfig(&tls.Config{ InsecureSkipVerify: true, ServerName: setting.Domain, diff --git a/modules/web/route.go b/modules/web/route.go index b02f66802e..77c411a97b 100644 --- a/modules/web/route.go +++ b/modules/web/route.go @@ -6,6 +6,7 @@ package web import ( "net/http" "net/url" + "reflect" "strings" "code.gitea.io/gitea/modules/setting" @@ -82,15 +83,23 @@ func (r *Router) getPattern(pattern string) string { return strings.TrimSuffix(newPattern, "/") } +func isNilOrFuncNil(v any) bool { + if v == nil { + return true + } + r := reflect.ValueOf(v) + return r.Kind() == reflect.Func && r.IsNil() +} + func (r *Router) wrapMiddlewareAndHandler(h []any) ([]func(http.Handler) http.Handler, http.HandlerFunc) { handlerProviders := make([]func(http.Handler) http.Handler, 0, len(r.curMiddlewares)+len(h)+1) for _, m := range r.curMiddlewares { - if m != nil { + if !isNilOrFuncNil(m) { handlerProviders = append(handlerProviders, toHandlerProvider(m)) } } for _, m := range h { - if h != nil { + if !isNilOrFuncNil(m) { handlerProviders = append(handlerProviders, toHandlerProvider(m)) } } diff --git a/routers/common/lfs.go b/routers/common/lfs.go new file mode 100644 index 0000000000..ba6e1163f1 --- /dev/null +++ b/routers/common/lfs.go @@ -0,0 +1,29 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package common + +import ( + "net/http" + + "code.gitea.io/gitea/modules/web" + "code.gitea.io/gitea/services/lfs" +) + +func AddOwnerRepoGitLFSRoutes(m *web.Router, middlewares ...any) { + // shared by web and internal routers + m.Group("/{username}/{reponame}/info/lfs", func() { + m.Post("/objects/batch", lfs.CheckAcceptMediaType, lfs.BatchHandler) + m.Put("/objects/{oid}/{size}", lfs.UploadHandler) + m.Get("/objects/{oid}/{filename}", lfs.DownloadHandler) + m.Get("/objects/{oid}", lfs.DownloadHandler) + m.Post("/verify", lfs.CheckAcceptMediaType, lfs.VerifyHandler) + m.Group("/locks", func() { + m.Get("/", lfs.GetListLockHandler) + m.Post("/", lfs.PostLockHandler) + m.Post("/verify", lfs.VerifyLockHandler) + m.Post("/{lid}/unlock", lfs.UnLockHandler) + }, lfs.CheckAcceptMediaType) + m.Any("/*", http.NotFound) + }, middlewares...) +} diff --git a/routers/private/internal.go b/routers/private/internal.go index f9adff388c..db074238c6 100644 --- a/routers/private/internal.go +++ b/routers/private/internal.go @@ -5,6 +5,7 @@ package private import ( + "crypto/subtle" "net/http" "strings" @@ -14,28 +15,30 @@ import ( "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/routers/common" "code.gitea.io/gitea/services/context" - "code.gitea.io/gitea/services/lfs" "gitea.com/go-chi/binding" chi_middleware "github.com/go-chi/chi/v5/middleware" ) -// CheckInternalToken check internal token is set -func CheckInternalToken(next http.Handler) http.Handler { +const RouterMockPointInternalLFS = "internal-lfs" + +func authInternal(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { - tokens := req.Header.Get("Authorization") - fields := strings.SplitN(tokens, " ", 2) if setting.InternalToken == "" { log.Warn(`The INTERNAL_TOKEN setting is missing from the configuration file: %q, internal API can't work.`, setting.CustomConf) http.Error(w, http.StatusText(http.StatusForbidden), http.StatusForbidden) return } - if len(fields) != 2 || fields[0] != "Bearer" || fields[1] != setting.InternalToken { + + tokens := req.Header.Get("X-Gitea-Internal-Auth") // TODO: use something like JWT or HMAC to avoid passing the token in the clear + after, found := strings.CutPrefix(tokens, "Bearer ") + authSucceeded := found && subtle.ConstantTimeCompare([]byte(after), []byte(setting.InternalToken)) == 1 + if !authSucceeded { log.Debug("Forbidden attempt to access internal url: Authorization header: %s", tokens) http.Error(w, http.StatusText(http.StatusForbidden), http.StatusForbidden) - } else { - next.ServeHTTP(w, req) + return } + next.ServeHTTP(w, req) }) } @@ -48,20 +51,12 @@ func bind[T any](_ T) any { } } -// SwapAuthToken swaps Authorization header with X-Auth header -func swapAuthToken(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { - req.Header.Set("Authorization", req.Header.Get("X-Auth")) - next.ServeHTTP(w, req) - }) -} - // Routes registers all internal APIs routes to web application. // These APIs will be invoked by internal commands for example `gitea serv` and etc. func Routes() *web.Router { r := web.NewRouter() r.Use(context.PrivateContexter()) - r.Use(CheckInternalToken) + r.Use(authInternal) // Log the real ip address of the request from SSH is really helpful for diagnosing sometimes. // Since internal API will be sent only from Gitea sub commands and it's under control (checked by InternalToken), we can trust the headers. r.Use(chi_middleware.RealIP) @@ -90,25 +85,13 @@ func Routes() *web.Router { r.Post("/restore_repo", RestoreRepo) r.Post("/actions/generate_actions_runner_token", GenerateActionsRunnerToken) - r.Group("/repo/{username}/{reponame}", func() { - r.Group("/info/lfs", func() { - r.Post("/objects/batch", lfs.CheckAcceptMediaType, lfs.BatchHandler) - r.Put("/objects/{oid}/{size}", lfs.UploadHandler) - r.Get("/objects/{oid}/{filename}", lfs.DownloadHandler) - r.Get("/objects/{oid}", lfs.DownloadHandler) - r.Post("/verify", lfs.CheckAcceptMediaType, lfs.VerifyHandler) - r.Group("/locks", func() { - r.Get("/", lfs.GetListLockHandler) - r.Post("/", lfs.PostLockHandler) - r.Post("/verify", lfs.VerifyLockHandler) - r.Post("/{lid}/unlock", lfs.UnLockHandler) - }, lfs.CheckAcceptMediaType) - r.Any("/*", func(ctx *context.Context) { - ctx.NotFound("", nil) - }) - }, swapAuthToken) - }, common.Sessioner(), context.Contexter()) - // end "/repo/{username}/{reponame}": git (LFS) API mirror + r.Group("/repo", func() { + // FIXME: it is not right to use context.Contexter here because all routes here should use PrivateContext + common.AddOwnerRepoGitLFSRoutes(r, func(ctx *context.PrivateContext) { + webContext := &context.Context{Base: ctx.Base} + ctx.AppendContextValue(context.WebContextKey, webContext) + }, web.RouterMockPoint(RouterMockPointInternalLFS)) + }) return r } diff --git a/routers/web/web.go b/routers/web/web.go index 907bf88f6f..e0915e6a6e 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -44,7 +44,6 @@ import ( auth_service "code.gitea.io/gitea/services/auth" "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/services/forms" - "code.gitea.io/gitea/services/lfs" _ "code.gitea.io/gitea/modules/session" // to registers all internal adapters @@ -1598,23 +1597,8 @@ func registerRoutes(m *web.Router) { m.Post("/action/{action}", reqSignIn, repo.Action) }, ignSignIn, context.RepoAssignment, context.RepoRef()) + common.AddOwnerRepoGitLFSRoutes(m, ignSignInAndCsrf, lfsServerEnabled) m.Group("/{username}/{reponame}", func() { - m.Group("/info/lfs", func() { - m.Post("/objects/batch", lfs.CheckAcceptMediaType, lfs.BatchHandler) - m.Put("/objects/{oid}/{size}", lfs.UploadHandler) - m.Get("/objects/{oid}/{filename}", lfs.DownloadHandler) - m.Get("/objects/{oid}", lfs.DownloadHandler) - m.Post("/verify", lfs.CheckAcceptMediaType, lfs.VerifyHandler) - m.Group("/locks", func() { - m.Get("/", lfs.GetListLockHandler) - m.Post("/", lfs.PostLockHandler) - m.Post("/verify", lfs.VerifyLockHandler) - m.Post("/{lid}/unlock", lfs.UnLockHandler) - }, lfs.CheckAcceptMediaType) - m.Any("/*", func(ctx *context.Context) { - ctx.NotFound("", nil) - }) - }, ignSignInAndCsrf, lfsServerEnabled) gitHTTPRouters(m) }) // end "/{username}/{reponame}.git": git support diff --git a/tests/integration/api_repo_file_get_test.go b/tests/integration/api_repo_file_get_test.go index 4649babad1..27bc9e25bf 100644 --- a/tests/integration/api_repo_file_get_test.go +++ b/tests/integration/api_repo_file_get_test.go @@ -39,7 +39,7 @@ func TestAPIGetRawFileOrLFS(t *testing.T) { t.Run("Partial Clone", doPartialGitClone(dstPath2, u)) - lfs, _ := lfsCommitAndPushTest(t, dstPath) + lfs := lfsCommitAndPushTest(t, dstPath, littleSize)[0] reqLFS := NewRequest(t, "GET", "/api/v1/repos/user2/repo1/media/"+lfs) respLFS := MakeRequestNilResponseRecorder(t, reqLFS, http.StatusOK) diff --git a/tests/integration/git_test.go b/tests/integration/git_general_test.go similarity index 85% rename from tests/integration/git_test.go rename to tests/integration/git_general_test.go index 76db3c6932..7fd19e7edd 100644 --- a/tests/integration/git_test.go +++ b/tests/integration/git_general_test.go @@ -4,8 +4,6 @@ package integration import ( - "bytes" - "context" "crypto/rand" "encoding/hex" "fmt" @@ -26,27 +24,25 @@ import ( "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" - "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/lfs" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" gitea_context "code.gitea.io/gitea/services/context" - files_service "code.gitea.io/gitea/services/repository/files" "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" ) const ( - littleSize = 1024 // 1ko - bigSize = 128 * 1024 * 1024 // 128Mo + littleSize = 1024 // 1K + bigSize = 128 * 1024 * 1024 // 128M ) -func TestGit(t *testing.T) { - onGiteaRun(t, testGit) +func TestGitGeneral(t *testing.T) { + onGiteaRun(t, testGitGeneral) } -func testGit(t *testing.T, u *url.URL) { +func testGitGeneral(t *testing.T, u *url.URL) { username := "user2" baseAPITestContext := NewAPITestContext(t, username, "repo1", auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser) @@ -77,10 +73,10 @@ func testGit(t *testing.T, u *url.URL) { t.Run("Partial Clone", doPartialGitClone(dstPath2, u)) - little, big := standardCommitAndPushTest(t, dstPath) - littleLFS, bigLFS := lfsCommitAndPushTest(t, dstPath) - rawTest(t, &httpContext, little, big, littleLFS, bigLFS) - mediaTest(t, &httpContext, little, big, littleLFS, bigLFS) + pushedFilesStandard := standardCommitAndPushTest(t, dstPath, littleSize, bigSize) + pushedFilesLFS := lfsCommitAndPushTest(t, dstPath, littleSize, bigSize) + rawTest(t, &httpContext, pushedFilesStandard[0], pushedFilesStandard[1], pushedFilesLFS[0], pushedFilesLFS[1]) + mediaTest(t, &httpContext, pushedFilesStandard[0], pushedFilesStandard[1], pushedFilesLFS[0], pushedFilesLFS[1]) t.Run("CreateAgitFlowPull", doCreateAgitFlowPull(dstPath, &httpContext, "test/head")) t.Run("BranchProtectMerge", doBranchProtectPRMerge(&httpContext, dstPath)) @@ -89,8 +85,8 @@ func testGit(t *testing.T, u *url.URL) { t.Run("MergeFork", func(t *testing.T) { defer tests.PrintCurrentTest(t)() t.Run("CreatePRAndMerge", doMergeFork(httpContext, forkedUserCtx, "master", httpContext.Username+":master")) - rawTest(t, &forkedUserCtx, little, big, littleLFS, bigLFS) - mediaTest(t, &forkedUserCtx, little, big, littleLFS, bigLFS) + rawTest(t, &forkedUserCtx, pushedFilesStandard[0], pushedFilesStandard[1], pushedFilesLFS[0], pushedFilesLFS[1]) + mediaTest(t, &forkedUserCtx, pushedFilesStandard[0], pushedFilesStandard[1], pushedFilesLFS[0], pushedFilesLFS[1]) }) t.Run("PushCreate", doPushCreate(httpContext, u)) @@ -118,18 +114,18 @@ func testGit(t *testing.T, u *url.URL) { t.Run("Clone", doGitClone(dstPath, sshURL)) - little, big := standardCommitAndPushTest(t, dstPath) - littleLFS, bigLFS := lfsCommitAndPushTest(t, dstPath) - rawTest(t, &sshContext, little, big, littleLFS, bigLFS) - mediaTest(t, &sshContext, little, big, littleLFS, bigLFS) + pushedFilesStandard := standardCommitAndPushTest(t, dstPath, littleSize, bigSize) + pushedFilesLFS := lfsCommitAndPushTest(t, dstPath, littleSize, bigSize) + rawTest(t, &sshContext, pushedFilesStandard[0], pushedFilesStandard[1], pushedFilesLFS[0], pushedFilesLFS[1]) + mediaTest(t, &sshContext, pushedFilesStandard[0], pushedFilesStandard[1], pushedFilesLFS[0], pushedFilesLFS[1]) t.Run("CreateAgitFlowPull", doCreateAgitFlowPull(dstPath, &sshContext, "test/head2")) t.Run("BranchProtectMerge", doBranchProtectPRMerge(&sshContext, dstPath)) t.Run("MergeFork", func(t *testing.T) { defer tests.PrintCurrentTest(t)() t.Run("CreatePRAndMerge", doMergeFork(sshContext, forkedUserCtx, "master", sshContext.Username+":master")) - rawTest(t, &forkedUserCtx, little, big, littleLFS, bigLFS) - mediaTest(t, &forkedUserCtx, little, big, littleLFS, bigLFS) + rawTest(t, &forkedUserCtx, pushedFilesStandard[0], pushedFilesStandard[1], pushedFilesLFS[0], pushedFilesLFS[1]) + mediaTest(t, &forkedUserCtx, pushedFilesStandard[0], pushedFilesStandard[1], pushedFilesLFS[0], pushedFilesLFS[1]) }) t.Run("PushCreate", doPushCreate(sshContext, sshURL)) @@ -142,16 +138,16 @@ func ensureAnonymousClone(t *testing.T, u *url.URL) { t.Run("CloneAnonymous", doGitClone(dstLocalPath, u)) } -func standardCommitAndPushTest(t *testing.T, dstPath string) (little, big string) { - t.Run("Standard", func(t *testing.T) { +func standardCommitAndPushTest(t *testing.T, dstPath string, sizes ...int) (pushedFiles []string) { + t.Run("CommitAndPushStandard", func(t *testing.T) { defer tests.PrintCurrentTest(t)() - little, big = commitAndPushTest(t, dstPath, "data-file-") + pushedFiles = commitAndPushTest(t, dstPath, "data-file-", sizes...) }) - return little, big + return pushedFiles } -func lfsCommitAndPushTest(t *testing.T, dstPath string) (littleLFS, bigLFS string) { - t.Run("LFS", func(t *testing.T) { +func lfsCommitAndPushTest(t *testing.T, dstPath string, sizes ...int) (pushedFiles []string) { + t.Run("CommitAndPushLFS", func(t *testing.T) { defer tests.PrintCurrentTest(t)() prefix := "lfs-data-file-" err := git.NewCommand(git.DefaultContext, "lfs").AddArguments("install").Run(&git.RunOpts{Dir: dstPath}) @@ -176,33 +172,23 @@ func lfsCommitAndPushTest(t *testing.T, dstPath string) (littleLFS, bigLFS strin }) assert.NoError(t, err) - littleLFS, bigLFS = commitAndPushTest(t, dstPath, prefix) - + pushedFiles = commitAndPushTest(t, dstPath, prefix, sizes...) t.Run("Locks", func(t *testing.T) { defer tests.PrintCurrentTest(t)() lockTest(t, dstPath) }) }) - return littleLFS, bigLFS + return pushedFiles } -func commitAndPushTest(t *testing.T, dstPath, prefix string) (little, big string) { - t.Run("PushCommit", func(t *testing.T) { - defer tests.PrintCurrentTest(t)() - t.Run("Little", func(t *testing.T) { +func commitAndPushTest(t *testing.T, dstPath, prefix string, sizes ...int) (pushedFiles []string) { + for _, size := range sizes { + t.Run("PushCommit Size-"+strconv.Itoa(size), func(t *testing.T) { defer tests.PrintCurrentTest(t)() - little = doCommitAndPush(t, littleSize, dstPath, prefix) + pushedFiles = append(pushedFiles, doCommitAndPush(t, size, dstPath, prefix)) }) - t.Run("Big", func(t *testing.T) { - if testing.Short() { - t.Skip("Skipping test in short mode.") - return - } - defer tests.PrintCurrentTest(t)() - big = doCommitAndPush(t, bigSize, dstPath, prefix) - }) - }) - return little, big + } + return pushedFiles } func rawTest(t *testing.T, ctx *APITestContext, little, big, littleLFS, bigLFS string) { @@ -903,100 +889,3 @@ func doCreateAgitFlowPull(dstPath string, ctx *APITestContext, headBranch string t.Run("CheckoutMasterAgain", doGitCheckoutBranch(dstPath, "master")) } } - -func TestDataAsync_Issue29101(t *testing.T) { - onGiteaRun(t, func(t *testing.T, u *url.URL) { - user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) - - resp, err := files_service.ChangeRepoFiles(db.DefaultContext, repo, user, &files_service.ChangeRepoFilesOptions{ - Files: []*files_service.ChangeRepoFile{ - { - Operation: "create", - TreePath: "test.txt", - ContentReader: bytes.NewReader(make([]byte, 10000)), - }, - }, - OldBranch: repo.DefaultBranch, - NewBranch: repo.DefaultBranch, - }) - assert.NoError(t, err) - - sha := resp.Commit.SHA - - gitRepo, err := gitrepo.OpenRepository(db.DefaultContext, repo) - assert.NoError(t, err) - - commit, err := gitRepo.GetCommit(sha) - assert.NoError(t, err) - - entry, err := commit.GetTreeEntryByPath("test.txt") - assert.NoError(t, err) - - b := entry.Blob() - - r, err := b.DataAsync() - assert.NoError(t, err) - defer r.Close() - - r2, err := b.DataAsync() - assert.NoError(t, err) - defer r2.Close() - }) -} - -func TestAgitPullPush(t *testing.T) { - onGiteaRun(t, func(t *testing.T, u *url.URL) { - baseAPITestContext := NewAPITestContext(t, "user2", "repo1", auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser) - - u.Path = baseAPITestContext.GitPath() - u.User = url.UserPassword("user2", userPassword) - - dstPath := t.TempDir() - doGitClone(dstPath, u)(t) - - gitRepo, err := git.OpenRepository(context.Background(), dstPath) - assert.NoError(t, err) - defer gitRepo.Close() - - doGitCreateBranch(dstPath, "test-agit-push") - - // commit 1 - _, err = generateCommitWithNewData(littleSize, dstPath, "user2@example.com", "User Two", "branch-data-file-") - assert.NoError(t, err) - - // push to create an agit pull request - err = git.NewCommand(git.DefaultContext, "push", "origin", - "-o", "title=test-title", "-o", "description=test-description", - "HEAD:refs/for/master/test-agit-push", - ).Run(&git.RunOpts{Dir: dstPath}) - assert.NoError(t, err) - - // check pull request exist - pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: 1, Flow: issues_model.PullRequestFlowAGit, HeadBranch: "user2/test-agit-push"}) - assert.NoError(t, pr.LoadIssue(db.DefaultContext)) - assert.Equal(t, "test-title", pr.Issue.Title) - assert.Equal(t, "test-description", pr.Issue.Content) - - // commit 2 - _, err = generateCommitWithNewData(littleSize, dstPath, "user2@example.com", "User Two", "branch-data-file-2-") - assert.NoError(t, err) - - // push 2 - err = git.NewCommand(git.DefaultContext, "push", "origin", "HEAD:refs/for/master/test-agit-push").Run(&git.RunOpts{Dir: dstPath}) - assert.NoError(t, err) - - // reset to first commit - err = git.NewCommand(git.DefaultContext, "reset", "--hard", "HEAD~1").Run(&git.RunOpts{Dir: dstPath}) - assert.NoError(t, err) - - // test force push without confirm - _, stderr, err := git.NewCommand(git.DefaultContext, "push", "origin", "HEAD:refs/for/master/test-agit-push").RunStdString(&git.RunOpts{Dir: dstPath}) - assert.Error(t, err) - assert.Contains(t, stderr, "[remote rejected] HEAD -> refs/for/master/test-agit-push (request `force-push` push option)") - - // test force push with confirm - err = git.NewCommand(git.DefaultContext, "push", "origin", "HEAD:refs/for/master/test-agit-push", "-o", "force-push").Run(&git.RunOpts{Dir: dstPath}) - assert.NoError(t, err) - }) -} diff --git a/tests/integration/git_lfs_ssh_test.go b/tests/integration/git_lfs_ssh_test.go new file mode 100644 index 0000000000..33c2fba620 --- /dev/null +++ b/tests/integration/git_lfs_ssh_test.go @@ -0,0 +1,61 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package integration + +import ( + "net/url" + "sync" + "testing" + + auth_model "code.gitea.io/gitea/models/auth" + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/web" + "code.gitea.io/gitea/routers/private" + "code.gitea.io/gitea/services/context" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGitLFSSSH(t *testing.T) { + onGiteaRun(t, func(t *testing.T, u *url.URL) { + dstPath := t.TempDir() + apiTestContext := NewAPITestContext(t, "user2", "repo1", auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser) + + var mu sync.Mutex + var routerCalls []string + web.RouteMock(private.RouterMockPointInternalLFS, func(ctx *context.PrivateContext) { + mu.Lock() + routerCalls = append(routerCalls, ctx.Req.Method+" "+ctx.Req.URL.Path) + mu.Unlock() + }) + + withKeyFile(t, "my-testing-key", func(keyFile string) { + t.Run("CreateUserKey", doAPICreateUserKey(apiTestContext, "test-key", keyFile)) + cloneURL := createSSHUrl(apiTestContext.GitPath(), u) + t.Run("Clone", doGitClone(dstPath, cloneURL)) + + cfg, err := setting.CfgProvider.PrepareSaving() + require.NoError(t, err) + cfg.Section("server").Key("LFS_ALLOW_PURE_SSH").SetValue("true") + setting.LFS.AllowPureSSH = true + require.NoError(t, cfg.Save()) + + // do LFS SSH transfer? + lfsCommitAndPushTest(t, dstPath, 10) + }) + + // FIXME: Here we only see the following calls, but actually there should be calls to "PUT"? + // 0 = {string} "GET /api/internal/repo/user2/repo1.git/info/lfs/locks" + // 1 = {string} "POST /api/internal/repo/user2/repo1.git/info/lfs/objects/batch" + // 2 = {string} "GET /api/internal/repo/user2/repo1.git/info/lfs/locks" + // 3 = {string} "POST /api/internal/repo/user2/repo1.git/info/lfs/locks" + // 4 = {string} "GET /api/internal/repo/user2/repo1.git/info/lfs/locks" + // 5 = {string} "GET /api/internal/repo/user2/repo1.git/info/lfs/locks" + // 6 = {string} "GET /api/internal/repo/user2/repo1.git/info/lfs/locks" + // 7 = {string} "POST /api/internal/repo/user2/repo1.git/info/lfs/locks/24/unlock" + assert.NotEmpty(t, routerCalls) + // assert.Contains(t, routerCalls, "PUT /api/internal/repo/user2/repo1.git/info/lfs/objects/....") + }) +} diff --git a/tests/integration/git_misc_test.go b/tests/integration/git_misc_test.go new file mode 100644 index 0000000000..82ab184bb0 --- /dev/null +++ b/tests/integration/git_misc_test.go @@ -0,0 +1,138 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package integration + +import ( + "bytes" + "context" + "io" + "net/url" + "sync" + "testing" + + auth_model "code.gitea.io/gitea/models/auth" + "code.gitea.io/gitea/models/db" + issues_model "code.gitea.io/gitea/models/issues" + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/gitrepo" + files_service "code.gitea.io/gitea/services/repository/files" + + "github.com/stretchr/testify/assert" +) + +func TestDataAsyncDoubleRead_Issue29101(t *testing.T) { + onGiteaRun(t, func(t *testing.T, u *url.URL) { + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + + testContent := bytes.Repeat([]byte{'a'}, 10000) + resp, err := files_service.ChangeRepoFiles(db.DefaultContext, repo, user, &files_service.ChangeRepoFilesOptions{ + Files: []*files_service.ChangeRepoFile{ + { + Operation: "create", + TreePath: "test.txt", + ContentReader: bytes.NewReader(testContent), + }, + }, + OldBranch: repo.DefaultBranch, + NewBranch: repo.DefaultBranch, + }) + assert.NoError(t, err) + + sha := resp.Commit.SHA + + gitRepo, err := gitrepo.OpenRepository(db.DefaultContext, repo) + assert.NoError(t, err) + + commit, err := gitRepo.GetCommit(sha) + assert.NoError(t, err) + + entry, err := commit.GetTreeEntryByPath("test.txt") + assert.NoError(t, err) + + b := entry.Blob() + r1, err := b.DataAsync() + assert.NoError(t, err) + defer r1.Close() + r2, err := b.DataAsync() + assert.NoError(t, err) + defer r2.Close() + + var data1, data2 []byte + wg := sync.WaitGroup{} + wg.Add(2) + go func() { + data1, _ = io.ReadAll(r1) + assert.NoError(t, err) + wg.Done() + }() + go func() { + data2, _ = io.ReadAll(r2) + assert.NoError(t, err) + wg.Done() + }() + wg.Wait() + assert.Equal(t, testContent, data1) + assert.Equal(t, testContent, data2) + }) +} + +func TestAgitPullPush(t *testing.T) { + onGiteaRun(t, func(t *testing.T, u *url.URL) { + baseAPITestContext := NewAPITestContext(t, "user2", "repo1", auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser) + + u.Path = baseAPITestContext.GitPath() + u.User = url.UserPassword("user2", userPassword) + + dstPath := t.TempDir() + doGitClone(dstPath, u)(t) + + gitRepo, err := git.OpenRepository(context.Background(), dstPath) + assert.NoError(t, err) + defer gitRepo.Close() + + doGitCreateBranch(dstPath, "test-agit-push") + + // commit 1 + _, err = generateCommitWithNewData(littleSize, dstPath, "user2@example.com", "User Two", "branch-data-file-") + assert.NoError(t, err) + + // push to create an agit pull request + err = git.NewCommand(git.DefaultContext, "push", "origin", + "-o", "title=test-title", "-o", "description=test-description", + "HEAD:refs/for/master/test-agit-push", + ).Run(&git.RunOpts{Dir: dstPath}) + assert.NoError(t, err) + + // check pull request exist + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: 1, Flow: issues_model.PullRequestFlowAGit, HeadBranch: "user2/test-agit-push"}) + assert.NoError(t, pr.LoadIssue(db.DefaultContext)) + assert.Equal(t, "test-title", pr.Issue.Title) + assert.Equal(t, "test-description", pr.Issue.Content) + + // commit 2 + _, err = generateCommitWithNewData(littleSize, dstPath, "user2@example.com", "User Two", "branch-data-file-2-") + assert.NoError(t, err) + + // push 2 + err = git.NewCommand(git.DefaultContext, "push", "origin", "HEAD:refs/for/master/test-agit-push").Run(&git.RunOpts{Dir: dstPath}) + assert.NoError(t, err) + + // reset to first commit + err = git.NewCommand(git.DefaultContext, "reset", "--hard", "HEAD~1").Run(&git.RunOpts{Dir: dstPath}) + assert.NoError(t, err) + + // test force push without confirm + _, stderr, err := git.NewCommand(git.DefaultContext, "push", "origin", "HEAD:refs/for/master/test-agit-push").RunStdString(&git.RunOpts{Dir: dstPath}) + assert.Error(t, err) + assert.Contains(t, stderr, "[remote rejected] HEAD -> refs/for/master/test-agit-push (request `force-push` push option)") + + // test force push with confirm + err = git.NewCommand(git.DefaultContext, "push", "origin", "HEAD:refs/for/master/test-agit-push", "-o", "force-push").Run(&git.RunOpts{Dir: dstPath}) + assert.NoError(t, err) + }) +} diff --git a/tests/test_utils.go b/tests/test_utils.go index e6ce3cce0e..3503ca1975 100644 --- a/tests/test_utils.go +++ b/tests/test_utils.go @@ -9,7 +9,6 @@ import ( "database/sql" "fmt" "os" - "path" "path/filepath" "testing" @@ -53,7 +52,7 @@ func InitTest(requireGitea bool) { if setting.IsWindows { giteaBinary += ".exe" } - setting.AppPath = path.Join(giteaRoot, giteaBinary) + setting.AppPath = filepath.Join(giteaRoot, giteaBinary) if _, err := os.Stat(setting.AppPath); err != nil { exitf("Could not find gitea binary at %s", setting.AppPath) } @@ -70,7 +69,7 @@ func InitTest(requireGitea bool) { exitf(`sqlite3 requires: import _ "github.com/mattn/go-sqlite3" or -tags sqlite,sqlite_unlock_notify`) } } - if !path.IsAbs(giteaConf) { + if !filepath.IsAbs(giteaConf) { setting.CustomConf = filepath.Join(giteaRoot, giteaConf) } else { setting.CustomConf = giteaConf @@ -193,8 +192,12 @@ func PrepareAttachmentsStorage(t testing.TB) { } func PrepareGitRepoDirectory(t testing.TB) { + if !assert.NotEmpty(t, setting.RepoRootPath) { + return + } + assert.NoError(t, util.RemoveAll(setting.RepoRootPath)) - assert.NoError(t, unittest.CopyDir(path.Join(filepath.Dir(setting.AppPath), "tests/gitea-repositories-meta"), setting.RepoRootPath)) + assert.NoError(t, unittest.CopyDir(filepath.Join(filepath.Dir(setting.AppPath), "tests/gitea-repositories-meta"), setting.RepoRootPath)) ownerDirs, err := os.ReadDir(setting.RepoRootPath) if err != nil { -- 2.39.5