Fix push-create SSH bugs (#10145) (#10151)

* Attempt to fix push-create SSH bugs

Signed-off-by: jolheiser <john.olheiser@gmail.com>

* Fix binding

Signed-off-by: jolheiser <john.olheiser@gmail.com>

* Invalid ctx

Signed-off-by: jolheiser <john.olheiser@gmail.com>
This commit is contained in:
John Olheiser 2020-02-05 08:50:06 -06:00 committed by GitHub
parent 0129e76ef5
commit 2cd2614eaa
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 33 additions and 1 deletions

View file

@ -12,6 +12,7 @@ import (
"net/url" "net/url"
"os" "os"
"os/exec" "os/exec"
"regexp"
"strconv" "strconv"
"strings" "strings"
"time" "time"
@ -72,6 +73,7 @@ var (
"git-receive-pack": models.AccessModeWrite, "git-receive-pack": models.AccessModeWrite,
lfsAuthenticateVerb: models.AccessModeNone, lfsAuthenticateVerb: models.AccessModeNone,
} }
alphaDashDotPattern = regexp.MustCompile(`[^\w-\.]`)
) )
func fail(userMessage, logMessage string, args ...interface{}) { func fail(userMessage, logMessage string, args ...interface{}) {
@ -147,6 +149,10 @@ func runServ(c *cli.Context) error {
username := strings.ToLower(rr[0]) username := strings.ToLower(rr[0])
reponame := strings.ToLower(strings.TrimSuffix(rr[1], ".git")) reponame := strings.ToLower(strings.TrimSuffix(rr[1], ".git"))
if alphaDashDotPattern.MatchString(reponame) {
fail("Invalid repo name", "Invalid repo name: %s", reponame)
}
if setting.EnablePprof || c.Bool("enable-pprof") { if setting.EnablePprof || c.Bool("enable-pprof") {
if err := os.MkdirAll(setting.PprofDataPath, os.ModePerm); err != nil { if err := os.MkdirAll(setting.PprofDataPath, os.ModePerm); err != nil {
fail("Error while trying to create PPROF_DATA_PATH", "Error while trying to create PPROF_DATA_PATH: %v", err) fail("Error while trying to create PPROF_DATA_PATH", "Error while trying to create PPROF_DATA_PATH: %v", err)

View file

@ -422,6 +422,9 @@ func doPushCreate(ctx APITestContext, u *url.URL) func(t *testing.T) {
tmpDir, err := ioutil.TempDir("", ctx.Reponame) tmpDir, err := ioutil.TempDir("", ctx.Reponame)
assert.NoError(t, err) assert.NoError(t, err)
_, err = git.NewCommand("clone", u.String()).RunInDir(tmpDir)
assert.Error(t, err)
err = git.InitRepository(tmpDir, false) err = git.InitRepository(tmpDir, false)
assert.NoError(t, err) assert.NoError(t, err)
@ -449,6 +452,13 @@ func doPushCreate(ctx APITestContext, u *url.URL) func(t *testing.T) {
_, err = git.NewCommand("remote", "add", "origin", u.String()).RunInDir(tmpDir) _, err = git.NewCommand("remote", "add", "origin", u.String()).RunInDir(tmpDir)
assert.NoError(t, err) assert.NoError(t, err)
invalidCtx := ctx
invalidCtx.Reponame = fmt.Sprintf("invalid/repo-tmp-push-create-%s", u.Scheme)
u.Path = invalidCtx.GitPath()
_, err = git.NewCommand("remote", "add", "invalid", u.String()).RunInDir(tmpDir)
assert.NoError(t, err)
// Push to create disabled // Push to create disabled
setting.Repository.EnablePushCreateUser = false setting.Repository.EnablePushCreateUser = false
_, err = git.NewCommand("push", "origin", "master").RunInDir(tmpDir) _, err = git.NewCommand("push", "origin", "master").RunInDir(tmpDir)
@ -456,6 +466,12 @@ func doPushCreate(ctx APITestContext, u *url.URL) func(t *testing.T) {
// Push to create enabled // Push to create enabled
setting.Repository.EnablePushCreateUser = true setting.Repository.EnablePushCreateUser = true
// Invalid repo
_, err = git.NewCommand("push", "invalid", "master").RunInDir(tmpDir)
assert.Error(t, err)
// Valid repo
_, err = git.NewCommand("push", "origin", "master").RunInDir(tmpDir) _, err = git.NewCommand("push", "origin", "master").RunInDir(tmpDir)
assert.NoError(t, err) assert.NoError(t, err)

View file

@ -68,7 +68,6 @@ func ServNoCommand(ctx *macaron.Context) {
// ServCommand returns information about the provided keyid // ServCommand returns information about the provided keyid
func ServCommand(ctx *macaron.Context) { func ServCommand(ctx *macaron.Context) {
// Although we provide the verbs we don't need them at present they're just for logging purposes
keyID := ctx.ParamsInt64(":keyid") keyID := ctx.ParamsInt64(":keyid")
ownerName := ctx.Params(":owner") ownerName := ctx.Params(":owner")
repoName := ctx.Params(":repo") repoName := ctx.Params(":repo")
@ -105,6 +104,17 @@ func ServCommand(ctx *macaron.Context) {
if err != nil { if err != nil {
if models.IsErrRepoNotExist(err) { if models.IsErrRepoNotExist(err) {
repoExist = false repoExist = false
for _, verb := range ctx.QueryStrings("verb") {
if "git-upload-pack" == verb {
// User is fetching/cloning a non-existent repository
ctx.JSON(http.StatusNotFound, map[string]interface{}{
"results": results,
"type": "ErrRepoNotExist",
"err": fmt.Sprintf("Cannot find repository: %s/%s", results.OwnerName, results.RepoName),
})
return
}
}
} else { } else {
log.Error("Unable to get repository: %s/%s Error: %v", results.OwnerName, results.RepoName, err) log.Error("Unable to get repository: %s/%s Error: %v", results.OwnerName, results.RepoName, err)
ctx.JSON(http.StatusInternalServerError, map[string]interface{}{ ctx.JSON(http.StatusInternalServerError, map[string]interface{}{