fix

Author: wxiaoguang

modules/private/serv.go

@@ -46,17 +46,16 @@ type ServCommandResults struct {
 }
 
 // ServCommand preps for a serv call
-func ServCommand(ctx context.Context, keyID int64, ownerName, repoName string, mode perm.AccessMode, verbs ...string) (*ServCommandResults, ResponseExtra) {
+func ServCommand(ctx context.Context, keyID int64, ownerName, repoName string, mode perm.AccessMode, verb, lfsVerb string) (*ServCommandResults, ResponseExtra) {
 	reqURL := setting.LocalURL + fmt.Sprintf("api/internal/serv/command/%d/%s/%s?mode=%d",
 		keyID,
 		url.PathEscape(ownerName),
 		url.PathEscape(repoName),
 		mode,
 	)
-	for _, verb := range verbs {
-		if verb != "" {
-			reqURL += "&verb=" + url.QueryEscape(verb)
-		}
+	reqURL += "&verb=" + url.QueryEscape(verb)
+	if lfsVerb != "" {
+		reqURL += "&lfs_verb=" + url.QueryEscape(lfsVerb)
 	}
 	req := newInternalRequestAPI(ctx, reqURL, "GET")
 	return requestJSONResp(req, &ServCommandResults{})

routers/private/serv.go

@@ -81,7 +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
+	verb := ctx.FormString("verb")
 
 	// Set the basic parts of the results to return
 	results := private.ServCommandResults{
@@ -296,7 +296,10 @@ func ServCommand(ctx *context.PrivateContext) {
 				return
 			}
 		} else {
-			// Because of the special ref "refs/for" we will need to delay write permission check
+			// 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 && verb == "git-receive-pack" && unitType == unit.TypeCode {
 				mode = perm.AccessModeRead
 			}

tests/integration/git_general_test.go

@@ -4,7 +4,6 @@
 package integration
 
 import (
-	"bytes"
 	"encoding/hex"
 	"fmt"
 	"io"
@@ -112,7 +111,7 @@ func testGitGeneral(t *testing.T, u *url.URL) {
 			t.Run("CreateUserKey", doAPICreateUserKey(sshContext, "test-key", keyFile, func(t *testing.T, key api.PublicKey) {
 				keyID = key.ID
 			}))
-			assert.Greater(t, keyID, int64(0))
+			assert.NotZero(t, keyID)
 
 			// Setup remote link
 			// TODO: get url from api
@@ -144,26 +143,22 @@ func testGitGeneral(t *testing.T, u *url.URL) {
 	})
 }
 
-func doLFSUploadTest(ctx APITestContext, keyID int64) func(*testing.T) {
+func doLFSUploadTest(_ 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()
+		sshCommand := os.Getenv("GIT_SSH_COMMAND")       // it is set in withKeyFile and ensure correctly
+		sshCmdParts, err := shellquote.Split(sshCommand) // and parse the ssh command to construct some mocked arguments
+		require.NoError(t, err)
 
-		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))
+		t.Run("User2AccessUser5Repo4", func(t *testing.T) {
+			sshCmdParts = append(sshCmdParts,
+				"-p", strconv.Itoa(setting.SSH.ListenPort), "git@"+setting.SSH.ListenHost,
+				"git-lfs-authenticate", "user5/repo4.git", "upload", // user2's key upload lfs file to user5/repo4
+			)
+			cmd := exec.CommandContext(t.Context(), sshCmdParts[0], sshCmdParts[1:]...)
+			out, err := cmd.Output()
+			assert.ErrorContains(t, err, "exit status 1")
+			assert.Contains(t, string(out), fmt.Sprintf("User: 2:user2 with Key: %d:test-key is not authorized to write to user5/repo4.", keyID))
+		})
 	}
 }