Fix a bug when uploading file via lfs ssh command

Author: lunny

cmd/serv.go

@@ -246,11 +246,7 @@ func runServ(c *cli.Context) error {
 		return fail(ctx, "Too few arguments", "Too few arguments in cmd: %s", cmd)
 	}
 
-	verb := words[0]
 	repoPath := strings.TrimPrefix(words[1], "/")
-
-	var lfsVerb string
-
 	rr := strings.SplitN(repoPath, "/", 2)
 	if len(rr) != 2 {
 		return fail(ctx, "Invalid repository path", "Invalid repository path: %v", repoPath)
@@ -286,22 +282,25 @@ func runServ(c *cli.Context) error {
 		}()
 	}
 
-	if allowedCommands.Contains(verb) {
-		if allowedCommandsLfs.Contains(verb) {
-			if !setting.LFS.StartServer {
-				return fail(ctx, "LFS Server is not enabled", "")
-			}
-			if verb == verbLfsTransfer && !setting.LFS.AllowPureSSH {
-				return fail(ctx, "LFS SSH transfer is not enabled", "")
-			}
-			if len(words) > 2 {
-				lfsVerb = words[2]
-			}
-		}
-	} else {
+	verb := words[0]
+	var lfsVerb string
+
+	if !allowedCommands.Contains(verb) {
 		return fail(ctx, "Unknown git command", "Unknown git command %s", verb)
 	}
 
+	if allowedCommandsLfs.Contains(verb) {
+		if !setting.LFS.StartServer {
+			return fail(ctx, "LFS Server is not enabled", "")
+		}
+		if verb == verbLfsTransfer && !setting.LFS.AllowPureSSH {
+			return fail(ctx, "LFS SSH transfer is not enabled", "")
+		}
+		if len(words) > 2 {
+			lfsVerb = words[2]
+		}
+	}
+
 	requestedMode := getAccessMode(verb, lfsVerb)
 
 	results, extra := private.ServCommand(ctx, keyID, username, reponame, requestedMode, verb, lfsVerb)

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") // there should be two verbs, but we only care about the first one
 
 	// Set the basic parts of the results to return
 	results := private.ServCommandResults{
@@ -296,7 +297,7 @@ func ServCommand(ctx *context.PrivateContext) {
 			}
 		} else {
 			// Because of the special ref "refs/for" we will need to delay write permission check
-			if git.DefaultFeatures().SupportProcReceive && unitType == unit.TypeCode {
+			if git.DefaultFeatures().SupportProcReceive && verb == "git-receive-pack" && unitType == unit.TypeCode {
 				mode = perm.AccessModeRead
 			}
 

tests/integration/git_general_test.go

@@ -4,13 +4,15 @@
 package integration
 
 import (
+	"bytes"
 	"encoding/hex"
 	"fmt"
 	"io"
 	mathRand "math/rand/v2"
 	"net/http"
 	"net/url"
 	"os"
+	"os/exec"
 	"path"
 	"path/filepath"
 	"strconv"
@@ -30,6 +32,7 @@ import (
 	api "code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/tests"
 
+	"github.com/kballard/go-shellquote"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -105,7 +108,11 @@ 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.Greater(t, keyID, int64(0))
 
 			// Setup remote link
 			// TODO: get url from api
@@ -132,10 +139,34 @@ func testGitGeneral(t *testing.T, u *url.URL) {
 			})
 
 			t.Run("PushCreate", doPushCreate(sshContext, sshURL))
+			t.Run("LFSUploadTest", doLFSUploadTest(sshContext, keyID))
 		})
 	})
 }
 
+func doLFSUploadTest(ctx APITestContext, keyID int64) func(*testing.T) {
+	return func(t *testing.T) {
+		// This is set in withKeyFile and ensure correctly
+		sshCommand := os.Getenv("GIT_SSH_COMMAND")
+
+		// We really have to split on the arguments and pass them individually.
+		sshOptions, err := shellquote.Split(sshCommand)
+		assert.NoError(t, err)
+
+		sshOptions = append(sshOptions, "-p "+strconv.Itoa(setting.SSH.ListenPort), "git@"+setting.SSH.ListenHost)
+		// user2's key upload lfs file to user5/repo4
+		sshOptions = append(sshOptions, "git-lfs-authenticate", "user5/repo4.git", "upload")
+
+		cmd := exec.CommandContext(t.Context(), sshOptions[0], sshOptions[1:]...)
+		stderr := bytes.Buffer{}
+		cmd.Stderr = &stderr
+		err = cmd.Run()
+
+		assert.ErrorContains(t, err, "exit status 1")
+		assert.Contains(t, stderr.String(), 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))