Skip to content

Commit 04f83f5

Browse files
wxiaoguangCharles
authored andcommitted
Fix a bug when uploading file via lfs ssh command 1.23 (#34408) (#34411)
Partially backport #34408
1 parent 909ef86 commit 04f83f5

File tree

2 files changed

+46
-3
lines changed

2 files changed

+46
-3
lines changed

routers/private/serv.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ func ServCommand(ctx *context.PrivateContext) {
8181
ownerName := ctx.Params(":owner")
8282
repoName := ctx.Params(":repo")
8383
mode := perm.AccessMode(ctx.FormInt("mode"))
84+
verb := ctx.FormString("verb")
8485

8586
// Set the basic parts of the results to return
8687
results := private.ServCommandResults{
@@ -296,8 +297,11 @@ func ServCommand(ctx *context.PrivateContext) {
296297
return
297298
}
298299
} else {
299-
// Because of the special ref "refs/for" we will need to delay write permission check
300-
if git.DefaultFeatures().SupportProcReceive && unitType == unit.TypeCode {
300+
// Because of the special ref "refs/for" (AGit) we will need to delay write permission check,
301+
// AGit flow needs to write its own ref when the doer has "reader" permission (allowing to create PR).
302+
// The real permission check is done in HookPreReceive (routers/private/hook_pre_receive.go).
303+
// Here it should relax the permission check for "git push (git-receive-pack)", but not for others like LFS operations.
304+
if git.DefaultFeatures().SupportProcReceive && unitType == unit.TypeCode && verb == "git-receive-pack" {
301305
mode = perm.AccessModeRead
302306
}
303307

tests/integration/git_test.go

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,10 @@ import (
1111
"net/http"
1212
"net/url"
1313
"os"
14+
"os/exec"
1415
"path"
1516
"path/filepath"
17+
"slices"
1618
"strconv"
1719
"testing"
1820
"time"
@@ -33,7 +35,9 @@ import (
3335
files_service "code.gitea.io/gitea/services/repository/files"
3436
"code.gitea.io/gitea/tests"
3537

38+
"github.com/kballard/go-shellquote"
3639
"github.com/stretchr/testify/assert"
40+
"github.com/stretchr/testify/require"
3741
)
3842

3943
const (
@@ -106,7 +110,12 @@ func testGit(t *testing.T, u *url.URL) {
106110

107111
// Setup key the user ssh key
108112
withKeyFile(t, keyname, func(keyFile string) {
109-
t.Run("CreateUserKey", doAPICreateUserKey(sshContext, "test-key", keyFile))
113+
var keyID int64
114+
t.Run("CreateUserKey", doAPICreateUserKey(sshContext, "test-key", keyFile, func(t *testing.T, key api.PublicKey) {
115+
keyID = key.ID
116+
}))
117+
assert.NotZero(t, keyID)
118+
t.Run("LFSAccessTest", doSSHLFSAccessTest(sshContext, keyID))
110119

111120
// Setup remote link
112121
// TODO: get url from api
@@ -136,6 +145,36 @@ func testGit(t *testing.T, u *url.URL) {
136145
})
137146
}
138147

148+
func doSSHLFSAccessTest(_ APITestContext, keyID int64) func(*testing.T) {
149+
return func(t *testing.T) {
150+
sshCommand := os.Getenv("GIT_SSH_COMMAND") // it is set in withKeyFile
151+
sshCmdParts, err := shellquote.Split(sshCommand) // and parse the ssh command to construct some mocked arguments
152+
require.NoError(t, err)
153+
154+
t.Run("User2AccessOwned", func(t *testing.T) {
155+
sshCmdUser2Self := append(slices.Clone(sshCmdParts),
156+
"-p", strconv.Itoa(setting.SSH.ListenPort), "git@"+setting.SSH.ListenHost,
157+
"git-lfs-authenticate", "user2/repo1.git", "upload", // accessible to own repo
158+
)
159+
cmd := exec.CommandContext(git.DefaultContext, sshCmdUser2Self[0], sshCmdUser2Self[1:]...)
160+
_, err := cmd.Output()
161+
assert.NoError(t, err) // accessible, no error
162+
})
163+
164+
t.Run("User2AccessOther", func(t *testing.T) {
165+
sshCmdUser2Other := append(slices.Clone(sshCmdParts),
166+
"-p", strconv.Itoa(setting.SSH.ListenPort), "git@"+setting.SSH.ListenHost,
167+
"git-lfs-authenticate", "user5/repo4.git", "upload", // inaccessible to other's (user5/repo4)
168+
)
169+
cmd := exec.CommandContext(git.DefaultContext, sshCmdUser2Other[0], sshCmdUser2Other[1:]...)
170+
_, err := cmd.Output()
171+
var errExit *exec.ExitError
172+
require.ErrorAs(t, err, &errExit) // inaccessible, error
173+
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))
174+
})
175+
}
176+
}
177+
139178
func ensureAnonymousClone(t *testing.T, u *url.URL) {
140179
dstLocalPath := t.TempDir()
141180
t.Run("CloneAnonymous", doGitClone(dstLocalPath, u))

0 commit comments

Comments
 (0)