sharpetronics/gitea
1
0
Fork 0
mirror of https://github.com/go-gitea/gitea.git synced 2025-05-13 01:12:36 -04:00
Code Issues Projects Releases Wiki Activity

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

Browse Source 
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
Lunny Xiao 2025-05-09 09:17:08 -07:00 committed by GitHub
parent 8b16ab719c
commit ad271444e9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 149 additions and 76 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

89
cmd/serv.go
View File

@ -11,7 +11,6 @@ import (
"os"
"os/exec"
"path/filepath"
"regexp"
"strconv"
"strings"
"time"
@ -20,7 +19,7 @@ import (
asymkey_model "code.gitea.io/gitea/models/asymkey"
git_model "code.gitea.io/gitea/models/git"
"code.gitea.io/gitea/models/perm"
"code.gitea.io/gitea/modules/container"
"code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/json"
"code.gitea.io/gitea/modules/lfstransfer"
@ -37,14 +36,6 @@ import (
"github.com/urfave/cli/v2"
)
const (
verbUploadPack = "git-upload-pack"
verbUploadArchive = "git-upload-archive"
verbReceivePack = "git-receive-pack"
verbLfsAuthenticate = "git-lfs-authenticate"
verbLfsTransfer = "git-lfs-transfer"
)
// CmdServ represents the available serv sub-command.
var CmdServ = &cli.Command{
Name: "serv",
@ -78,22 +69,6 @@ func setup(ctx context.Context, debug bool) {
}
}
var (
// keep getAccessMode() in sync
allowedCommands = container.SetOf(
verbUploadPack,
verbUploadArchive,
verbReceivePack,
verbLfsAuthenticate,
verbLfsTransfer,
)
allowedCommandsLfs = container.SetOf(
verbLfsAuthenticate,
verbLfsTransfer,
)
alphaDashDotPattern = regexp.MustCompile(`[^\w-\.]`)
)
// fail prints message to stdout, it's mainly used for git serv and git hook commands.
// The output will be passed to git client and shown to user.
func fail(ctx context.Context, userMessage, logMsgFmt string, args ...any) error {
@ -139,19 +114,20 @@ func handleCliResponseExtra(extra private.ResponseExtra) error {
func getAccessMode(verb, lfsVerb string) perm.AccessMode {
switch verb {
case verbUploadPack, verbUploadArchive:
case git.CmdVerbUploadPack, git.CmdVerbUploadArchive:
return perm.AccessModeRead
case verbReceivePack:
case git.CmdVerbReceivePack:
return perm.AccessModeWrite
case verbLfsAuthenticate, verbLfsTransfer:
case git.CmdVerbLfsAuthenticate, git.CmdVerbLfsTransfer:
switch lfsVerb {
case "upload":
case git.CmdSubVerbLfsUpload:
return perm.AccessModeWrite
case "download":
case git.CmdSubVerbLfsDownload:
return perm.AccessModeRead
}
}
// should be unreachable
setting.PanicInDevOrTesting("unknown verb: %s %s", verb, lfsVerb)
return perm.AccessModeNone
}
@ -230,12 +206,12 @@ func runServ(c *cli.Context) error {
log.Debug("SSH_ORIGINAL_COMMAND: %s", os.Getenv("SSH_ORIGINAL_COMMAND"))
}
words, err := shellquote.Split(cmd)
sshCmdArgs, err := shellquote.Split(cmd)
if err != nil {
return fail(ctx, "Error parsing arguments", "Failed to parse arguments: %v", err)
}
if len(words) < 2 {
if len(sshCmdArgs) < 2 {
if git.DefaultFeatures().SupportProcReceive {
// for AGit Flow
if cmd == "ssh_info" {
@ -246,25 +222,21 @@ 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 {
repoPath := strings.TrimPrefix(sshCmdArgs[1], "/")
repoPathFields := strings.SplitN(repoPath, "/", 2)
if len(repoPathFields) != 2 {
return fail(ctx, "Invalid repository path", "Invalid repository path: %v", repoPath)
}
username := rr[0]
reponame := strings.TrimSuffix(rr[1], ".git")
username := repoPathFields[0]
reponame := strings.TrimSuffix(repoPathFields[1], ".git") // “the-repo-name" or "the-repo-name.wiki"
// LowerCase and trim the repoPath as that's how they are stored.
// This should be done after splitting the repoPath into username and reponame
// so that username and reponame are not affected.
repoPath = strings.ToLower(strings.TrimSpace(repoPath))
if alphaDashDotPattern.MatchString(reponame) {
if !repo.IsValidSSHAccessRepoName(reponame) {
return fail(ctx, "Invalid repo name", "Invalid repo name: %s", reponame)
}
@ -286,22 +258,23 @@ 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, lfsVerb := sshCmdArgs[0], ""
if !git.IsAllowedVerbForServe(verb) {
return fail(ctx, "Unknown git command", "Unknown git command %s", verb)
}
if git.IsAllowedVerbForServeLfs(verb) {
if !setting.LFS.StartServer {
return fail(ctx, "LFS Server is not enabled", "")
}
if verb == git.CmdVerbLfsTransfer && !setting.LFS.AllowPureSSH {
return fail(ctx, "LFS SSH transfer is not enabled", "")
}
if len(sshCmdArgs) > 2 {
lfsVerb = sshCmdArgs[2]
}
}
requestedMode := getAccessMode(verb, lfsVerb)
results, extra := private.ServCommand(ctx, keyID, username, reponame, requestedMode, verb, lfsVerb)
@ -310,7 +283,7 @@ func runServ(c *cli.Context) error {
}
// LFS SSH protocol
if verb == verbLfsTransfer {
if verb == git.CmdVerbLfsTransfer {
token, err := getLFSAuthToken(ctx, lfsVerb, results)
if err != nil {
return err
@ -319,7 +292,7 @@ func runServ(c *cli.Context) error {
}
// LFS token authentication
if verb == verbLfsAuthenticate {
if verb == git.CmdVerbLfsAuthenticate {
url := fmt.Sprintf("%s%s/%s.git/info/lfs", setting.AppURL, url.PathEscape(results.OwnerName), url.PathEscape(results.RepoName))
token, err := getLFSAuthToken(ctx, lfsVerb, results)

27
models/repo/repo.go
View File

@ -64,18 +64,18 @@ func (err ErrRepoIsArchived) Error() string {
}
type globalVarsStruct struct {
validRepoNamePattern *regexp.Regexp
invalidRepoNamePattern *regexp.Regexp
reservedRepoNames []string
reservedRepoPatterns []string
validRepoNamePattern *regexp.Regexp
invalidRepoNamePattern *regexp.Regexp
reservedRepoNames []string
reservedRepoNamePatterns []string
}
var globalVars = sync.OnceValue(func() *globalVarsStruct {
return &globalVarsStruct{
validRepoNamePattern: regexp.MustCompile(`[-.\w]+`),
invalidRepoNamePattern: regexp.MustCompile(`[.]{2,}`),
reservedRepoNames: []string{".", "..", "-"},
reservedRepoPatterns: []string{"*.git", "*.wiki", "*.rss", "*.atom"},
validRepoNamePattern: regexp.MustCompile(`^[-.\w]+$`),
invalidRepoNamePattern: regexp.MustCompile(`[.]{2,}`),
reservedRepoNames: []string{".", "..", "-"},
reservedRepoNamePatterns: []string{"*.wiki", "*.git", "*.rss", "*.atom"},
}
})
@ -86,7 +86,16 @@ func IsUsableRepoName(name string) error {
// Note: usually this error is normally caught up earlier in the UI
return db.ErrNameCharsNotAllowed{Name: name}
}
return db.IsUsableName(vars.reservedRepoNames, vars.reservedRepoPatterns, name)
return db.IsUsableName(vars.reservedRepoNames, vars.reservedRepoNamePatterns, name)
}
// IsValidSSHAccessRepoName is like IsUsableRepoName, but it allows "*.wiki" because wiki repo needs to be accessed in SSH code
func IsValidSSHAccessRepoName(name string) bool {
vars := globalVars()
if !vars.validRepoNamePattern.MatchString(name) || vars.invalidRepoNamePattern.MatchString(name) {
return false
}
return db.IsUsableName(vars.reservedRepoNames, vars.reservedRepoNamePatterns[1:], name) == nil
}
// TrustModelType defines the types of trust model for this repository

15
models/repo/repo_test.go
View File

@ -216,8 +216,23 @@ func TestIsUsableRepoName(t *testing.T) {
assert.Error(t, IsUsableRepoName("-"))
assert.Error(t, IsUsableRepoName("🌞"))
assert.Error(t, IsUsableRepoName("the/repo"))
assert.Error(t, IsUsableRepoName("the..repo"))
assert.Error(t, IsUsableRepoName("foo.wiki"))
assert.Error(t, IsUsableRepoName("foo.git"))
assert.Error(t, IsUsableRepoName("foo.RSS"))
}
func TestIsValidSSHAccessRepoName(t *testing.T) {
assert.True(t, IsValidSSHAccessRepoName("a"))
assert.True(t, IsValidSSHAccessRepoName("-1_."))
assert.True(t, IsValidSSHAccessRepoName(".profile"))
assert.True(t, IsValidSSHAccessRepoName("foo.wiki"))
assert.False(t, IsValidSSHAccessRepoName("-"))
assert.False(t, IsValidSSHAccessRepoName("🌞"))
assert.False(t, IsValidSSHAccessRepoName("the/repo"))
assert.False(t, IsValidSSHAccessRepoName("the..repo"))
assert.False(t, IsValidSSHAccessRepoName("foo.git"))
assert.False(t, IsValidSSHAccessRepoName("foo.RSS"))
}

36
modules/git/cmdverb.go Normal file
View File

@ -0,0 +1,36 @@
// Copyright 2025 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package git
const (
CmdVerbUploadPack = "git-upload-pack"
CmdVerbUploadArchive = "git-upload-archive"
CmdVerbReceivePack = "git-receive-pack"
CmdVerbLfsAuthenticate = "git-lfs-authenticate"
CmdVerbLfsTransfer = "git-lfs-transfer"
CmdSubVerbLfsUpload = "upload"
CmdSubVerbLfsDownload = "download"
)
func IsAllowedVerbForServe(verb string) bool {
switch verb {
case CmdVerbUploadPack,
CmdVerbUploadArchive,
CmdVerbReceivePack,
CmdVerbLfsAuthenticate,
CmdVerbLfsTransfer:
return true
}
return false
}
func IsAllowedVerbForServeLfs(verb string) bool {
switch verb {
case CmdVerbLfsAuthenticate,
CmdVerbLfsTransfer:
return true
}
return false
}

10
modules/private/serv.go
View File

@ -46,18 +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)
// reqURL += "&lfs_verb=" + url.QueryEscape(lfsVerb) // TODO: actually there is no use of this parameter. In the future, the URL construction should be more flexible
_ = lfsVerb
req := newInternalRequestAPI(ctx, reqURL, "GET")
return requestJSONResp(req, &ServCommandResults{})
}

8
routers/private/serv.go
View File

@ -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.CmdVerbReceivePack {
mode = perm.AccessModeRead
}

40
tests/integration/git_general_test.go
View File

@ -11,8 +11,10 @@ import (
"net/http"
"net/url"
"os"
"os/exec"
"path"
"path/filepath"
"slices"
"strconv"
"testing"
"time"
@ -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,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 +144,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(t.Context(), 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(t.Context(), 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))