Fix a bug when uploading file via lfs ssh command 1.23 (#34408) (#34411)

Partially backport #34408

Author: wxiaoguang

routers/private/serv.go

@@ -81,6 +81,7 @@ func ServCommand(ctx *context.PrivateContext) {
 	ownerName := ctx.PathParam(":owner")
 	repoName := ctx.PathParam(":repo")
 	mode := perm.AccessMode(ctx.FormInt("mode"))
+	verb := ctx.FormString("verb")
 
 	// Set the basic parts of the results to return
 	results := private.ServCommandResults{
@@ -295,8 +296,11 @@ func ServCommand(ctx *context.PrivateContext) {
 				return
 			}
 		} else {
-			// Because of the special ref "refs/for" we will need to delay write permission check
-			if git.DefaultFeatures().SupportProcReceive && unitType == unit.TypeCode {
+			// Because of the special ref "refs/for" (AGit) we will need to delay write permission check,
+			// AGit flow needs to write its own ref when the doer has "reader" permission (allowing to create PR).
+			// The real permission check is done in HookPreReceive (routers/private/hook_pre_receive.go).
+			// Here it should relax the permission check for "git push (git-receive-pack)", but not for others like LFS operations.
+			if git.DefaultFeatures().SupportProcReceive && unitType == unit.TypeCode && verb == "git-receive-pack" {
 				mode = perm.AccessModeRead
 			}
 

tests/integration/git_general_test.go

@@ -11,8 +11,10 @@ import (
 	"net/http"
 	"net/url"
 	"os"
+	"os/exec"
 	"path"
 	"path/filepath"
+	"slices"
 	"strconv"
 	"testing"
 	"time"
@@ -31,7 +33,9 @@ import (
 	gitea_context "code.gitea.io/gitea/services/context"
 	"code.gitea.io/gitea/tests"
 
+	"github.com/kballard/go-shellquote"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 const (
@@ -105,7 +109,12 @@ func testGitGeneral(t *testing.T, u *url.URL) {
 
 		// Setup key the user ssh key
 		withKeyFile(t, keyname, func(keyFile string) {
-			t.Run("CreateUserKey", doAPICreateUserKey(sshContext, "test-key", keyFile))
+			var keyID int64
+			t.Run("CreateUserKey", doAPICreateUserKey(sshContext, "test-key", keyFile, func(t *testing.T, key api.PublicKey) {
+				keyID = key.ID
+			}))
+			assert.NotZero(t, keyID)
+			t.Run("LFSAccessTest", doSSHLFSAccessTest(sshContext, keyID))
 
 			// Setup remote link
 			// TODO: get url from api
@@ -136,6 +145,36 @@ func testGitGeneral(t *testing.T, u *url.URL) {
 	})
 }
 
+func doSSHLFSAccessTest(_ APITestContext, keyID int64) func(*testing.T) {
+	return func(t *testing.T) {
+		sshCommand := os.Getenv("GIT_SSH_COMMAND")       // it is set in withKeyFile
+		sshCmdParts, err := shellquote.Split(sshCommand) // and parse the ssh command to construct some mocked arguments
+		require.NoError(t, err)
+
+		t.Run("User2AccessOwned", func(t *testing.T) {
+			sshCmdUser2Self := append(slices.Clone(sshCmdParts),
+				"-p", strconv.Itoa(setting.SSH.ListenPort), "git@"+setting.SSH.ListenHost,
+				"git-lfs-authenticate", "user2/repo1.git", "upload", // accessible to own repo
+			)
+			cmd := exec.CommandContext(git.DefaultContext, sshCmdUser2Self[0], sshCmdUser2Self[1:]...)
+			_, err := cmd.Output()
+			assert.NoError(t, err) // accessible, no error
+		})
+
+		t.Run("User2AccessOther", func(t *testing.T) {
+			sshCmdUser2Other := append(slices.Clone(sshCmdParts),
+				"-p", strconv.Itoa(setting.SSH.ListenPort), "git@"+setting.SSH.ListenHost,
+				"git-lfs-authenticate", "user5/repo4.git", "upload", // inaccessible to other's (user5/repo4)
+			)
+			cmd := exec.CommandContext(git.DefaultContext, sshCmdUser2Other[0], sshCmdUser2Other[1:]...)
+			_, err := cmd.Output()
+			var errExit *exec.ExitError
+			require.ErrorAs(t, err, &errExit) // inaccessible, error
+			assert.Contains(t, string(errExit.Stderr), fmt.Sprintf("User: 2:user2 with Key: %d:test-key is not authorized to write to user5/repo4.", keyID))
+		})
+	}
+}
+
 func ensureAnonymousClone(t *testing.T, u *url.URL) {
 	dstLocalPath := t.TempDir()
 	t.Run("CloneAnonymous", doGitClone(dstLocalPath, u))