fix(security): enforce wiki git writes and LFS token access at request time (#37695) (#37714)

Backport #37695 by @lunny

This PR fixes two permission-checking gaps in Git and LFS request
handling.

## What it changes

- keep wiki Git HTTP pushes on the normal write-permission path, even
when proc-receive support is enabled
- revalidate LFS bearer token requests against the current user state
and current repository permissions before allowing access
- add regression coverage for unauthorized wiki HTTP pushes
- add LFS tests for blocked users, revoked repository access, read-only
upload attempts, and valid write access

## Why

- wiki repositories should not inherit the relaxed refs/for handling
used for normal code repositories
- LFS authorization tokens should not remain usable after a user is
disabled or loses repository access

---------

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

routers/web/repo/githttp.go

@@ -180,8 +180,8 @@ func httpBase(ctx *context.Context, optGitService ...string) *serviceHandler {
 		}
 
 		if repoExist {
-			// Because of special ref "refs/for" (agit) , need delay write permission check
+			// Only the main code repo accepts refs/for pushes, so wiki pushes must keep write checks.
-			if git.DefaultFeatures().SupportProcReceive {
+			if git.DefaultFeatures().SupportProcReceive && !isWiki {
 				accessMode = perm.AccessModeRead
 			}
 

services/lfs/server.go

@@ -32,6 +32,7 @@ import (
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/storage"
+	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/services/context"
 
 	"github.com/golang-jwt/jwt/v5"
@@ -605,6 +606,18 @@ func handleLFSToken(ctx stdCtx.Context, tokenSHA string, target *repo_model.Repo
 		log.Error("Unable to GetUserById[%d]: Error: %v", claims.UserID, err)
 		return nil, err
 	}
+	if !u.IsActive || u.ProhibitLogin {
+		return nil, util.NewPermissionDeniedErrorf("not allowed to access any repository")
+	}
+
+	perm, err := access_model.GetDoerRepoPermission(ctx, target, u)
+	if err != nil {
+		log.Error("Unable to GetDoerRepoPermission for user[%d] repo[%d]: %v", claims.UserID, target.ID, err)
+		return nil, err
+	}
+	if !perm.CanAccess(mode, unit.TypeCode) {
+		return nil, util.NewPermissionDeniedErrorf("no permission to access the repository")
+	}
 	return u, nil
 }
 

services/lfs/server_test.go

@@ -7,9 +7,11 @@ import (
 	"strings"
 	"testing"
 
+	"code.gitea.io/gitea/models/db"
 	perm_model "code.gitea.io/gitea/models/perm"
 	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/services/contexttest"
 
 	"github.com/stretchr/testify/assert"
@@ -22,11 +24,15 @@ func TestMain(m *testing.M) {
 
 func TestAuthenticate(t *testing.T) {
 	require.NoError(t, unittest.PrepareTestDatabase())
+	ctx, _ := contexttest.MockContext(t, "/")
+
 	repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
 
-	token2, _ := GetLFSAuthTokenWithBearer(AuthTokenOptions{Op: "download", UserID: 2, RepoID: 1})
+	getUserToken := func(op string, userID int64, repo *repo_model.Repository) string {
-	_, token2, _ = strings.Cut(token2, " ")
+		s, _ := GetLFSAuthTokenWithBearer(AuthTokenOptions{Op: op, UserID: userID, RepoID: repo.ID})
-	ctx, _ := contexttest.MockContext(t, "/")
+		_, token, _ := strings.Cut(s, " ")
+		return token
+	}
 
 	t.Run("handleLFSToken", func(t *testing.T) {
 		u, err := handleLFSToken(ctx, "", repo1, perm_model.AccessModeRead)
@@ -37,15 +43,62 @@ func TestAuthenticate(t *testing.T) {
 		require.Error(t, err)
 		assert.Nil(t, u)
 
-		u, err = handleLFSToken(ctx, token2, repo1, perm_model.AccessModeRead)
+		u, err = handleLFSToken(ctx, getUserToken("download", 2, repo1), repo1, perm_model.AccessModeRead)
 		require.NoError(t, err)
 		assert.EqualValues(t, 2, u.ID)
 	})
 
 	t.Run("authenticate", func(t *testing.T) {
 		const prefixBearer = "Bearer "
+		token := getUserToken("download", 2, repo1)
 		assert.False(t, authenticate(ctx, repo1, "", true, false))
 		assert.False(t, authenticate(ctx, repo1, prefixBearer+"invalid", true, false))
-		assert.True(t, authenticate(ctx, repo1, prefixBearer+token2, true, false))
+		assert.True(t, authenticate(ctx, repo1, prefixBearer+token, true, false))
+	})
+
+	handleLFSTokenTestPerm := func(op string, userID int64, repo *repo_model.Repository, accessMode perm_model.AccessMode) error {
+		token := getUserToken(op, userID, repo)
+		u, err := handleLFSToken(ctx, token, repo, accessMode)
+		if err == nil {
+			assert.Equal(t, userID, u.ID)
+		}
+		return err
+	}
+
+	t.Run("handleLFSToken blocks prohibited users", func(t *testing.T) {
+		user37 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 37})
+
+		// prohibited user
+		assert.True(t, user37.ProhibitLogin)
+		err := handleLFSTokenTestPerm("download", 37, repo1, perm_model.AccessModeRead)
+		assert.ErrorContains(t, err, "not allowed to access any repository")
+
+		// normal user
+		_, _ = db.GetEngine(t.Context()).ID(37).Cols("prohibit_login").Update(&user_model.User{ProhibitLogin: false})
+		err = handleLFSTokenTestPerm("download", 37, repo1, perm_model.AccessModeRead)
+		assert.NoError(t, err)
+
+		// inactive user
+		_, _ = db.GetEngine(t.Context()).ID(37).Cols("is_active").Update(&user_model.User{IsActive: false})
+		err = handleLFSTokenTestPerm("download", 37, repo1, perm_model.AccessModeRead)
+		assert.ErrorContains(t, err, "not allowed to access any repository")
+	})
+
+	t.Run("handleLFSToken blocks users without repo access", func(t *testing.T) {
+		repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2})
+		err := handleLFSTokenTestPerm("download", 10, repo2, perm_model.AccessModeRead)
+		assert.ErrorContains(t, err, "no permission to access the repository")
+	})
+
+	t.Run("handleLFSToken requires write access for uploads", func(t *testing.T) {
+		err := handleLFSTokenTestPerm("download", 10, repo1, perm_model.AccessModeRead)
+		assert.NoError(t, err)
+		err = handleLFSTokenTestPerm("upload", 10, repo1, perm_model.AccessModeWrite)
+		assert.ErrorContains(t, err, "no permission to access the repository")
+	})
+
+	t.Run("handleLFSToken allows writes for authorized users", func(t *testing.T) {
+		err := handleLFSTokenTestPerm("upload", 2, repo1, perm_model.AccessModeWrite)
+		assert.NoError(t, err)
 	})
 }

tests/integration/wiki_test.go

@@ -13,52 +13,17 @@ import (
 
 	auth_model "code.gitea.io/gitea/models/auth"
 	"code.gitea.io/gitea/modules/git"
-	api "code.gitea.io/gitea/modules/structs"
+	"code.gitea.io/gitea/modules/git/gitcmd"
-	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/tests"
 
 	"github.com/PuerkitoBio/goquery"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
-func assertFileExist(t *testing.T, p string) {
+func TestRepoWikiPages(t *testing.T) {
-	exist, err := util.IsExist(p)
-	assert.NoError(t, err)
-	assert.True(t, exist)
-}
-
-func assertFileEqual(t *testing.T, p string, content []byte) {
-	bs, err := os.ReadFile(p)
-	assert.NoError(t, err)
-	assert.Equal(t, content, bs)
-}
-
-func TestRepoCloneWiki(t *testing.T) {
-	onGiteaRun(t, func(t *testing.T, u *url.URL) {
-		defer tests.PrepareTestEnv(t)()
-
-		dstPath := t.TempDir()
-
-		r := u.String() + "user2/repo1.wiki.git"
-		u, _ = url.Parse(r)
-		u.User = url.UserPassword("user2", userPassword)
-		t.Run("Clone", func(t *testing.T) {
-			assert.NoError(t, git.Clone(t.Context(), u.String(), dstPath, git.CloneRepoOptions{}))
-			assertFileEqual(t, filepath.Join(dstPath, "Home.md"), []byte("# Home page\n\nThis is the home page!\n"))
-			assertFileExist(t, filepath.Join(dstPath, "Page-With-Image.md"))
-			assertFileExist(t, filepath.Join(dstPath, "Page-With-Spaced-Name.md"))
-			assertFileExist(t, filepath.Join(dstPath, "images"))
-			assertFileExist(t, filepath.Join(dstPath, "files/Non-Renderable-File.zip"))
-			assertFileExist(t, filepath.Join(dstPath, "jpeg.jpg"))
-		})
-	})
-}
-
-func Test_RepoWikiPages(t *testing.T) {
 	defer tests.PrepareTestEnv(t)()
-
+	req := NewRequest(t, "GET", "/user2/repo1/wiki/?action=_pages")
-	url := "/user2/repo1/wiki/?action=_pages"
-	req := NewRequest(t, "GET", url)
 	resp := MakeRequest(t, req, http.StatusOK)
 
 	doc := NewHTMLParser(t, resp.Body)
@@ -74,45 +39,72 @@ func Test_RepoWikiPages(t *testing.T) {
 	})
 }
 
-func Test_WikiClone(t *testing.T) {
+func testRepoWikiCloneHTTP(t *testing.T, u *url.URL) {
-	onGiteaRun(t, func(t *testing.T, u *url.URL) {
+	// When proc-receive support is enabled globally, the HTTP receive-pack pre-check
-		username := "user2"
+	// must still require write access for wiki repositories. Exercise this with a
-		reponame := "repo1"
+	// normal wiki push because the regression is about the pre-check, not agit refs.
-		wikiPath := username + "/" + reponame + ".wiki.git"
+	require.True(t, git.DefaultFeatures().SupportProcReceive) // modern git should all support proc-receive
-		keyname := "my-testing-key"
-		baseAPITestContext := NewAPITestContext(t, username, "repo1", auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser)
 
-		u.Path = wikiPath
+	wikiURL := *u
+	wikiURL.Path = "/user2/repo1.wiki.git"
 
-		t.Run("Clone HTTP", func(t *testing.T) {
+	dstLocalPath := t.TempDir()
-			defer tests.PrintCurrentTest(t)()
 
-			dstLocalPath := t.TempDir()
+	// reader can clone
-			assert.NoError(t, git.Clone(t.Context(), u.String(), dstLocalPath, git.CloneRepoOptions{}))
+	wikiURL.User = url.UserPassword("user20", userPassword)
-			content, err := os.ReadFile(filepath.Join(dstLocalPath, "Home.md"))
+	require.NoError(t, git.Clone(t.Context(), wikiURL.String(), dstLocalPath, git.CloneRepoOptions{}))
-			assert.NoError(t, err)
+	_, _, runErr := gitcmd.NewCommand("fast-import").WithDir(dstLocalPath).WithStdinBytes([]byte(`commit refs/heads/master
-			assert.Equal(t, "# Home page\n\nThis is the home page!\n", string(content))
+committer unauthorized-user <user20@example.com> 1714310400 +0000
-		})
+data <<EOM
+dummy-message
+EOM
+from refs/heads/master^0
+M 100644 inline Home.md
+data <<EOF
+changed-content
+EOF
+`)).RunStdString(t.Context())
+	require.NoError(t, runErr)
 
-		t.Run("Clone SSH", func(t *testing.T) {
+	content, err := os.ReadFile(filepath.Join(dstLocalPath, "Home.md"))
-			defer tests.PrintCurrentTest(t)()
+	assert.NoError(t, err)
+	assert.Equal(t, "# Home page\n\nThis is the home page!\n", string(content))
 
-			dstLocalPath := t.TempDir()
+	// reader can't push
-			sshURL := createSSHUrl(wikiPath, u)
+	_, _, runErr = gitcmd.NewCommand("push", "origin", "refs/heads/master").WithDir(dstLocalPath).RunStdString(t.Context())
+	assert.Contains(t, runErr.Error(), "remote: Repository not found\n")
+	req := NewRequest(t, "GET", "/user2/repo1/wiki/raw/Home.md")
+	resp := MakeRequest(t, req, http.StatusOK)
+	assert.Contains(t, resp.Body.String(), "This is the home page!")
 
-			withKeyFile(t, keyname, func(keyFile string) {
+	// owner can push
-				var keyID int64
+	wikiURL.User = url.UserPassword("user2", userPassword)
-				t.Run("CreateUserKey", doAPICreateUserKey(baseAPITestContext, "test-key", keyFile, func(t *testing.T, key api.PublicKey) {
+	_, _, runErr = gitcmd.NewCommand("remote", "add", "origin-owner").AddDynamicArguments(wikiURL.String()).WithDir(dstLocalPath).RunStdString(t.Context())
-					keyID = key.ID
+	require.NoError(t, runErr)
-				}))
+	_, _, runErr = gitcmd.NewCommand("push", "origin-owner", "refs/heads/master").WithDir(dstLocalPath).RunStdString(t.Context())
-				assert.NotZero(t, keyID)
+	assert.NoError(t, runErr)
+	req = NewRequest(t, "GET", "/user2/repo1/wiki/raw/Home.md")
+	resp = MakeRequest(t, req, http.StatusOK)
+	assert.Equal(t, "changed-content", strings.TrimSpace(resp.Body.String()))
+}
 
-				// Setup clone folder
+func testRepoWikiCloneSSH(t *testing.T, u *url.URL) {
-				assert.NoError(t, git.Clone(t.Context(), sshURL.String(), dstLocalPath, git.CloneRepoOptions{}))
+	dstLocalPath := t.TempDir()
-				content, err := os.ReadFile(filepath.Join(dstLocalPath, "Home.md"))
+	baseAPITestContext := NewAPITestContext(t, "user2", "repo1", auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser)
-				assert.NoError(t, err)
+	sshURL := createSSHUrl("/user2/repo1.wiki.git", u)
-				assert.Equal(t, "# Home page\n\nThis is the home page!\n", string(content))
+
-			})
+	withKeyFile(t, "my-testing-key", func(keyFile string) {
-		})
+		t.Run("CreateUserKey", doAPICreateUserKey(baseAPITestContext, "test-key", keyFile))
+		assert.NoError(t, git.Clone(t.Context(), sshURL.String(), dstLocalPath, git.CloneRepoOptions{}))
+		content, err := os.ReadFile(filepath.Join(dstLocalPath, "Home.md"))
+		assert.NoError(t, err)
+		assert.Equal(t, "# Home page\n\nThis is the home page!\n", string(content))
+	})
+}
+
+func TestRepoWikiClonePush(t *testing.T) {
+	onGiteaRun(t, func(t *testing.T, u *url.URL) {
+		t.Run("SSH", func(t *testing.T) { testRepoWikiCloneSSH(t, u) })
+		t.Run("HTTP", func(t *testing.T) { testRepoWikiCloneHTTP(t, u) })
 	})
 }