From 44b8b07631666e3ae691149bdba31ca0f51569f5 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Fri, 25 Jun 2021 16:28:55 +0200 Subject: [PATCH 01/12] Add tag protection (#15629) * Added tag protection in hook. * Prevent UI tag creation if protected. * Added settings page. * Added tests. * Added suggestions. * Moved tests. * Use individual errors. * Removed unneeded methods. * Switched delete selector. * Changed method names. * No reason to be unique. * Allow editing of protected tags. * Removed unique key from migration. * Added docs page. * Changed date. * Respond with 404 to not found tags. * Replaced glob with regex pattern. * Added support for glob and regex pattern. * Updated documentation. * Changed white* to allow*. * Fixed edit button link. * Added cancel button. Co-authored-by: zeripath Co-authored-by: Lunny Xiao --- cmd/hook.go | 8 +- .../doc/advanced/protected-tags.en-us.md | 57 +++ integrations/mirror_pull_test.go | 2 + integrations/repo_tag_test.go | 74 ++++ models/error.go | 15 + models/migrations/migrations.go | 2 + models/migrations/v186.go | 26 ++ models/models.go | 1 + models/protected_tag.go | 131 +++++++ models/protected_tag_test.go | 162 ++++++++ models/repo.go | 1 + modules/validation/binding.go | 59 ++- modules/validation/binding_test.go | 7 +- modules/validation/regex_pattern_test.go | 60 +++ modules/web/middleware/binding.go | 2 + options/locale/locale_en-US.ini | 18 +- routers/private/hook.go | 353 ++++++++++-------- routers/web/repo/release.go | 16 + routers/web/repo/setting.go | 1 + routers/web/repo/tag.go | 182 +++++++++ routers/web/web.go | 9 + services/forms/repo_tag_form.go | 27 ++ services/release/release.go | 27 +- services/release/release_test.go | 26 ++ templates/repo/settings/nav.tmpl | 1 + templates/repo/settings/navbar.tmpl | 3 + templates/repo/settings/tags.tmpl | 132 +++++++ 27 files changed, 1220 insertions(+), 182 deletions(-) create mode 100644 docs/content/doc/advanced/protected-tags.en-us.md create mode 100644 integrations/repo_tag_test.go create mode 100644 models/migrations/v186.go create mode 100644 models/protected_tag.go create mode 100644 models/protected_tag_test.go create mode 100644 modules/validation/regex_pattern_test.go create mode 100644 routers/web/repo/tag.go create mode 100644 services/forms/repo_tag_form.go create mode 100644 templates/repo/settings/tags.tmpl diff --git a/cmd/hook.go b/cmd/hook.go index 312c9a14fc56..2fbbfb4d21ae 100644 --- a/cmd/hook.go +++ b/cmd/hook.go @@ -221,8 +221,8 @@ Gitea or set your environment appropriately.`, "") total++ lastline++ - // If the ref is a branch, check if it's protected - if strings.HasPrefix(refFullName, git.BranchPrefix) { + // If the ref is a branch or tag, check if it's protected + if strings.HasPrefix(refFullName, git.BranchPrefix) || strings.HasPrefix(refFullName, git.TagPrefix) { oldCommitIDs[count] = oldCommitID newCommitIDs[count] = newCommitID refFullNames[count] = refFullName @@ -230,7 +230,7 @@ Gitea or set your environment appropriately.`, "") fmt.Fprintf(out, "*") if count >= hookBatchSize { - fmt.Fprintf(out, " Checking %d branches\n", count) + fmt.Fprintf(out, " Checking %d references\n", count) hookOptions.OldCommitIDs = oldCommitIDs hookOptions.NewCommitIDs = newCommitIDs @@ -261,7 +261,7 @@ Gitea or set your environment appropriately.`, "") hookOptions.NewCommitIDs = newCommitIDs[:count] hookOptions.RefFullNames = refFullNames[:count] - fmt.Fprintf(out, " Checking %d branches\n", count) + fmt.Fprintf(out, " Checking %d references\n", count) statusCode, msg := private.HookPreReceive(username, reponame, hookOptions) switch statusCode { diff --git a/docs/content/doc/advanced/protected-tags.en-us.md b/docs/content/doc/advanced/protected-tags.en-us.md new file mode 100644 index 000000000000..36e6e169753c --- /dev/null +++ b/docs/content/doc/advanced/protected-tags.en-us.md @@ -0,0 +1,57 @@ +--- +date: "2021-05-14T00:00:00-00:00" +title: "Protected tags" +slug: "protected-tags" +weight: 45 +toc: false +draft: false +menu: + sidebar: + parent: "advanced" + name: "Protected tags" + weight: 45 + identifier: "protected-tags" +--- + +# Protected tags + +Protected tags allow control over who has permission to create or update git tags. Each rule allows you to match either an individual tag name, or use an appropriate pattern to control multiple tags at once. + +**Table of Contents** + +{{< toc >}} + +## Setting up protected tags + +To protect a tag, you need to follow these steps: + +1. Go to the repository’s **Settings** > **Tags** page. +1. Type a pattern to match a name. You can use a single name, a [glob pattern](https://pkg.go.dev/github.com/gobwas/glob#Compile) or a regular expression. +1. Choose the allowed users and/or teams. If you leave these fields empty noone is allowed to create or modify this tag. +1. Select **Save** to save the configuration. + +## Pattern protected tags + +The pattern uses [glob](https://pkg.go.dev/github.com/gobwas/glob#Compile) or regular expressions to match a tag name. For regular expressions you need to enclose the pattern in slashes. + +Examples: + +| Type | Pattern Protected Tag | Possible Matching Tags | +| ----- | ------------------------ | --------------------------------------- | +| Glob | `v*` | `v`, `v-1`, `version2` | +| Glob | `v[0-9]` | `v0`, `v1` up to `v9` | +| Glob | `*-release` | `2.1-release`, `final-release` | +| Glob | `gitea` | only `gitea` | +| Glob | `*gitea*` | `gitea`, `2.1-gitea`, `1_gitea-release` | +| Glob | `{v,rel}-*` | `v-`, `v-1`, `v-final`, `rel-`, `rel-x` | +| Glob | `*` | matches all possible tag names | +| Regex | `/\Av/` | `v`, `v-1`, `version2` | +| Regex | `/\Av[0-9]\z/` | `v0`, `v1` up to `v9` | +| Regex | `/\Av\d+\.\d+\.\d+\z/` | `v1.0.17`, `v2.1.0` | +| Regex | `/\Av\d+(\.\d+){0,2}\z/` | `v1`, `v2.1`, `v1.2.34` | +| Regex | `/-release\z/` | `2.1-release`, `final-release` | +| Regex | `/gitea/` | `gitea`, `2.1-gitea`, `1_gitea-release` | +| Regex | `/\Agitea\z/` | only `gitea` | +| Regex | `/^gitea$/` | only `gitea` | +| Regex | `/\A(v\|rel)-/` | `v-`, `v-1`, `v-final`, `rel-`, `rel-x` | +| Regex | `/.+/` | matches all possible tag names | diff --git a/integrations/mirror_pull_test.go b/integrations/mirror_pull_test.go index 0e4da74fcf42..3908f355575d 100644 --- a/integrations/mirror_pull_test.go +++ b/integrations/mirror_pull_test.go @@ -59,7 +59,9 @@ func TestMirrorPull(t *testing.T) { assert.NoError(t, release_service.CreateRelease(gitRepo, &models.Release{ RepoID: repo.ID, + Repo: repo, PublisherID: user.ID, + Publisher: user, TagName: "v0.2", Target: "master", Title: "v0.2 is released", diff --git a/integrations/repo_tag_test.go b/integrations/repo_tag_test.go new file mode 100644 index 000000000000..eb3f2b47fb9e --- /dev/null +++ b/integrations/repo_tag_test.go @@ -0,0 +1,74 @@ +// Copyright 2021 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package integrations + +import ( + "io/ioutil" + "net/url" + "testing" + + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/util" + "code.gitea.io/gitea/services/release" + + "github.com/stretchr/testify/assert" +) + +func TestCreateNewTagProtected(t *testing.T) { + defer prepareTestEnv(t)() + + repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1}).(*models.Repository) + owner := models.AssertExistsAndLoadBean(t, &models.User{ID: repo.OwnerID}).(*models.User) + + t.Run("API", func(t *testing.T) { + defer PrintCurrentTest(t)() + + err := release.CreateNewTag(owner, repo, "master", "v-1", "first tag") + assert.NoError(t, err) + + err = models.InsertProtectedTag(&models.ProtectedTag{ + RepoID: repo.ID, + NamePattern: "v-*", + }) + assert.NoError(t, err) + err = models.InsertProtectedTag(&models.ProtectedTag{ + RepoID: repo.ID, + NamePattern: "v-1.1", + AllowlistUserIDs: []int64{repo.OwnerID}, + }) + assert.NoError(t, err) + + err = release.CreateNewTag(owner, repo, "master", "v-2", "second tag") + assert.Error(t, err) + assert.True(t, models.IsErrProtectedTagName(err)) + + err = release.CreateNewTag(owner, repo, "master", "v-1.1", "third tag") + assert.NoError(t, err) + }) + + t.Run("Git", func(t *testing.T) { + onGiteaRun(t, func(t *testing.T, u *url.URL) { + username := "user2" + httpContext := NewAPITestContext(t, username, "repo1") + + dstPath, err := ioutil.TempDir("", httpContext.Reponame) + assert.NoError(t, err) + defer util.RemoveAll(dstPath) + + u.Path = httpContext.GitPath() + u.User = url.UserPassword(username, userPassword) + + doGitClone(dstPath, u)(t) + + _, err = git.NewCommand("tag", "v-2").RunInDir(dstPath) + assert.NoError(t, err) + + _, err = git.NewCommand("push", "--tags").RunInDir(dstPath) + assert.Error(t, err) + assert.Contains(t, err.Error(), "Tag v-2 is protected") + }) + }) +} diff --git a/models/error.go b/models/error.go index 501bf8686936..513effdb0293 100644 --- a/models/error.go +++ b/models/error.go @@ -985,6 +985,21 @@ func (err ErrInvalidTagName) Error() string { return fmt.Sprintf("release tag name is not valid [tag_name: %s]", err.TagName) } +// ErrProtectedTagName represents a "ProtectedTagName" kind of error. +type ErrProtectedTagName struct { + TagName string +} + +// IsErrProtectedTagName checks if an error is a ErrProtectedTagName. +func IsErrProtectedTagName(err error) bool { + _, ok := err.(ErrProtectedTagName) + return ok +} + +func (err ErrProtectedTagName) Error() string { + return fmt.Sprintf("release tag name is protected [tag_name: %s]", err.TagName) +} + // ErrRepoFileAlreadyExists represents a "RepoFileAlreadyExist" kind of error. type ErrRepoFileAlreadyExists struct { Path string diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 4e17a6a2c8a2..978ba6368f95 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -321,6 +321,8 @@ var migrations = []Migration{ NewMigration("Rename Task errors to message", renameTaskErrorsToMessage), // v185 -> v186 NewMigration("Add new table repo_archiver", addRepoArchiver), + // v186 -> v187 + NewMigration("Create protected tag table", createProtectedTagTable), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v186.go b/models/migrations/v186.go new file mode 100644 index 000000000000..eb6ec7118cd7 --- /dev/null +++ b/models/migrations/v186.go @@ -0,0 +1,26 @@ +// Copyright 2021 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package migrations + +import ( + "code.gitea.io/gitea/modules/timeutil" + + "xorm.io/xorm" +) + +func createProtectedTagTable(x *xorm.Engine) error { + type ProtectedTag struct { + ID int64 `xorm:"pk autoincr"` + RepoID int64 + NamePattern string + AllowlistUserIDs []int64 `xorm:"JSON TEXT"` + AllowlistTeamIDs []int64 `xorm:"JSON TEXT"` + + CreatedUnix timeutil.TimeStamp `xorm:"created"` + UpdatedUnix timeutil.TimeStamp `xorm:"updated"` + } + + return x.Sync2(new(ProtectedTag)) +} diff --git a/models/models.go b/models/models.go index 3266be0f4ab8..610933d3270b 100644 --- a/models/models.go +++ b/models/models.go @@ -137,6 +137,7 @@ func init() { new(IssueIndex), new(PushMirror), new(RepoArchiver), + new(ProtectedTag), ) gonicNames := []string{"SSL", "UID"} diff --git a/models/protected_tag.go b/models/protected_tag.go new file mode 100644 index 000000000000..88f20dd29a86 --- /dev/null +++ b/models/protected_tag.go @@ -0,0 +1,131 @@ +// Copyright 2021 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package models + +import ( + "regexp" + "strings" + + "code.gitea.io/gitea/modules/base" + "code.gitea.io/gitea/modules/timeutil" + + "github.com/gobwas/glob" +) + +// ProtectedTag struct +type ProtectedTag struct { + ID int64 `xorm:"pk autoincr"` + RepoID int64 + NamePattern string + RegexPattern *regexp.Regexp `xorm:"-"` + GlobPattern glob.Glob `xorm:"-"` + AllowlistUserIDs []int64 `xorm:"JSON TEXT"` + AllowlistTeamIDs []int64 `xorm:"JSON TEXT"` + + CreatedUnix timeutil.TimeStamp `xorm:"created"` + UpdatedUnix timeutil.TimeStamp `xorm:"updated"` +} + +// InsertProtectedTag inserts a protected tag to database +func InsertProtectedTag(pt *ProtectedTag) error { + _, err := x.Insert(pt) + return err +} + +// UpdateProtectedTag updates the protected tag +func UpdateProtectedTag(pt *ProtectedTag) error { + _, err := x.ID(pt.ID).AllCols().Update(pt) + return err +} + +// DeleteProtectedTag deletes a protected tag by ID +func DeleteProtectedTag(pt *ProtectedTag) error { + _, err := x.ID(pt.ID).Delete(&ProtectedTag{}) + return err +} + +// EnsureCompiledPattern ensures the glob pattern is compiled +func (pt *ProtectedTag) EnsureCompiledPattern() error { + if pt.RegexPattern != nil || pt.GlobPattern != nil { + return nil + } + + var err error + if len(pt.NamePattern) >= 2 && strings.HasPrefix(pt.NamePattern, "/") && strings.HasSuffix(pt.NamePattern, "/") { + pt.RegexPattern, err = regexp.Compile(pt.NamePattern[1 : len(pt.NamePattern)-1]) + } else { + pt.GlobPattern, err = glob.Compile(pt.NamePattern) + } + return err +} + +// IsUserAllowed returns true if the user is allowed to modify the tag +func (pt *ProtectedTag) IsUserAllowed(userID int64) (bool, error) { + if base.Int64sContains(pt.AllowlistUserIDs, userID) { + return true, nil + } + + if len(pt.AllowlistTeamIDs) == 0 { + return false, nil + } + + in, err := IsUserInTeams(userID, pt.AllowlistTeamIDs) + if err != nil { + return false, err + } + return in, nil +} + +// GetProtectedTags gets all protected tags of the repository +func (repo *Repository) GetProtectedTags() ([]*ProtectedTag, error) { + tags := make([]*ProtectedTag, 0) + return tags, x.Find(&tags, &ProtectedTag{RepoID: repo.ID}) +} + +// GetProtectedTagByID gets the protected tag with the specific id +func GetProtectedTagByID(id int64) (*ProtectedTag, error) { + tag := new(ProtectedTag) + has, err := x.ID(id).Get(tag) + if err != nil { + return nil, err + } + if !has { + return nil, nil + } + return tag, nil +} + +// IsUserAllowedToControlTag checks if a user can control the specific tag. +// It returns true if the tag name is not protected or the user is allowed to control it. +func IsUserAllowedToControlTag(tags []*ProtectedTag, tagName string, userID int64) (bool, error) { + isAllowed := true + for _, tag := range tags { + err := tag.EnsureCompiledPattern() + if err != nil { + return false, err + } + + if !tag.matchString(tagName) { + continue + } + + isAllowed, err = tag.IsUserAllowed(userID) + if err != nil { + return false, err + } + if isAllowed { + break + } + } + + return isAllowed, nil +} + +func (pt *ProtectedTag) matchString(name string) bool { + if pt.RegexPattern != nil { + return pt.RegexPattern.MatchString(name) + } + return pt.GlobPattern.Match(name) +} diff --git a/models/protected_tag_test.go b/models/protected_tag_test.go new file mode 100644 index 000000000000..3dc895c69fe1 --- /dev/null +++ b/models/protected_tag_test.go @@ -0,0 +1,162 @@ +// Copyright 2021 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package models + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestIsUserAllowed(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + + pt := &ProtectedTag{} + allowed, err := pt.IsUserAllowed(1) + assert.NoError(t, err) + assert.False(t, allowed) + + pt = &ProtectedTag{ + AllowlistUserIDs: []int64{1}, + } + allowed, err = pt.IsUserAllowed(1) + assert.NoError(t, err) + assert.True(t, allowed) + + allowed, err = pt.IsUserAllowed(2) + assert.NoError(t, err) + assert.False(t, allowed) + + pt = &ProtectedTag{ + AllowlistTeamIDs: []int64{1}, + } + allowed, err = pt.IsUserAllowed(1) + assert.NoError(t, err) + assert.False(t, allowed) + + allowed, err = pt.IsUserAllowed(2) + assert.NoError(t, err) + assert.True(t, allowed) + + pt = &ProtectedTag{ + AllowlistUserIDs: []int64{1}, + AllowlistTeamIDs: []int64{1}, + } + allowed, err = pt.IsUserAllowed(1) + assert.NoError(t, err) + assert.True(t, allowed) + + allowed, err = pt.IsUserAllowed(2) + assert.NoError(t, err) + assert.True(t, allowed) +} + +func TestIsUserAllowedToControlTag(t *testing.T) { + cases := []struct { + name string + userid int64 + allowed bool + }{ + { + name: "test", + userid: 1, + allowed: true, + }, + { + name: "test", + userid: 3, + allowed: true, + }, + { + name: "gitea", + userid: 1, + allowed: true, + }, + { + name: "gitea", + userid: 3, + allowed: false, + }, + { + name: "test-gitea", + userid: 1, + allowed: true, + }, + { + name: "test-gitea", + userid: 3, + allowed: false, + }, + { + name: "gitea-test", + userid: 1, + allowed: true, + }, + { + name: "gitea-test", + userid: 3, + allowed: true, + }, + { + name: "v-1", + userid: 1, + allowed: false, + }, + { + name: "v-1", + userid: 2, + allowed: true, + }, + { + name: "release", + userid: 1, + allowed: false, + }, + } + + t.Run("Glob", func(t *testing.T) { + protectedTags := []*ProtectedTag{ + { + NamePattern: `*gitea`, + AllowlistUserIDs: []int64{1}, + }, + { + NamePattern: `v-*`, + AllowlistUserIDs: []int64{2}, + }, + { + NamePattern: "release", + }, + } + + for n, c := range cases { + isAllowed, err := IsUserAllowedToControlTag(protectedTags, c.name, c.userid) + assert.NoError(t, err) + assert.Equal(t, c.allowed, isAllowed, "case %d: error should match", n) + } + }) + + t.Run("Regex", func(t *testing.T) { + protectedTags := []*ProtectedTag{ + { + NamePattern: `/gitea\z/`, + AllowlistUserIDs: []int64{1}, + }, + { + NamePattern: `/\Av-/`, + AllowlistUserIDs: []int64{2}, + }, + { + NamePattern: "/release/", + }, + } + + for n, c := range cases { + isAllowed, err := IsUserAllowedToControlTag(protectedTags, c.name, c.userid) + assert.NoError(t, err) + assert.Equal(t, c.allowed, isAllowed, "case %d: error should match", n) + } + }) +} diff --git a/models/repo.go b/models/repo.go index 2baf6e9bdd94..4ce3d4839bc3 100644 --- a/models/repo.go +++ b/models/repo.go @@ -1498,6 +1498,7 @@ func DeleteRepository(doer *User, uid, repoID int64) error { &Mirror{RepoID: repoID}, &Notification{RepoID: repoID}, &ProtectedBranch{RepoID: repoID}, + &ProtectedTag{RepoID: repoID}, &PullRequest{BaseRepoID: repoID}, &PushMirror{RepoID: repoID}, &Release{RepoID: repoID}, diff --git a/modules/validation/binding.go b/modules/validation/binding.go index 5cfd994d2dae..4cef48daf32d 100644 --- a/modules/validation/binding.go +++ b/modules/validation/binding.go @@ -19,6 +19,9 @@ const ( // ErrGlobPattern is returned when glob pattern is invalid ErrGlobPattern = "GlobPattern" + + // ErrRegexPattern is returned when a regex pattern is invalid + ErrRegexPattern = "RegexPattern" ) var ( @@ -53,6 +56,8 @@ func AddBindingRules() { addGitRefNameBindingRule() addValidURLBindingRule() addGlobPatternRule() + addRegexPatternRule() + addGlobOrRegexPatternRule() } func addGitRefNameBindingRule() { @@ -102,17 +107,55 @@ func addGlobPatternRule() { IsMatch: func(rule string) bool { return rule == "GlobPattern" }, + IsValid: globPatternValidator, + }) +} + +func globPatternValidator(errs binding.Errors, name string, val interface{}) (bool, binding.Errors) { + str := fmt.Sprintf("%v", val) + + if len(str) != 0 { + if _, err := glob.Compile(str); err != nil { + errs.Add([]string{name}, ErrGlobPattern, err.Error()) + return false, errs + } + } + + return true, errs +} + +func addRegexPatternRule() { + binding.AddRule(&binding.Rule{ + IsMatch: func(rule string) bool { + return rule == "RegexPattern" + }, + IsValid: regexPatternValidator, + }) +} + +func regexPatternValidator(errs binding.Errors, name string, val interface{}) (bool, binding.Errors) { + str := fmt.Sprintf("%v", val) + + if _, err := regexp.Compile(str); err != nil { + errs.Add([]string{name}, ErrRegexPattern, err.Error()) + return false, errs + } + + return true, errs +} + +func addGlobOrRegexPatternRule() { + binding.AddRule(&binding.Rule{ + IsMatch: func(rule string) bool { + return rule == "GlobOrRegexPattern" + }, IsValid: func(errs binding.Errors, name string, val interface{}) (bool, binding.Errors) { - str := fmt.Sprintf("%v", val) + str := strings.TrimSpace(fmt.Sprintf("%v", val)) - if len(str) != 0 { - if _, err := glob.Compile(str); err != nil { - errs.Add([]string{name}, ErrGlobPattern, err.Error()) - return false, errs - } + if len(str) >= 2 && strings.HasPrefix(str, "/") && strings.HasSuffix(str, "/") { + return regexPatternValidator(errs, name, str[1:len(str)-1]) } - - return true, errs + return globPatternValidator(errs, name, val) }, }) } diff --git a/modules/validation/binding_test.go b/modules/validation/binding_test.go index e0daba89e502..d3b4e686ae29 100644 --- a/modules/validation/binding_test.go +++ b/modules/validation/binding_test.go @@ -26,9 +26,10 @@ type ( } TestForm struct { - BranchName string `form:"BranchName" binding:"GitRefName"` - URL string `form:"ValidUrl" binding:"ValidUrl"` - GlobPattern string `form:"GlobPattern" binding:"GlobPattern"` + BranchName string `form:"BranchName" binding:"GitRefName"` + URL string `form:"ValidUrl" binding:"ValidUrl"` + GlobPattern string `form:"GlobPattern" binding:"GlobPattern"` + RegexPattern string `form:"RegexPattern" binding:"RegexPattern"` } ) diff --git a/modules/validation/regex_pattern_test.go b/modules/validation/regex_pattern_test.go new file mode 100644 index 000000000000..afe1bcf425df --- /dev/null +++ b/modules/validation/regex_pattern_test.go @@ -0,0 +1,60 @@ +// Copyright 2021 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package validation + +import ( + "regexp" + "testing" + + "gitea.com/go-chi/binding" +) + +func getRegexPatternErrorString(pattern string) string { + if _, err := regexp.Compile(pattern); err != nil { + return err.Error() + } + return "" +} + +var regexValidationTestCases = []validationTestCase{ + { + description: "Empty regex pattern", + data: TestForm{ + RegexPattern: "", + }, + expectedErrors: binding.Errors{}, + }, + { + description: "Valid regex", + data: TestForm{ + RegexPattern: `(\d{1,3})+`, + }, + expectedErrors: binding.Errors{}, + }, + + { + description: "Invalid regex", + data: TestForm{ + RegexPattern: "[a-", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"RegexPattern"}, + Classification: ErrRegexPattern, + Message: getRegexPatternErrorString("[a-"), + }, + }, + }, +} + +func Test_RegexPatternValidation(t *testing.T) { + AddBindingRules() + + for _, testCase := range regexValidationTestCases { + t.Run(testCase.description, func(t *testing.T) { + performValidationTest(t, testCase) + }) + } +} diff --git a/modules/web/middleware/binding.go b/modules/web/middleware/binding.go index cd418c9792b7..cbdb29b81294 100644 --- a/modules/web/middleware/binding.go +++ b/modules/web/middleware/binding.go @@ -135,6 +135,8 @@ func Validate(errs binding.Errors, data map[string]interface{}, f Form, l transl data["ErrorMsg"] = trName + l.Tr("form.include_error", GetInclude(field)) case validation.ErrGlobPattern: data["ErrorMsg"] = trName + l.Tr("form.glob_pattern_error", errs[0].Message) + case validation.ErrRegexPattern: + data["ErrorMsg"] = trName + l.Tr("form.regex_pattern_error", errs[0].Message) default: data["ErrorMsg"] = l.Tr("form.unknown_error") + " " + errs[0].Classification } diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 59ee6e48ea7f..a809f49eebae 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -83,6 +83,7 @@ add = Add add_all = Add All remove = Remove remove_all = Remove All +edit = Edit write = Write preview = Preview @@ -415,6 +416,7 @@ email_error = ` is not a valid email address.` url_error = ` is not a valid URL.` include_error = ` must contain substring '%s'.` glob_pattern_error = ` glob pattern is invalid: %s.` +regex_pattern_error = ` regex pattern is invalid: %s.` unknown_error = Unknown error: captcha_incorrect = The CAPTCHA code is incorrect. password_not_match = The passwords do not match. @@ -1802,7 +1804,7 @@ settings.event_pull_request_review_desc = Pull request approved, rejected, or re settings.event_pull_request_sync = Pull Request Synchronized settings.event_pull_request_sync_desc = Pull request synchronized. settings.branch_filter = Branch filter -settings.branch_filter_desc = Branch whitelist for push, branch creation and branch deletion events, specified as glob pattern. If empty or *, events for all branches are reported. See github.com/gobwas/glob documentation for syntax. Examples: master, {master,release*}. +settings.branch_filter_desc = Branch whitelist for push, branch creation and branch deletion events, specified as glob pattern. If empty or *, events for all branches are reported. See github.com/gobwas/glob documentation for syntax. Examples: master, {master,release*}. settings.active = Active settings.active_helper = Information about triggered events will be sent to this webhook URL. settings.add_hook_success = The webhook has been added. @@ -1872,7 +1874,7 @@ settings.dismiss_stale_approvals_desc = When new commits that change the content settings.require_signed_commits = Require Signed Commits settings.require_signed_commits_desc = Reject pushes to this branch if they are unsigned or unverifiable. settings.protect_protected_file_patterns = Protected file patterns (separated using semicolon '\;'): -settings.protect_protected_file_patterns_desc = Protected files that are not allowed to be changed directly even if user has rights to add, edit, or delete files in this branch. Multiple patterns can be separated using semicolon ('\;'). See github.com/gobwas/glob documentation for pattern syntax. Examples: .drone.yml, /docs/**/*.txt. +settings.protect_protected_file_patterns_desc = Protected files that are not allowed to be changed directly even if user has rights to add, edit, or delete files in this branch. Multiple patterns can be separated using semicolon ('\;'). See github.com/gobwas/glob documentation for pattern syntax. Examples: .drone.yml, /docs/**/*.txt. settings.add_protected_branch = Enable protection settings.delete_protected_branch = Disable protection settings.update_protect_branch_success = Branch protection for branch '%s' has been updated. @@ -1891,6 +1893,16 @@ settings.choose_branch = Choose a branch… settings.no_protected_branch = There are no protected branches. settings.edit_protected_branch = Edit settings.protected_branch_required_approvals_min = Required approvals cannot be negative. +settings.tags = Tags +settings.tags.protection = Tag Protection +settings.tags.protection.pattern = Tag Pattern +settings.tags.protection.allowed = Allowed +settings.tags.protection.allowed.users = Allowed users +settings.tags.protection.allowed.teams = Allowed teams +settings.tags.protection.allowed.noone = No One +settings.tags.protection.create = Protect Tag +settings.tags.protection.none = There are no protected tags. +settings.tags.protection.pattern.description = You can use a single name or a glob pattern or regular expression to match multiple tags. Read more in the protected tags guide. settings.bot_token = Bot Token settings.chat_id = Chat ID settings.matrix.homeserver_url = Homeserver URL @@ -1904,6 +1916,7 @@ settings.archive.success = The repo was successfully archived. settings.archive.error = An error occurred while trying to archive the repo. See the log for more details. settings.archive.error_ismirror = You cannot archive a mirrored repo. settings.archive.branchsettings_unavailable = Branch settings are not available if the repo is archived. +settings.archive.tagsettings_unavailable = Tag settings are not available if the repo is archived. settings.unarchive.button = Un-Archive Repo settings.unarchive.header = Un-Archive This Repo settings.unarchive.text = Un-Archiving the repo will restore its ability to receive commits and pushes, as well as new issues and pull-requests. @@ -2018,6 +2031,7 @@ release.deletion_tag_desc = Will delete this tag from repository. Repository con release.deletion_tag_success = The tag has been deleted. release.tag_name_already_exist = A release with this tag name already exists. release.tag_name_invalid = The tag name is not valid. +release.tag_name_protected = The tag name is protected. release.tag_already_exist = This tag name already exists. release.downloads = Downloads release.download_count = Downloads: %s diff --git a/routers/private/hook.go b/routers/private/hook.go index 17ea4f2437b0..9f5579b6ae68 100644 --- a/routers/private/hook.go +++ b/routers/private/hook.go @@ -155,125 +155,202 @@ func HookPreReceive(ctx *gitea_context.PrivateContext) { private.GitQuarantinePath+"="+opts.GitQuarantinePath) } + protectedTags, err := repo.GetProtectedTags() + if err != nil { + log.Error("Unable to get protected tags for %-v Error: %v", repo, err) + ctx.JSON(http.StatusInternalServerError, private.Response{ + Err: err.Error(), + }) + return + } + // Iterate across the provided old commit IDs for i := range opts.OldCommitIDs { oldCommitID := opts.OldCommitIDs[i] newCommitID := opts.NewCommitIDs[i] refFullName := opts.RefFullNames[i] - branchName := strings.TrimPrefix(refFullName, git.BranchPrefix) - if branchName == repo.DefaultBranch && newCommitID == git.EmptySHA { - log.Warn("Forbidden: Branch: %s is the default branch in %-v and cannot be deleted", branchName, repo) - ctx.JSON(http.StatusForbidden, private.Response{ - Err: fmt.Sprintf("branch %s is the default branch and cannot be deleted", branchName), - }) - return - } - - protectBranch, err := models.GetProtectedBranchBy(repo.ID, branchName) - if err != nil { - log.Error("Unable to get protected branch: %s in %-v Error: %v", branchName, repo, err) - ctx.JSON(http.StatusInternalServerError, private.Response{ - Err: err.Error(), - }) - return - } - - // Allow pushes to non-protected branches - if protectBranch == nil || !protectBranch.IsProtected() { - continue - } - - // This ref is a protected branch. - // - // First of all we need to enforce absolutely: - // - // 1. Detect and prevent deletion of the branch - if newCommitID == git.EmptySHA { - log.Warn("Forbidden: Branch: %s in %-v is protected from deletion", branchName, repo) - ctx.JSON(http.StatusForbidden, private.Response{ - Err: fmt.Sprintf("branch %s is protected from deletion", branchName), - }) - return - } + if strings.HasPrefix(refFullName, git.BranchPrefix) { + branchName := strings.TrimPrefix(refFullName, git.BranchPrefix) + if branchName == repo.DefaultBranch && newCommitID == git.EmptySHA { + log.Warn("Forbidden: Branch: %s is the default branch in %-v and cannot be deleted", branchName, repo) + ctx.JSON(http.StatusForbidden, private.Response{ + Err: fmt.Sprintf("branch %s is the default branch and cannot be deleted", branchName), + }) + return + } - // 2. Disallow force pushes to protected branches - if git.EmptySHA != oldCommitID { - output, err := git.NewCommand("rev-list", "--max-count=1", oldCommitID, "^"+newCommitID).RunInDirWithEnv(repo.RepoPath(), env) + protectBranch, err := models.GetProtectedBranchBy(repo.ID, branchName) if err != nil { - log.Error("Unable to detect force push between: %s and %s in %-v Error: %v", oldCommitID, newCommitID, repo, err) + log.Error("Unable to get protected branch: %s in %-v Error: %v", branchName, repo, err) ctx.JSON(http.StatusInternalServerError, private.Response{ - Err: fmt.Sprintf("Fail to detect force push: %v", err), + Err: err.Error(), }) return - } else if len(output) > 0 { - log.Warn("Forbidden: Branch: %s in %-v is protected from force push", branchName, repo) + } + + // Allow pushes to non-protected branches + if protectBranch == nil || !protectBranch.IsProtected() { + continue + } + + // This ref is a protected branch. + // + // First of all we need to enforce absolutely: + // + // 1. Detect and prevent deletion of the branch + if newCommitID == git.EmptySHA { + log.Warn("Forbidden: Branch: %s in %-v is protected from deletion", branchName, repo) ctx.JSON(http.StatusForbidden, private.Response{ - Err: fmt.Sprintf("branch %s is protected from force push", branchName), + Err: fmt.Sprintf("branch %s is protected from deletion", branchName), }) return - } - } - // 3. Enforce require signed commits - if protectBranch.RequireSignedCommits { - err := verifyCommits(oldCommitID, newCommitID, gitRepo, env) - if err != nil { - if !isErrUnverifiedCommit(err) { - log.Error("Unable to check commits from %s to %s in %-v: %v", oldCommitID, newCommitID, repo, err) + // 2. Disallow force pushes to protected branches + if git.EmptySHA != oldCommitID { + output, err := git.NewCommand("rev-list", "--max-count=1", oldCommitID, "^"+newCommitID).RunInDirWithEnv(repo.RepoPath(), env) + if err != nil { + log.Error("Unable to detect force push between: %s and %s in %-v Error: %v", oldCommitID, newCommitID, repo, err) ctx.JSON(http.StatusInternalServerError, private.Response{ - Err: fmt.Sprintf("Unable to check commits from %s to %s: %v", oldCommitID, newCommitID, err), + Err: fmt.Sprintf("Fail to detect force push: %v", err), + }) + return + } else if len(output) > 0 { + log.Warn("Forbidden: Branch: %s in %-v is protected from force push", branchName, repo) + ctx.JSON(http.StatusForbidden, private.Response{ + Err: fmt.Sprintf("branch %s is protected from force push", branchName), }) return + } - unverifiedCommit := err.(*errUnverifiedCommit).sha - log.Warn("Forbidden: Branch: %s in %-v is protected from unverified commit %s", branchName, repo, unverifiedCommit) - ctx.JSON(http.StatusForbidden, private.Response{ - Err: fmt.Sprintf("branch %s is protected from unverified commit %s", branchName, unverifiedCommit), - }) - return } - } - // Now there are several tests which can be overridden: - // - // 4. Check protected file patterns - this is overridable from the UI - changedProtectedfiles := false - protectedFilePath := "" + // 3. Enforce require signed commits + if protectBranch.RequireSignedCommits { + err := verifyCommits(oldCommitID, newCommitID, gitRepo, env) + if err != nil { + if !isErrUnverifiedCommit(err) { + log.Error("Unable to check commits from %s to %s in %-v: %v", oldCommitID, newCommitID, repo, err) + ctx.JSON(http.StatusInternalServerError, private.Response{ + Err: fmt.Sprintf("Unable to check commits from %s to %s: %v", oldCommitID, newCommitID, err), + }) + return + } + unverifiedCommit := err.(*errUnverifiedCommit).sha + log.Warn("Forbidden: Branch: %s in %-v is protected from unverified commit %s", branchName, repo, unverifiedCommit) + ctx.JSON(http.StatusForbidden, private.Response{ + Err: fmt.Sprintf("branch %s is protected from unverified commit %s", branchName, unverifiedCommit), + }) + return + } + } - globs := protectBranch.GetProtectedFilePatterns() - if len(globs) > 0 { - _, err := pull_service.CheckFileProtection(oldCommitID, newCommitID, globs, 1, env, gitRepo) - if err != nil { - if !models.IsErrFilePathProtected(err) { - log.Error("Unable to check file protection for commits from %s to %s in %-v: %v", oldCommitID, newCommitID, repo, err) + // Now there are several tests which can be overridden: + // + // 4. Check protected file patterns - this is overridable from the UI + changedProtectedfiles := false + protectedFilePath := "" + + globs := protectBranch.GetProtectedFilePatterns() + if len(globs) > 0 { + _, err := pull_service.CheckFileProtection(oldCommitID, newCommitID, globs, 1, env, gitRepo) + if err != nil { + if !models.IsErrFilePathProtected(err) { + log.Error("Unable to check file protection for commits from %s to %s in %-v: %v", oldCommitID, newCommitID, repo, err) + ctx.JSON(http.StatusInternalServerError, private.Response{ + Err: fmt.Sprintf("Unable to check file protection for commits from %s to %s: %v", oldCommitID, newCommitID, err), + }) + return + } + + changedProtectedfiles = true + protectedFilePath = err.(models.ErrFilePathProtected).Path + } + } + + // 5. Check if the doer is allowed to push + canPush := false + if opts.IsDeployKey { + canPush = !changedProtectedfiles && protectBranch.CanPush && (!protectBranch.EnableWhitelist || protectBranch.WhitelistDeployKeys) + } else { + canPush = !changedProtectedfiles && protectBranch.CanUserPush(opts.UserID) + } + + // 6. If we're not allowed to push directly + if !canPush { + // Is this is a merge from the UI/API? + if opts.PullRequestID == 0 { + // 6a. If we're not merging from the UI/API then there are two ways we got here: + // + // We are changing a protected file and we're not allowed to do that + if changedProtectedfiles { + log.Warn("Forbidden: Branch: %s in %-v is protected from changing file %s", branchName, repo, protectedFilePath) + ctx.JSON(http.StatusForbidden, private.Response{ + Err: fmt.Sprintf("branch %s is protected from changing file %s", branchName, protectedFilePath), + }) + return + } + + // Or we're simply not able to push to this protected branch + log.Warn("Forbidden: User %d is not allowed to push to protected branch: %s in %-v", opts.UserID, branchName, repo) + ctx.JSON(http.StatusForbidden, private.Response{ + Err: fmt.Sprintf("Not allowed to push to protected branch %s", branchName), + }) + return + } + // 6b. Merge (from UI or API) + + // Get the PR, user and permissions for the user in the repository + pr, err := models.GetPullRequestByID(opts.PullRequestID) + if err != nil { + log.Error("Unable to get PullRequest %d Error: %v", opts.PullRequestID, err) + ctx.JSON(http.StatusInternalServerError, private.Response{ + Err: fmt.Sprintf("Unable to get PullRequest %d Error: %v", opts.PullRequestID, err), + }) + return + } + user, err := models.GetUserByID(opts.UserID) + if err != nil { + log.Error("Unable to get User id %d Error: %v", opts.UserID, err) ctx.JSON(http.StatusInternalServerError, private.Response{ - Err: fmt.Sprintf("Unable to check file protection for commits from %s to %s: %v", oldCommitID, newCommitID, err), + Err: fmt.Sprintf("Unable to get User id %d Error: %v", opts.UserID, err), + }) + return + } + perm, err := models.GetUserRepoPermission(repo, user) + if err != nil { + log.Error("Unable to get Repo permission of repo %s/%s of User %s", repo.OwnerName, repo.Name, user.Name, err) + ctx.JSON(http.StatusInternalServerError, private.Response{ + Err: fmt.Sprintf("Unable to get Repo permission of repo %s/%s of User %s: %v", repo.OwnerName, repo.Name, user.Name, err), }) return } - changedProtectedfiles = true - protectedFilePath = err.(models.ErrFilePathProtected).Path - } - } + // Now check if the user is allowed to merge PRs for this repository + allowedMerge, err := pull_service.IsUserAllowedToMerge(pr, perm, user) + if err != nil { + log.Error("Error calculating if allowed to merge: %v", err) + ctx.JSON(http.StatusInternalServerError, private.Response{ + Err: fmt.Sprintf("Error calculating if allowed to merge: %v", err), + }) + return + } - // 5. Check if the doer is allowed to push - canPush := false - if opts.IsDeployKey { - canPush = !changedProtectedfiles && protectBranch.CanPush && (!protectBranch.EnableWhitelist || protectBranch.WhitelistDeployKeys) - } else { - canPush = !changedProtectedfiles && protectBranch.CanUserPush(opts.UserID) - } + if !allowedMerge { + log.Warn("Forbidden: User %d is not allowed to push to protected branch: %s in %-v and is not allowed to merge pr #%d", opts.UserID, branchName, repo, pr.Index) + ctx.JSON(http.StatusForbidden, private.Response{ + Err: fmt.Sprintf("Not allowed to push to protected branch %s", branchName), + }) + return + } - // 6. If we're not allowed to push directly - if !canPush { - // Is this is a merge from the UI/API? - if opts.PullRequestID == 0 { - // 6a. If we're not merging from the UI/API then there are two ways we got here: - // - // We are changing a protected file and we're not allowed to do that + // If we're an admin for the repository we can ignore status checks, reviews and override protected files + if perm.IsAdmin() { + continue + } + + // Now if we're not an admin - we can't overwrite protected files so fail now if changedProtectedfiles { log.Warn("Forbidden: Branch: %s in %-v is protected from changing file %s", branchName, repo, protectedFilePath) ctx.JSON(http.StatusForbidden, private.Response{ @@ -282,88 +359,44 @@ func HookPreReceive(ctx *gitea_context.PrivateContext) { return } - // Or we're simply not able to push to this protected branch - log.Warn("Forbidden: User %d is not allowed to push to protected branch: %s in %-v", opts.UserID, branchName, repo) - ctx.JSON(http.StatusForbidden, private.Response{ - Err: fmt.Sprintf("Not allowed to push to protected branch %s", branchName), - }) - return - } - // 6b. Merge (from UI or API) - - // Get the PR, user and permissions for the user in the repository - pr, err := models.GetPullRequestByID(opts.PullRequestID) - if err != nil { - log.Error("Unable to get PullRequest %d Error: %v", opts.PullRequestID, err) - ctx.JSON(http.StatusInternalServerError, private.Response{ - Err: fmt.Sprintf("Unable to get PullRequest %d Error: %v", opts.PullRequestID, err), - }) - return - } - user, err := models.GetUserByID(opts.UserID) - if err != nil { - log.Error("Unable to get User id %d Error: %v", opts.UserID, err) - ctx.JSON(http.StatusInternalServerError, private.Response{ - Err: fmt.Sprintf("Unable to get User id %d Error: %v", opts.UserID, err), - }) - return - } - perm, err := models.GetUserRepoPermission(repo, user) - if err != nil { - log.Error("Unable to get Repo permission of repo %s/%s of User %s", repo.OwnerName, repo.Name, user.Name, err) - ctx.JSON(http.StatusInternalServerError, private.Response{ - Err: fmt.Sprintf("Unable to get Repo permission of repo %s/%s of User %s: %v", repo.OwnerName, repo.Name, user.Name, err), - }) - return + // Check all status checks and reviews are ok + if err := pull_service.CheckPRReadyToMerge(pr, true); err != nil { + if models.IsErrNotAllowedToMerge(err) { + log.Warn("Forbidden: User %d is not allowed push to protected branch %s in %-v and pr #%d is not ready to be merged: %s", opts.UserID, branchName, repo, pr.Index, err.Error()) + ctx.JSON(http.StatusForbidden, private.Response{ + Err: fmt.Sprintf("Not allowed to push to protected branch %s and pr #%d is not ready to be merged: %s", branchName, opts.PullRequestID, err.Error()), + }) + return + } + log.Error("Unable to check if mergable: protected branch %s in %-v and pr #%d. Error: %v", opts.UserID, branchName, repo, pr.Index, err) + ctx.JSON(http.StatusInternalServerError, private.Response{ + Err: fmt.Sprintf("Unable to get status of pull request %d. Error: %v", opts.PullRequestID, err), + }) + return + } } + } else if strings.HasPrefix(refFullName, git.TagPrefix) { + tagName := strings.TrimPrefix(refFullName, git.TagPrefix) - // Now check if the user is allowed to merge PRs for this repository - allowedMerge, err := pull_service.IsUserAllowedToMerge(pr, perm, user) + isAllowed, err := models.IsUserAllowedToControlTag(protectedTags, tagName, opts.UserID) if err != nil { - log.Error("Error calculating if allowed to merge: %v", err) ctx.JSON(http.StatusInternalServerError, private.Response{ - Err: fmt.Sprintf("Error calculating if allowed to merge: %v", err), - }) - return - } - - if !allowedMerge { - log.Warn("Forbidden: User %d is not allowed to push to protected branch: %s in %-v and is not allowed to merge pr #%d", opts.UserID, branchName, repo, pr.Index) - ctx.JSON(http.StatusForbidden, private.Response{ - Err: fmt.Sprintf("Not allowed to push to protected branch %s", branchName), + Err: err.Error(), }) return } - - // If we're an admin for the repository we can ignore status checks, reviews and override protected files - if perm.IsAdmin() { - continue - } - - // Now if we're not an admin - we can't overwrite protected files so fail now - if changedProtectedfiles { - log.Warn("Forbidden: Branch: %s in %-v is protected from changing file %s", branchName, repo, protectedFilePath) + if !isAllowed { + log.Warn("Forbidden: Tag %s in %-v is protected", tagName, repo) ctx.JSON(http.StatusForbidden, private.Response{ - Err: fmt.Sprintf("branch %s is protected from changing file %s", branchName, protectedFilePath), - }) - return - } - - // Check all status checks and reviews are ok - if err := pull_service.CheckPRReadyToMerge(pr, true); err != nil { - if models.IsErrNotAllowedToMerge(err) { - log.Warn("Forbidden: User %d is not allowed push to protected branch %s in %-v and pr #%d is not ready to be merged: %s", opts.UserID, branchName, repo, pr.Index, err.Error()) - ctx.JSON(http.StatusForbidden, private.Response{ - Err: fmt.Sprintf("Not allowed to push to protected branch %s and pr #%d is not ready to be merged: %s", branchName, opts.PullRequestID, err.Error()), - }) - return - } - log.Error("Unable to check if mergable: protected branch %s in %-v and pr #%d. Error: %v", opts.UserID, branchName, repo, pr.Index, err) - ctx.JSON(http.StatusInternalServerError, private.Response{ - Err: fmt.Sprintf("Unable to get status of pull request %d. Error: %v", opts.PullRequestID, err), + Err: fmt.Sprintf("Tag %s is protected", tagName), }) return } + } else { + log.Error("Unexpected ref: %s", refFullName) + ctx.JSON(http.StatusInternalServerError, private.Response{ + Err: fmt.Sprintf("Unexpected ref: %s", refFullName), + }) } } diff --git a/routers/web/repo/release.go b/routers/web/repo/release.go index 3b700e80160c..0665496d44c3 100644 --- a/routers/web/repo/release.go +++ b/routers/web/repo/release.go @@ -322,6 +322,18 @@ func NewReleasePost(ctx *context.Context) { return } + if models.IsErrInvalidTagName(err) { + ctx.Flash.Error(ctx.Tr("repo.release.tag_name_invalid")) + ctx.Redirect(ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchNameSubURL()) + return + } + + if models.IsErrProtectedTagName(err) { + ctx.Flash.Error(ctx.Tr("repo.release.tag_name_protected")) + ctx.Redirect(ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchNameSubURL()) + return + } + ctx.ServerError("releaseservice.CreateNewTag", err) return } @@ -333,7 +345,9 @@ func NewReleasePost(ctx *context.Context) { rel = &models.Release{ RepoID: ctx.Repo.Repository.ID, + Repo: ctx.Repo.Repository, PublisherID: ctx.User.ID, + Publisher: ctx.User, Title: form.Title, TagName: form.TagName, Target: form.Target, @@ -350,6 +364,8 @@ func NewReleasePost(ctx *context.Context) { ctx.RenderWithErr(ctx.Tr("repo.release.tag_name_already_exist"), tplReleaseNew, &form) case models.IsErrInvalidTagName(err): ctx.RenderWithErr(ctx.Tr("repo.release.tag_name_invalid"), tplReleaseNew, &form) + case models.IsErrProtectedTagName(err): + ctx.RenderWithErr(ctx.Tr("repo.release.tag_name_protected"), tplReleaseNew, &form) default: ctx.ServerError("CreateRelease", err) } diff --git a/routers/web/repo/setting.go b/routers/web/repo/setting.go index c48b19b63c1b..5e8c2c527625 100644 --- a/routers/web/repo/setting.go +++ b/routers/web/repo/setting.go @@ -40,6 +40,7 @@ const ( tplSettingsOptions base.TplName = "repo/settings/options" tplCollaboration base.TplName = "repo/settings/collaboration" tplBranches base.TplName = "repo/settings/branches" + tplTags base.TplName = "repo/settings/tags" tplGithooks base.TplName = "repo/settings/githooks" tplGithookEdit base.TplName = "repo/settings/githook_edit" tplDeployKeys base.TplName = "repo/settings/deploy_keys" diff --git a/routers/web/repo/tag.go b/routers/web/repo/tag.go new file mode 100644 index 000000000000..7928591371b7 --- /dev/null +++ b/routers/web/repo/tag.go @@ -0,0 +1,182 @@ +// Copyright 2021 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package repo + +import ( + "fmt" + "net/http" + "strings" + + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/base" + "code.gitea.io/gitea/modules/context" + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/web" + "code.gitea.io/gitea/services/forms" +) + +// Tags render the page to protect tags +func Tags(ctx *context.Context) { + if setTagsContext(ctx) != nil { + return + } + + ctx.HTML(http.StatusOK, tplTags) +} + +// NewProtectedTagPost handles creation of a protect tag +func NewProtectedTagPost(ctx *context.Context) { + if setTagsContext(ctx) != nil { + return + } + + if ctx.HasError() { + ctx.HTML(http.StatusOK, tplTags) + return + } + + repo := ctx.Repo.Repository + form := web.GetForm(ctx).(*forms.ProtectTagForm) + + pt := &models.ProtectedTag{ + RepoID: repo.ID, + NamePattern: strings.TrimSpace(form.NamePattern), + } + + if strings.TrimSpace(form.AllowlistUsers) != "" { + pt.AllowlistUserIDs, _ = base.StringsToInt64s(strings.Split(form.AllowlistUsers, ",")) + } + if strings.TrimSpace(form.AllowlistTeams) != "" { + pt.AllowlistTeamIDs, _ = base.StringsToInt64s(strings.Split(form.AllowlistTeams, ",")) + } + + if err := models.InsertProtectedTag(pt); err != nil { + ctx.ServerError("InsertProtectedTag", err) + return + } + + ctx.Flash.Success(ctx.Tr("repo.settings.update_settings_success")) + ctx.Redirect(setting.AppSubURL + ctx.Req.URL.Path) +} + +// EditProtectedTag render the page to edit a protect tag +func EditProtectedTag(ctx *context.Context) { + if setTagsContext(ctx) != nil { + return + } + + ctx.Data["PageIsEditProtectedTag"] = true + + pt := selectProtectedTagByContext(ctx) + if pt == nil { + return + } + + ctx.Data["name_pattern"] = pt.NamePattern + ctx.Data["allowlist_users"] = strings.Join(base.Int64sToStrings(pt.AllowlistUserIDs), ",") + ctx.Data["allowlist_teams"] = strings.Join(base.Int64sToStrings(pt.AllowlistTeamIDs), ",") + + ctx.HTML(http.StatusOK, tplTags) +} + +// EditProtectedTagPost handles creation of a protect tag +func EditProtectedTagPost(ctx *context.Context) { + if setTagsContext(ctx) != nil { + return + } + + ctx.Data["PageIsEditProtectedTag"] = true + + if ctx.HasError() { + ctx.HTML(http.StatusOK, tplTags) + return + } + + pt := selectProtectedTagByContext(ctx) + if pt == nil { + return + } + + form := web.GetForm(ctx).(*forms.ProtectTagForm) + + pt.NamePattern = strings.TrimSpace(form.NamePattern) + pt.AllowlistUserIDs, _ = base.StringsToInt64s(strings.Split(form.AllowlistUsers, ",")) + pt.AllowlistTeamIDs, _ = base.StringsToInt64s(strings.Split(form.AllowlistTeams, ",")) + + if err := models.UpdateProtectedTag(pt); err != nil { + ctx.ServerError("UpdateProtectedTag", err) + return + } + + ctx.Flash.Success(ctx.Tr("repo.settings.update_settings_success")) + ctx.Redirect(ctx.Repo.Repository.Link() + "/settings/tags") +} + +// DeleteProtectedTagPost handles deletion of a protected tag +func DeleteProtectedTagPost(ctx *context.Context) { + pt := selectProtectedTagByContext(ctx) + if pt == nil { + return + } + + if err := models.DeleteProtectedTag(pt); err != nil { + ctx.ServerError("DeleteProtectedTag", err) + return + } + + ctx.Flash.Success(ctx.Tr("repo.settings.update_settings_success")) + ctx.Redirect(ctx.Repo.Repository.Link() + "/settings/tags") +} + +func setTagsContext(ctx *context.Context) error { + ctx.Data["Title"] = ctx.Tr("repo.settings") + ctx.Data["PageIsSettingsTags"] = true + + protectedTags, err := ctx.Repo.Repository.GetProtectedTags() + if err != nil { + ctx.ServerError("GetProtectedTags", err) + return err + } + ctx.Data["ProtectedTags"] = protectedTags + + users, err := ctx.Repo.Repository.GetReaders() + if err != nil { + ctx.ServerError("Repo.Repository.GetReaders", err) + return err + } + ctx.Data["Users"] = users + + if ctx.Repo.Owner.IsOrganization() { + teams, err := ctx.Repo.Owner.TeamsWithAccessToRepo(ctx.Repo.Repository.ID, models.AccessModeRead) + if err != nil { + ctx.ServerError("Repo.Owner.TeamsWithAccessToRepo", err) + return err + } + ctx.Data["Teams"] = teams + } + + return nil +} + +func selectProtectedTagByContext(ctx *context.Context) *models.ProtectedTag { + id := ctx.QueryInt64("id") + if id == 0 { + id = ctx.ParamsInt64(":id") + } + + tag, err := models.GetProtectedTagByID(id) + if err != nil { + ctx.ServerError("GetProtectedTagByID", err) + return nil + } + + if tag != nil && tag.RepoID == ctx.Repo.Repository.ID { + return tag + } + + ctx.NotFound("", fmt.Errorf("ProtectedTag[%v] not associated to repository %v", id, ctx.Repo.Repository)) + + return nil +} diff --git a/routers/web/web.go b/routers/web/web.go index 883213479c86..627c88aab1c5 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -594,12 +594,21 @@ func RegisterRoutes(m *web.Route) { m.Post("/delete", repo.DeleteTeam) }) }) + m.Group("/branches", func() { m.Combo("").Get(repo.ProtectedBranch).Post(repo.ProtectedBranchPost) m.Combo("/*").Get(repo.SettingsProtectedBranch). Post(bindIgnErr(forms.ProtectBranchForm{}), context.RepoMustNotBeArchived(), repo.SettingsProtectedBranchPost) }, repo.MustBeNotEmpty) + m.Group("/tags", func() { + m.Get("", repo.Tags) + m.Post("", bindIgnErr(forms.ProtectTagForm{}), context.RepoMustNotBeArchived(), repo.NewProtectedTagPost) + m.Post("/delete", context.RepoMustNotBeArchived(), repo.DeleteProtectedTagPost) + m.Get("/{id}", repo.EditProtectedTag) + m.Post("/{id}", bindIgnErr(forms.ProtectTagForm{}), context.RepoMustNotBeArchived(), repo.EditProtectedTagPost) + }) + m.Group("/hooks/git", func() { m.Get("", repo.GitHooks) m.Combo("/{name}").Get(repo.GitHooksEdit). diff --git a/services/forms/repo_tag_form.go b/services/forms/repo_tag_form.go new file mode 100644 index 000000000000..337e7fe1ea64 --- /dev/null +++ b/services/forms/repo_tag_form.go @@ -0,0 +1,27 @@ +// Copyright 2021 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package forms + +import ( + "net/http" + + "code.gitea.io/gitea/modules/context" + "code.gitea.io/gitea/modules/web/middleware" + + "gitea.com/go-chi/binding" +) + +// ProtectTagForm form for changing protected tag settings +type ProtectTagForm struct { + NamePattern string `binding:"Required;GlobOrRegexPattern"` + AllowlistUsers string + AllowlistTeams string +} + +// Validate validates the fields +func (f *ProtectTagForm) Validate(req *http.Request, errs binding.Errors) binding.Errors { + ctx := context.GetContext(req) + return middleware.Validate(errs, ctx.Data, f, ctx.Locale) +} diff --git a/services/release/release.go b/services/release/release.go index 9d201edf6d28..6f5aa02c85d8 100644 --- a/services/release/release.go +++ b/services/release/release.go @@ -23,6 +23,25 @@ func createTag(gitRepo *git.Repository, rel *models.Release, msg string) (bool, // Only actual create when publish. if !rel.IsDraft { if !gitRepo.IsTagExist(rel.TagName) { + if err := rel.LoadAttributes(); err != nil { + log.Error("LoadAttributes: %v", err) + return false, err + } + + protectedTags, err := rel.Repo.GetProtectedTags() + if err != nil { + return false, fmt.Errorf("GetProtectedTags: %v", err) + } + isAllowed, err := models.IsUserAllowedToControlTag(protectedTags, rel.TagName, rel.PublisherID) + if err != nil { + return false, err + } + if !isAllowed { + return false, models.ErrProtectedTagName{ + TagName: rel.TagName, + } + } + commit, err := gitRepo.GetCommit(rel.Target) if err != nil { return false, fmt.Errorf("GetCommit: %v", err) @@ -49,11 +68,7 @@ func createTag(gitRepo *git.Repository, rel *models.Release, msg string) (bool, } created = true rel.LowerTagName = strings.ToLower(rel.TagName) - // Prepare Notify - if err := rel.LoadAttributes(); err != nil { - log.Error("LoadAttributes: %v", err) - return false, err - } + notification.NotifyPushCommits( rel.Publisher, rel.Repo, &repository.PushUpdateOptions{ @@ -137,7 +152,9 @@ func CreateNewTag(doer *models.User, repo *models.Repository, commit, tagName, m rel := &models.Release{ RepoID: repo.ID, + Repo: repo, PublisherID: doer.ID, + Publisher: doer, TagName: tagName, Target: commit, IsDraft: false, diff --git a/services/release/release_test.go b/services/release/release_test.go index 085be55cb4b1..9f665fabab6f 100644 --- a/services/release/release_test.go +++ b/services/release/release_test.go @@ -33,7 +33,9 @@ func TestRelease_Create(t *testing.T) { assert.NoError(t, CreateRelease(gitRepo, &models.Release{ RepoID: repo.ID, + Repo: repo, PublisherID: user.ID, + Publisher: user, TagName: "v0.1", Target: "master", Title: "v0.1 is released", @@ -45,7 +47,9 @@ func TestRelease_Create(t *testing.T) { assert.NoError(t, CreateRelease(gitRepo, &models.Release{ RepoID: repo.ID, + Repo: repo, PublisherID: user.ID, + Publisher: user, TagName: "v0.1.1", Target: "65f1bf27bc3bf70f64657658635e66094edbcb4d", Title: "v0.1.1 is released", @@ -57,7 +61,9 @@ func TestRelease_Create(t *testing.T) { assert.NoError(t, CreateRelease(gitRepo, &models.Release{ RepoID: repo.ID, + Repo: repo, PublisherID: user.ID, + Publisher: user, TagName: "v0.1.2", Target: "65f1bf2", Title: "v0.1.2 is released", @@ -69,7 +75,9 @@ func TestRelease_Create(t *testing.T) { assert.NoError(t, CreateRelease(gitRepo, &models.Release{ RepoID: repo.ID, + Repo: repo, PublisherID: user.ID, + Publisher: user, TagName: "v0.1.3", Target: "65f1bf2", Title: "v0.1.3 is released", @@ -81,7 +89,9 @@ func TestRelease_Create(t *testing.T) { assert.NoError(t, CreateRelease(gitRepo, &models.Release{ RepoID: repo.ID, + Repo: repo, PublisherID: user.ID, + Publisher: user, TagName: "v0.1.4", Target: "65f1bf2", Title: "v0.1.4 is released", @@ -99,7 +109,9 @@ func TestRelease_Create(t *testing.T) { var release = models.Release{ RepoID: repo.ID, + Repo: repo, PublisherID: user.ID, + Publisher: user, TagName: "v0.1.5", Target: "65f1bf2", Title: "v0.1.5 is released", @@ -125,7 +137,9 @@ func TestRelease_Update(t *testing.T) { // Test a changed release assert.NoError(t, CreateRelease(gitRepo, &models.Release{ RepoID: repo.ID, + Repo: repo, PublisherID: user.ID, + Publisher: user, TagName: "v1.1.1", Target: "master", Title: "v1.1.1 is released", @@ -147,7 +161,9 @@ func TestRelease_Update(t *testing.T) { // Test a changed draft assert.NoError(t, CreateRelease(gitRepo, &models.Release{ RepoID: repo.ID, + Repo: repo, PublisherID: user.ID, + Publisher: user, TagName: "v1.2.1", Target: "65f1bf2", Title: "v1.2.1 is draft", @@ -169,7 +185,9 @@ func TestRelease_Update(t *testing.T) { // Test a changed pre-release assert.NoError(t, CreateRelease(gitRepo, &models.Release{ RepoID: repo.ID, + Repo: repo, PublisherID: user.ID, + Publisher: user, TagName: "v1.3.1", Target: "65f1bf2", Title: "v1.3.1 is pre-released", @@ -192,7 +210,9 @@ func TestRelease_Update(t *testing.T) { // Test create release release = &models.Release{ RepoID: repo.ID, + Repo: repo, PublisherID: user.ID, + Publisher: user, TagName: "v1.1.2", Target: "master", Title: "v1.1.2 is released", @@ -258,7 +278,9 @@ func TestRelease_createTag(t *testing.T) { // Test a changed release release := &models.Release{ RepoID: repo.ID, + Repo: repo, PublisherID: user.ID, + Publisher: user, TagName: "v2.1.1", Target: "master", Title: "v2.1.1 is released", @@ -280,7 +302,9 @@ func TestRelease_createTag(t *testing.T) { // Test a changed draft release = &models.Release{ RepoID: repo.ID, + Repo: repo, PublisherID: user.ID, + Publisher: user, TagName: "v2.2.1", Target: "65f1bf2", Title: "v2.2.1 is draft", @@ -301,7 +325,9 @@ func TestRelease_createTag(t *testing.T) { // Test a changed pre-release release = &models.Release{ RepoID: repo.ID, + Repo: repo, PublisherID: user.ID, + Publisher: user, TagName: "v2.3.1", Target: "65f1bf2", Title: "v2.3.1 is pre-released", diff --git a/templates/repo/settings/nav.tmpl b/templates/repo/settings/nav.tmpl index 4b89ece34918..31672cb5ead0 100644 --- a/templates/repo/settings/nav.tmpl +++ b/templates/repo/settings/nav.tmpl @@ -5,6 +5,7 @@
  • {{.i18n.Tr "repo.settings.options"}}
  • {{.i18n.Tr "repo.settings.collaboration"}}
  • {{.i18n.Tr "repo.settings.branches"}}
    • +
  • {{.i18n.Tr "repo.settings.tags"}}
    • {{if not DisableWebhooks}}
  • {{.i18n.Tr "repo.settings.hooks"}}
    • {{end}} diff --git a/templates/repo/settings/navbar.tmpl b/templates/repo/settings/navbar.tmpl index 501c3c4630a4..d8cdf218719b 100644 --- a/templates/repo/settings/navbar.tmpl +++ b/templates/repo/settings/navbar.tmpl @@ -11,6 +11,9 @@ {{.i18n.Tr "repo.settings.branches"}} {{end}} + + {{.i18n.Tr "repo.settings.tags"}} + {{if not DisableWebhooks}} {{.i18n.Tr "repo.settings.hooks"}} diff --git a/templates/repo/settings/tags.tmpl b/templates/repo/settings/tags.tmpl new file mode 100644 index 000000000000..a2c887b1f8f3 --- /dev/null +++ b/templates/repo/settings/tags.tmpl @@ -0,0 +1,132 @@ +{{template "base/head" .}} +
    + {{template "repo/header" .}} + {{template "repo/settings/navbar" .}} +
    + {{template "base/alert" .}} + {{if .Repository.IsArchived}} +
    + {{.i18n.Tr "repo.settings.archive.tagsettings_unavailable"}} +
    + {{else}} +

    + {{.i18n.Tr "repo.settings.tags.protection"}} +

    + +
    +
    +
    +
    +
    + {{.CsrfTokenHtml}} +
    + + +
    +
    + +
    + +
    {{.i18n.Tr "repo.settings.protect_whitelist_search_users"}}
    +
    + {{range .Users}} +
    + {{avatar . 28 "mini"}} + {{.GetDisplayName}} +
    + {{end}} +
    +
    +
    + {{if .Owner.IsOrganization}} +
    + +
    + +
    {{.i18n.Tr "repo.settings.protect_whitelist_search_teams"}}
    +
    + {{range .Teams}} +
    + {{svg "octicon-people"}} + {{.Name}} +
    + {{end}} +
    +
    +
    + {{end}} +
    + {{if .PageIsEditProtectedTag}} + + + {{$.i18n.Tr "cancel"}} + + {{else}} + + {{end}} +
    + +
    +
    + +
    + + + + + + + + {{range .ProtectedTags}} + + + + + + {{else}} + + {{end}} + +
    {{.i18n.Tr "repo.settings.tags.protection.pattern"}}{{.i18n.Tr "repo.settings.tags.protection.allowed"}}
    {{.NamePattern}}
    		 + {{if or .AllowlistUserIDs (and $.Owner.IsOrganization .AllowlistTeamIDs)}} + {{$userIDs := .AllowlistUserIDs}} + {{range $.Users}} + {{if contain $userIDs .ID }} + {{avatar . 26}} {{.GetDisplayName}} + {{end}} + {{end}} + {{if $.Owner.IsOrganization}} + {{$teamIDs := .AllowlistTeamIDs}} + {{range $.Teams}} + {{if contain $teamIDs .ID }} + {{.Name}} + {{end}} + {{end}} + {{end}} + {{else}} + {{$.i18n.Tr "repo.settings.tags.protection.allowed.noone"}} + {{end}} + + {{$.i18n.Tr "edit"}} +
    + {{$.CsrfTokenHtml}} + + +
    +
    {{.i18n.Tr "repo.settings.tags.protection.none"}}
    +
    +
    +
    + {{end}} +
    +
    +{{template "base/footer" .}} From 3ef23d5411732b4b714d6fc9739bc5dac75aadd4 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Fri, 25 Jun 2021 18:54:08 +0200 Subject: [PATCH 02/12] Use gitea logging module for git module (#16243) remove log() func from gogs times and switch to proper logging Signed-off-by: Andrew Thornton Co-authored-by: Andrew Thornton --- modules/git/batch_reader.go | 6 ++++-- modules/git/blob_nogogit.go | 6 ++++-- modules/git/command.go | 10 +++++----- modules/git/commit_info_nogogit.go | 4 +++- modules/git/diff.go | 3 ++- modules/git/git.go | 16 ---------------- modules/git/git_test.go | 4 ++++ modules/git/hook.go | 6 ++++-- modules/git/last_commit_cache.go | 4 +++- modules/git/last_commit_cache_gogit.go | 6 ++++-- modules/git/last_commit_cache_nogogit.go | 6 ++++-- modules/git/parse_nogogit.go | 4 +++- modules/git/repo_base_nogogit.go | 6 ++++-- modules/git/repo_branch_nogogit.go | 6 ++++-- modules/git/repo_commit_nogogit.go | 4 +++- modules/git/repo_language_stats_nogogit.go | 9 +++++---- modules/git/repo_tag.go | 4 +++- routers/init.go | 12 +----------- services/gitdiff/gitdiff_test.go | 2 -- 19 files changed, 60 insertions(+), 58 deletions(-) diff --git a/modules/git/batch_reader.go b/modules/git/batch_reader.go index 678b18470878..bdf82bde8913 100644 --- a/modules/git/batch_reader.go +++ b/modules/git/batch_reader.go @@ -12,6 +12,8 @@ import ( "strconv" "strings" + "code.gitea.io/gitea/modules/log" + "github.com/djherbis/buffer" "github.com/djherbis/nio/v3" ) @@ -99,7 +101,7 @@ func ReadBatchLine(rd *bufio.Reader) (sha []byte, typ string, size int64, err er } idx := strings.IndexByte(typ, ' ') if idx < 0 { - log("missing space typ: %s", typ) + log.Debug("missing space typ: %s", typ) err = ErrNotExist{ID: string(sha)} return } @@ -230,7 +232,7 @@ func ParseTreeLine(rd *bufio.Reader, modeBuf, fnameBuf, shaBuf []byte) (mode, fn } idx := bytes.IndexByte(readBytes, ' ') if idx < 0 { - log("missing space in readBytes ParseTreeLine: %s", readBytes) + log.Debug("missing space in readBytes ParseTreeLine: %s", readBytes) err = &ErrNotExist{} return diff --git a/modules/git/blob_nogogit.go b/modules/git/blob_nogogit.go index cdaeb636a944..5b42920ebe1d 100644 --- a/modules/git/blob_nogogit.go +++ b/modules/git/blob_nogogit.go @@ -12,6 +12,8 @@ import ( "io" "io/ioutil" "math" + + "code.gitea.io/gitea/modules/log" ) // Blob represents a Git object. @@ -69,12 +71,12 @@ func (b *Blob) Size() int64 { defer cancel() _, err := wr.Write([]byte(b.ID.String() + "\n")) if err != nil { - log("error whilst reading size for %s in %s. Error: %v", b.ID.String(), b.repo.Path, err) + log.Debug("error whilst reading size for %s in %s. Error: %v", b.ID.String(), b.repo.Path, err) return 0 } _, _, b.size, err = ReadBatchLine(rd) if err != nil { - log("error whilst reading size for %s in %s. Error: %v", b.ID.String(), b.repo.Path, err) + log.Debug("error whilst reading size for %s in %s. Error: %v", b.ID.String(), b.repo.Path, err) return 0 } diff --git a/modules/git/command.go b/modules/git/command.go index ef78464d5f1c..2e375fd4f9f9 100644 --- a/modules/git/command.go +++ b/modules/git/command.go @@ -15,6 +15,7 @@ import ( "strings" "time" + "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/process" ) @@ -114,9 +115,9 @@ func (c *Command) RunInDirTimeoutEnvFullPipelineFunc(env []string, timeout time. } if len(dir) == 0 { - log(c.String()) + log.Debug("%s", c) } else { - log("%s: %v", dir, c) + log.Debug("%s: %v", dir, c) } ctx, cancel := context.WithTimeout(c.parentContext, timeout) @@ -197,9 +198,8 @@ func (c *Command) RunInDirTimeoutEnv(env []string, timeout time.Duration, dir st if err := c.RunInDirTimeoutEnvPipeline(env, timeout, dir, stdout, stderr); err != nil { return nil, ConcatenateError(err, stderr.String()) } - - if stdout.Len() > 0 { - log("stdout:\n%s", stdout.Bytes()[:1024]) + if stdout.Len() > 0 && log.IsTrace() { + log.Trace("Stdout:\n %s", stdout.Bytes()[:1024]) } return stdout.Bytes(), nil } diff --git a/modules/git/commit_info_nogogit.go b/modules/git/commit_info_nogogit.go index 2283510d9635..060ecba26190 100644 --- a/modules/git/commit_info_nogogit.go +++ b/modules/git/commit_info_nogogit.go @@ -12,6 +12,8 @@ import ( "io" "path" "sort" + + "code.gitea.io/gitea/modules/log" ) // GetCommitsInfo gets information of all commits that are corresponding to these entries @@ -78,7 +80,7 @@ func (tes Entries) GetCommitsInfo(ctx context.Context, commit *Commit, treePath commitsInfo[i].SubModuleFile = subModuleFile } } else { - log("missing commit for %s", entry.Name()) + log.Debug("missing commit for %s", entry.Name()) } } diff --git a/modules/git/diff.go b/modules/git/diff.go index 5da53568e570..20f25c1bee3f 100644 --- a/modules/git/diff.go +++ b/modules/git/diff.go @@ -15,6 +15,7 @@ import ( "strconv" "strings" + "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/process" ) @@ -113,7 +114,7 @@ func ParseDiffHunkString(diffhunk string) (leftLine, leftHunk, rightLine, righHu righHunk, _ = strconv.Atoi(rightRange[1]) } } else { - log("Parse line number failed: %v", diffhunk) + log.Debug("Parse line number failed: %v", diffhunk) rightLine = leftLine righHunk = leftHunk } diff --git a/modules/git/git.go b/modules/git/git.go index 6b15138a5c74..ce1b15c95351 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -19,9 +19,6 @@ import ( ) var ( - // Debug enables verbose logging on everything. - // This should be false in case Gogs starts in SSH mode. - Debug = false // Prefix the log prefix Prefix = "[git-module] " // GitVersionRequired is the minimum Git version required @@ -41,19 +38,6 @@ var ( goVersionLessThan115 = true ) -func log(format string, args ...interface{}) { - if !Debug { - return - } - - fmt.Print(Prefix) - if len(args) == 0 { - fmt.Println(format) - } else { - fmt.Printf(format+"\n", args...) - } -} - // LocalVersion returns current Git version from shell. func LocalVersion() (*version.Version, error) { if err := LoadGitVersion(); err != nil { diff --git a/modules/git/git_test.go b/modules/git/git_test.go index 27951d639bb7..c62a55badc68 100644 --- a/modules/git/git_test.go +++ b/modules/git/git_test.go @@ -9,6 +9,8 @@ import ( "fmt" "os" "testing" + + "code.gitea.io/gitea/modules/log" ) func fatalTestError(fmtStr string, args ...interface{}) { @@ -17,6 +19,8 @@ func fatalTestError(fmtStr string, args ...interface{}) { } func TestMain(m *testing.M) { + _ = log.NewLogger(1000, "console", "console", `{"level":"trace","stacktracelevel":"NONE","stderr":true}`) + if err := Init(context.Background()); err != nil { fatalTestError("Init failed: %v", err) } diff --git a/modules/git/hook.go b/modules/git/hook.go index c23fbf8aa1bb..7007d23be22e 100644 --- a/modules/git/hook.go +++ b/modules/git/hook.go @@ -1,4 +1,5 @@ // Copyright 2015 The Gogs Authors. All rights reserved. +// Copyright 2021 The Gitea Authors. All rights reserved. // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. @@ -12,6 +13,7 @@ import ( "path/filepath" "strings" + "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/util" ) @@ -126,11 +128,11 @@ const ( // SetUpdateHook writes given content to update hook of the repository. func SetUpdateHook(repoPath, content string) (err error) { - log("Setting update hook: %s", repoPath) + log.Debug("Setting update hook: %s", repoPath) hookPath := path.Join(repoPath, HookPathUpdate) isExist, err := util.IsExist(hookPath) if err != nil { - log("Unable to check if %s exists. Error: %v", hookPath, err) + log.Debug("Unable to check if %s exists. Error: %v", hookPath, err) return err } if isExist { diff --git a/modules/git/last_commit_cache.go b/modules/git/last_commit_cache.go index 37a59e1fa812..e2d296641f3b 100644 --- a/modules/git/last_commit_cache.go +++ b/modules/git/last_commit_cache.go @@ -7,6 +7,8 @@ package git import ( "crypto/sha256" "fmt" + + "code.gitea.io/gitea/modules/log" ) // Cache represents a caching interface @@ -24,6 +26,6 @@ func (c *LastCommitCache) getCacheKey(repoPath, ref, entryPath string) string { // Put put the last commit id with commit and entry path func (c *LastCommitCache) Put(ref, entryPath, commitID string) error { - log("LastCommitCache save: [%s:%s:%s]", ref, entryPath, commitID) + log.Debug("LastCommitCache save: [%s:%s:%s]", ref, entryPath, commitID) return c.cache.Put(c.getCacheKey(c.repoPath, ref, entryPath), commitID, c.ttl()) } diff --git a/modules/git/last_commit_cache_gogit.go b/modules/git/last_commit_cache_gogit.go index 16fb1c988cca..b8e0db46a926 100644 --- a/modules/git/last_commit_cache_gogit.go +++ b/modules/git/last_commit_cache_gogit.go @@ -10,6 +10,8 @@ import ( "context" "path" + "code.gitea.io/gitea/modules/log" + "github.com/go-git/go-git/v5/plumbing/object" cgobject "github.com/go-git/go-git/v5/plumbing/object/commitgraph" ) @@ -41,9 +43,9 @@ func NewLastCommitCache(repoPath string, gitRepo *Repository, ttl func() int64, func (c *LastCommitCache) Get(ref, entryPath string) (interface{}, error) { v := c.cache.Get(c.getCacheKey(c.repoPath, ref, entryPath)) if vs, ok := v.(string); ok { - log("LastCommitCache hit level 1: [%s:%s:%s]", ref, entryPath, vs) + log.Debug("LastCommitCache hit level 1: [%s:%s:%s]", ref, entryPath, vs) if commit, ok := c.commitCache[vs]; ok { - log("LastCommitCache hit level 2: [%s:%s:%s]", ref, entryPath, vs) + log.Debug("LastCommitCache hit level 2: [%s:%s:%s]", ref, entryPath, vs) return commit, nil } id, err := c.repo.ConvertToSHA1(vs) diff --git a/modules/git/last_commit_cache_nogogit.go b/modules/git/last_commit_cache_nogogit.go index 84c8ee132c26..ff9f9ff1cfad 100644 --- a/modules/git/last_commit_cache_nogogit.go +++ b/modules/git/last_commit_cache_nogogit.go @@ -10,6 +10,8 @@ import ( "bufio" "context" "path" + + "code.gitea.io/gitea/modules/log" ) // LastCommitCache represents a cache to store last commit @@ -39,9 +41,9 @@ func NewLastCommitCache(repoPath string, gitRepo *Repository, ttl func() int64, func (c *LastCommitCache) Get(ref, entryPath string, wr WriteCloserError, rd *bufio.Reader) (interface{}, error) { v := c.cache.Get(c.getCacheKey(c.repoPath, ref, entryPath)) if vs, ok := v.(string); ok { - log("LastCommitCache hit level 1: [%s:%s:%s]", ref, entryPath, vs) + log.Debug("LastCommitCache hit level 1: [%s:%s:%s]", ref, entryPath, vs) if commit, ok := c.commitCache[vs]; ok { - log("LastCommitCache hit level 2: [%s:%s:%s]", ref, entryPath, vs) + log.Debug("LastCommitCache hit level 2: [%s:%s:%s]", ref, entryPath, vs) return commit, nil } id, err := c.repo.ConvertToSHA1(vs) diff --git a/modules/git/parse_nogogit.go b/modules/git/parse_nogogit.go index b45b31f23945..667111ec4a31 100644 --- a/modules/git/parse_nogogit.go +++ b/modules/git/parse_nogogit.go @@ -13,6 +13,8 @@ import ( "io" "strconv" "strings" + + "code.gitea.io/gitea/modules/log" ) // ParseTreeEntries parses the output of a `git ls-tree -l` command. @@ -120,7 +122,7 @@ loop: case "40000": entry.entryMode = EntryModeTree default: - log("Unknown mode: %v", string(mode)) + log.Debug("Unknown mode: %v", string(mode)) return nil, fmt.Errorf("unknown mode: %v", string(mode)) } diff --git a/modules/git/repo_base_nogogit.go b/modules/git/repo_base_nogogit.go index c7d6019d7731..1675967d181b 100644 --- a/modules/git/repo_base_nogogit.go +++ b/modules/git/repo_base_nogogit.go @@ -12,6 +12,8 @@ import ( "context" "errors" "path/filepath" + + "code.gitea.io/gitea/modules/log" ) // Repository represents a Git repository. @@ -54,7 +56,7 @@ func OpenRepository(repoPath string) (*Repository, error) { // CatFileBatch obtains a CatFileBatch for this repository func (repo *Repository) CatFileBatch() (WriteCloserError, *bufio.Reader, func()) { if repo.batchCancel == nil || repo.batchReader.Buffered() > 0 { - log("Opening temporary cat file batch for: %s", repo.Path) + log.Debug("Opening temporary cat file batch for: %s", repo.Path) return CatFileBatch(repo.Path) } return repo.batchWriter, repo.batchReader, func() {} @@ -63,7 +65,7 @@ func (repo *Repository) CatFileBatch() (WriteCloserError, *bufio.Reader, func()) // CatFileBatchCheck obtains a CatFileBatchCheck for this repository func (repo *Repository) CatFileBatchCheck() (WriteCloserError, *bufio.Reader, func()) { if repo.checkCancel == nil || repo.checkReader.Buffered() > 0 { - log("Opening temporary cat file batch-check: %s", repo.Path) + log.Debug("Opening temporary cat file batch-check: %s", repo.Path) return CatFileBatchCheck(repo.Path) } return repo.checkWriter, repo.checkReader, func() {} diff --git a/modules/git/repo_branch_nogogit.go b/modules/git/repo_branch_nogogit.go index dd34e4889903..7d10b8ba0fae 100644 --- a/modules/git/repo_branch_nogogit.go +++ b/modules/git/repo_branch_nogogit.go @@ -12,6 +12,8 @@ import ( "bytes" "io" "strings" + + "code.gitea.io/gitea/modules/log" ) // IsObjectExist returns true if given reference exists in the repository. @@ -24,7 +26,7 @@ func (repo *Repository) IsObjectExist(name string) bool { defer cancel() _, err := wr.Write([]byte(name + "\n")) if err != nil { - log("Error writing to CatFileBatchCheck %v", err) + log.Debug("Error writing to CatFileBatchCheck %v", err) return false } sha, _, _, err := ReadBatchLine(rd) @@ -41,7 +43,7 @@ func (repo *Repository) IsReferenceExist(name string) bool { defer cancel() _, err := wr.Write([]byte(name + "\n")) if err != nil { - log("Error writing to CatFileBatchCheck %v", err) + log.Debug("Error writing to CatFileBatchCheck %v", err) return false } _, _, _, err = ReadBatchLine(rd) diff --git a/modules/git/repo_commit_nogogit.go b/modules/git/repo_commit_nogogit.go index d00c1bfc67c7..afd5166f1d54 100644 --- a/modules/git/repo_commit_nogogit.go +++ b/modules/git/repo_commit_nogogit.go @@ -12,6 +12,8 @@ import ( "io" "io/ioutil" "strings" + + "code.gitea.io/gitea/modules/log" ) // ResolveReference resolves a name to a reference @@ -110,7 +112,7 @@ func (repo *Repository) getCommitFromBatchReader(rd *bufio.Reader, id SHA1) (*Co return commit, nil default: - log("Unknown typ: %s", typ) + log.Debug("Unknown typ: %s", typ) _, err = rd.Discard(int(size) + 1) if err != nil { return nil, err diff --git a/modules/git/repo_language_stats_nogogit.go b/modules/git/repo_language_stats_nogogit.go index 46b084cf01e3..1684f21d1675 100644 --- a/modules/git/repo_language_stats_nogogit.go +++ b/modules/git/repo_language_stats_nogogit.go @@ -13,6 +13,7 @@ import ( "math" "code.gitea.io/gitea/modules/analyze" + "code.gitea.io/gitea/modules/log" "github.com/go-enry/go-enry/v2" ) @@ -34,19 +35,19 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err } shaBytes, typ, size, err := ReadBatchLine(batchReader) if typ != "commit" { - log("Unable to get commit for: %s. Err: %v", commitID, err) + log.Debug("Unable to get commit for: %s. Err: %v", commitID, err) return nil, ErrNotExist{commitID, ""} } sha, err := NewIDFromString(string(shaBytes)) if err != nil { - log("Unable to get commit for: %s. Err: %v", commitID, err) + log.Debug("Unable to get commit for: %s. Err: %v", commitID, err) return nil, ErrNotExist{commitID, ""} } commit, err := CommitFromReader(repo, sha, io.LimitReader(batchReader, size)) if err != nil { - log("Unable to get commit for: %s. Err: %v", commitID, err) + log.Debug("Unable to get commit for: %s. Err: %v", commitID, err) return nil, err } if _, err = batchReader.Discard(1); err != nil { @@ -79,7 +80,7 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err } _, _, size, err := ReadBatchLine(batchReader) if err != nil { - log("Error reading blob: %s Err: %v", f.ID.String(), err) + log.Debug("Error reading blob: %s Err: %v", f.ID.String(), err) return nil, err } diff --git a/modules/git/repo_tag.go b/modules/git/repo_tag.go index 59ab7020964d..d91c3ca97973 100644 --- a/modules/git/repo_tag.go +++ b/modules/git/repo_tag.go @@ -8,6 +8,8 @@ package git import ( "fmt" "strings" + + "code.gitea.io/gitea/modules/log" ) // TagPrefix tags prefix path on the repository @@ -33,7 +35,7 @@ func (repo *Repository) CreateAnnotatedTag(name, message, revision string) error func (repo *Repository) getTag(tagID SHA1, name string) (*Tag, error) { t, ok := repo.tagCache.Get(tagID.String()) if ok { - log("Hit cache: %s", tagID) + log.Debug("Hit cache: %s", tagID) tagClone := *t.(*Tag) tagClone.Name = name // This is necessary because lightweight tags may have same id return &tagClone, nil diff --git a/routers/init.go b/routers/init.go index bbf39a3f509e..e52e547517f0 100644 --- a/routers/init.go +++ b/routers/init.go @@ -42,16 +42,6 @@ import ( "code.gitea.io/gitea/services/webhook" ) -func checkRunMode() { - switch setting.RunMode { - case "dev", "test": - git.Debug = true - default: - git.Debug = false - } - log.Info("Run Mode: %s", strings.Title(setting.RunMode)) -} - // NewServices init new services func NewServices() { setting.NewServices() @@ -84,7 +74,7 @@ func GlobalInit(ctx context.Context) { log.Trace("AppWorkPath: %s", setting.AppWorkPath) log.Trace("Custom path: %s", setting.CustomPath) log.Trace("Log path: %s", setting.LogRootPath) - checkRunMode() + log.Info("Run Mode: %s", strings.Title(setting.RunMode)) // Setup i18n translation.InitLocales() diff --git a/services/gitdiff/gitdiff_test.go b/services/gitdiff/gitdiff_test.go index f8c25a3912d5..2386552efec7 100644 --- a/services/gitdiff/gitdiff_test.go +++ b/services/gitdiff/gitdiff_test.go @@ -13,7 +13,6 @@ import ( "testing" "code.gitea.io/gitea/models" - "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/highlight" "code.gitea.io/gitea/modules/setting" jsoniter "github.com/json-iterator/go" @@ -514,7 +513,6 @@ func TestDiffLine_GetCommentSide(t *testing.T) { } func TestGetDiffRangeWithWhitespaceBehavior(t *testing.T) { - git.Debug = true for _, behavior := range []string{"-w", "--ignore-space-at-eol", "-b", ""} { diffs, err := GetDiffRangeWithWhitespaceBehavior("./testdata/academic-module", "559c156f8e0178b71cb44355428f24001b08fc68", "bd7063cc7c04689c4d082183d32a604ed27a24f9", setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffFiles, behavior) From f573e93ed429d1d1dd0a7be8a3b0042f0817cc00 Mon Sep 17 00:00:00 2001 From: siddweiker Date: Fri, 25 Jun 2021 12:59:25 -0400 Subject: [PATCH 03/12] Fix heatmap activity (#15252) * Group heatmap actions by 15 minute intervals Signed-off-by: Sidd Weiker * Add multi-contribution test for user heatmap Signed-off-by: Sidd Weiker * Add timezone aware summation for activity heatmap Signed-off-by: Sidd Weiker * Fix api user heatmap test Signed-off-by: Sidd Weiker * Update variable declaration style Co-authored-by: Lunny Xiao Co-authored-by: 6543 <6543@obermui.de> Co-authored-by: techknowlogick --- integrations/api_user_heatmap_test.go | 2 +- models/fixtures/action.yml | 24 +++++++++++++++++++++++ models/user_heatmap.go | 11 ++++------- models/user_heatmap_test.go | 28 +++++++++++++++++++-------- web_src/js/features/heatmap.js | 11 +++++++++-- 5 files changed, 58 insertions(+), 18 deletions(-) diff --git a/integrations/api_user_heatmap_test.go b/integrations/api_user_heatmap_test.go index 105d39e9ae2d..a0f0552a1794 100644 --- a/integrations/api_user_heatmap_test.go +++ b/integrations/api_user_heatmap_test.go @@ -26,7 +26,7 @@ func TestUserHeatmap(t *testing.T) { var heatmap []*models.UserHeatmapData DecodeJSON(t, resp, &heatmap) var dummyheatmap []*models.UserHeatmapData - dummyheatmap = append(dummyheatmap, &models.UserHeatmapData{Timestamp: 1603152000, Contributions: 1}) + dummyheatmap = append(dummyheatmap, &models.UserHeatmapData{Timestamp: 1603227600, Contributions: 1}) assert.Equal(t, dummyheatmap, heatmap) } diff --git a/models/fixtures/action.yml b/models/fixtures/action.yml index 14cfd90423cf..e3f3d2a97126 100644 --- a/models/fixtures/action.yml +++ b/models/fixtures/action.yml @@ -32,3 +32,27 @@ repo_id: 22 is_private: true created_unix: 1603267920 + +- id: 5 + user_id: 10 + op_type: 1 # create repo + act_user_id: 10 + repo_id: 6 + is_private: true + created_unix: 1603010100 + +- id: 6 + user_id: 10 + op_type: 1 # create repo + act_user_id: 10 + repo_id: 7 + is_private: true + created_unix: 1603011300 + +- id: 7 + user_id: 10 + op_type: 1 # create repo + act_user_id: 10 + repo_id: 8 + is_private: false + created_unix: 1603011540 # grouped with id:7 diff --git a/models/user_heatmap.go b/models/user_heatmap.go index 0e2767212e9d..306bd1819b70 100644 --- a/models/user_heatmap.go +++ b/models/user_heatmap.go @@ -32,17 +32,14 @@ func getUserHeatmapData(user *User, team *Team, doer *User) ([]*UserHeatmapData, return hdata, nil } - var groupBy string + // Group by 15 minute intervals which will allow the client to accurately shift the timestamp to their timezone. + // The interval is based on the fact that there are timezones such as UTC +5:30 and UTC +12:45. + groupBy := "created_unix / 900 * 900" groupByName := "timestamp" // We need this extra case because mssql doesn't allow grouping by alias switch { - case setting.Database.UseSQLite3: - groupBy = "strftime('%s', strftime('%Y-%m-%d', created_unix, 'unixepoch'))" case setting.Database.UseMySQL: - groupBy = "UNIX_TIMESTAMP(DATE(FROM_UNIXTIME(created_unix)))" - case setting.Database.UsePostgreSQL: - groupBy = "extract(epoch from date_trunc('day', to_timestamp(created_unix)))" + groupBy = "created_unix DIV 900 * 900" case setting.Database.UseMSSQL: - groupBy = "datediff(SECOND, '19700101', dateadd(DAY, 0, datediff(day, 0, dateadd(s, created_unix, '19700101'))))" groupByName = groupBy } diff --git a/models/user_heatmap_test.go b/models/user_heatmap_test.go index 31e78a19cc1b..b2aaea649948 100644 --- a/models/user_heatmap_test.go +++ b/models/user_heatmap_test.go @@ -19,12 +19,20 @@ func TestGetUserHeatmapDataByUser(t *testing.T) { CountResult int JSONResult string }{ - {2, 2, 1, `[{"timestamp":1603152000,"contributions":1}]`}, // self looks at action in private repo - {2, 1, 1, `[{"timestamp":1603152000,"contributions":1}]`}, // admin looks at action in private repo - {2, 3, 0, `[]`}, // other user looks at action in private repo - {2, 0, 0, `[]`}, // nobody looks at action in private repo - {16, 15, 1, `[{"timestamp":1603238400,"contributions":1}]`}, // collaborator looks at action in private repo - {3, 3, 0, `[]`}, // no action action not performed by target user + // self looks at action in private repo + {2, 2, 1, `[{"timestamp":1603227600,"contributions":1}]`}, + // admin looks at action in private repo + {2, 1, 1, `[{"timestamp":1603227600,"contributions":1}]`}, + // other user looks at action in private repo + {2, 3, 0, `[]`}, + // nobody looks at action in private repo + {2, 0, 0, `[]`}, + // collaborator looks at action in private repo + {16, 15, 1, `[{"timestamp":1603267200,"contributions":1}]`}, + // no action action not performed by target user + {3, 3, 0, `[]`}, + // multiple actions performed with two grouped together + {10, 10, 3, `[{"timestamp":1603009800,"contributions":1},{"timestamp":1603010700,"contributions":2}]`}, } // Prepare assert.NoError(t, PrepareTestDatabase()) @@ -51,9 +59,13 @@ func TestGetUserHeatmapDataByUser(t *testing.T) { // Get the heatmap and compare heatmap, err := GetUserHeatmapDataByUser(user, doer) + var contributions int + for _, hm := range heatmap { + contributions += int(hm.Contributions) + } assert.NoError(t, err) - assert.Len(t, heatmap, len(actions), "invalid action count: did the test data became too old?") - assert.Len(t, heatmap, tc.CountResult, fmt.Sprintf("testcase %d", i)) + assert.Len(t, actions, contributions, "invalid action count: did the test data became too old?") + assert.Equal(t, tc.CountResult, contributions, fmt.Sprintf("testcase %d", i)) // Test JSON rendering json := jsoniter.ConfigCompatibleWithStandardLibrary diff --git a/web_src/js/features/heatmap.js b/web_src/js/features/heatmap.js index d1cb43dde0ed..07ecaee46167 100644 --- a/web_src/js/features/heatmap.js +++ b/web_src/js/features/heatmap.js @@ -7,8 +7,15 @@ export default async function initHeatmap() { if (!el) return; try { - const values = JSON.parse(el.dataset.heatmapData).map(({contributions, timestamp}) => { - return {date: new Date(timestamp * 1000), count: contributions}; + const heatmap = {}; + JSON.parse(el.dataset.heatmapData).forEach(({contributions, timestamp}) => { + // Convert to user timezone and sum contributions by date + const dateStr = new Date(timestamp * 1000).toDateString(); + heatmap[dateStr] = (heatmap[dateStr] || 0) + contributions; + }); + + const values = Object.keys(heatmap).map((v) => { + return {date: new Date(v), count: heatmap[v]}; }); const View = Vue.extend({ From 31acd3c0c2de788fa995d5660b617f51d5b94078 Mon Sep 17 00:00:00 2001 From: Steven <61625851+justusbunsi@users.noreply.github.com> Date: Fri, 25 Jun 2021 19:00:09 +0200 Subject: [PATCH 04/12] Prevent webhook action buttons from shifting (#16087) On long webhook urls the action buttons (edit, delete) have been shifted by the url text. Signed-off-by: Steven Kriegler <61625851+justusbunsi@users.noreply.github.com> Co-authored-by: 6543 <6543@obermui.de> Co-authored-by: techknowlogick --- templates/repo/settings/webhook/base_list.tmpl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/templates/repo/settings/webhook/base_list.tmpl b/templates/repo/settings/webhook/base_list.tmpl index 916272a97a72..e77e747742df 100644 --- a/templates/repo/settings/webhook/base_list.tmpl +++ b/templates/repo/settings/webhook/base_list.tmpl @@ -41,7 +41,7 @@ {{.Description | Str2html}} {{range .Webhooks}} -
    +
    {{if eq .LastStatus 1}} {{svg "octicon-check"}} {{else if eq .LastStatus 2}} @@ -49,8 +49,8 @@ {{else}} {{svg "octicon-dot-fill"}} {{end}} - {{.URL}} -
    + {{.URL}} +
    {{svg "octicon-pencil"}} {{svg "octicon-trash"}}
    From 06f483d0c4d11f32faae60a2a6c77b164f7e88e6 Mon Sep 17 00:00:00 2001 From: Jimmy Praet Date: Fri, 25 Jun 2021 19:01:43 +0200 Subject: [PATCH 05/12] Append to existing trailers in generated squash commit message (#15980) * Remove superfluous newline before Co-authored-by trailers * Append to existing PR description trailer section If the existing PR description message already contains a trailer section (e.g. Signed-off-by: ), append to it instead of creating a new trailer section. * Reuse compiled regexp * Simplify regex and deal with trailing \n in PR description * Add tests for CommitMessageTrailersPattern - add support for Key:Value (no space after colon) - add support for whitespace "folding" * Update services/pull/pull_test.go Co-authored-by: Norwin Co-authored-by: zeripath Co-authored-by: Norwin Co-authored-by: Lunny Xiao Co-authored-by: techknowlogick --- services/pull/pull.go | 17 ++++++++--------- services/pull/pull_test.go | 23 +++++++++++++++++++++++ 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/services/pull/pull.go b/services/pull/pull.go index 7b5dd6a964db..db216ddbf455 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -9,6 +9,7 @@ import ( "bytes" "context" "fmt" + "regexp" "strings" "time" @@ -528,6 +529,8 @@ func CloseRepoBranchesPulls(doer *models.User, repo *models.Repository) error { return nil } +var commitMessageTrailersPattern = regexp.MustCompile(`(?:^|\n\n)(?:[\w-]+[ \t]*:[^\n]+\n*(?:[ \t]+[^\n]+\n*)*)+$`) + // GetSquashMergeCommitMessages returns the commit messages between head and merge base (if there is one) func GetSquashMergeCommitMessages(pr *models.PullRequest) string { if err := pr.LoadIssue(); err != nil { @@ -583,10 +586,13 @@ func GetSquashMergeCommitMessages(pr *models.PullRequest) string { stringBuilder := strings.Builder{} if !setting.Repository.PullRequest.PopulateSquashCommentWithCommitMessages { - stringBuilder.WriteString(pr.Issue.Content) + message := strings.TrimSpace(pr.Issue.Content) + stringBuilder.WriteString(message) if stringBuilder.Len() > 0 { stringBuilder.WriteRune('\n') - stringBuilder.WriteRune('\n') + if !commitMessageTrailersPattern.MatchString(message) { + stringBuilder.WriteRune('\n') + } } } @@ -657,13 +663,6 @@ func GetSquashMergeCommitMessages(pr *models.PullRequest) string { } } - if len(authors) > 0 { - if _, err := stringBuilder.WriteRune('\n'); err != nil { - log.Error("Unable to write to string builder Error: %v", err) - return "" - } - } - for _, author := range authors { if _, err := stringBuilder.Write([]byte("Co-authored-by: ")); err != nil { log.Error("Unable to write to string builder Error: %v", err) diff --git a/services/pull/pull_test.go b/services/pull/pull_test.go index 64920e355096..81627ebb77bc 100644 --- a/services/pull/pull_test.go +++ b/services/pull/pull_test.go @@ -5,4 +5,27 @@ package pull +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + // TODO TestPullRequest_PushToBaseRepo + +func TestPullRequest_CommitMessageTrailersPattern(t *testing.T) { + // Not a valid trailer section + assert.False(t, commitMessageTrailersPattern.MatchString("")) + assert.False(t, commitMessageTrailersPattern.MatchString("No trailer.")) + assert.False(t, commitMessageTrailersPattern.MatchString("Signed-off-by: Bob \nNot a trailer due to following text.")) + assert.False(t, commitMessageTrailersPattern.MatchString("Message body not correctly separated from trailer section by empty line.\nSigned-off-by: Bob ")) + // Valid trailer section + assert.True(t, commitMessageTrailersPattern.MatchString("Signed-off-by: Bob ")) + assert.True(t, commitMessageTrailersPattern.MatchString("Signed-off-by: Bob \nOther-Trailer: Value")) + assert.True(t, commitMessageTrailersPattern.MatchString("Message body correctly separated from trailer section by empty line.\n\nSigned-off-by: Bob ")) + assert.True(t, commitMessageTrailersPattern.MatchString("Multiple trailers.\n\nSigned-off-by: Bob \nOther-Trailer: Value")) + assert.True(t, commitMessageTrailersPattern.MatchString("Newline after trailer section.\n\nSigned-off-by: Bob \n")) + assert.True(t, commitMessageTrailersPattern.MatchString("No space after colon is accepted.\n\nSigned-off-by:Bob ")) + assert.True(t, commitMessageTrailersPattern.MatchString("Additional whitespace is accepted.\n\nSigned-off-by \t : \tBob ")) + assert.True(t, commitMessageTrailersPattern.MatchString("Folded value.\n\nFolded-trailer: This is\n a folded\n trailer value\nOther-Trailer: Value")) +} From 1a1ce9b7216ab80e94987270da8fc2def57237c0 Mon Sep 17 00:00:00 2001 From: zeripath Date: Fri, 25 Jun 2021 19:14:49 +0100 Subject: [PATCH 06/12] Fuzzer finds an NPE due to incorrect URLPrefix (#16249) The Fuzzer is running on a non-repo urlprefix which is incorrect for RenderRaw Signed-off-by: Andrew Thornton Co-authored-by: 6543 <6543@obermui.de> --- tools/fuzz.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/fuzz.go b/tools/fuzz.go index 4b5b72d1d0d5..b48ae0add944 100644 --- a/tools/fuzz.go +++ b/tools/fuzz.go @@ -23,7 +23,7 @@ import ( var ( renderContext = markup.RenderContext{ - URLPrefix: "https://example.com", + URLPrefix: "https://example.com/go-gitea/gitea", Metas: map[string]string{ "user": "go-gitea", "repo": "gitea", From 9b33d18899b7e825e4754969ffcc9d7b541d2d28 Mon Sep 17 00:00:00 2001 From: ayb Date: Sat, 26 Jun 2021 00:38:27 +0200 Subject: [PATCH 07/12] Added support for gopher URLs. (#14749) * Added support for gopher URLs. * Add setting and make this user settable instead Signed-off-by: Andrew Thornton Co-authored-by: Andrew Thornton --- custom/conf/app.example.ini | 2 ++ .../doc/advanced/config-cheat-sheet.en-us.md | 1 + modules/setting/service.go | 12 ++++++++++++ modules/validation/binding.go | 19 +++++++++++++++++++ modules/validation/helpers.go | 19 +++++++++++++++++++ services/forms/user_form.go | 2 +- 6 files changed, 54 insertions(+), 1 deletion(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 5adfb0546f0b..fa6a9e3fac0b 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -705,6 +705,8 @@ PATH = ;; ;; Minimum amount of time a user must exist before comments are kept when the user is deleted. ;USER_DELETE_WITH_COMMENTS_MAX_TIME = 0 +;; Valid site url schemes for user profiles +;VALID_SITE_URL_SCHEMES=http,https ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/docs/content/doc/advanced/config-cheat-sheet.en-us.md b/docs/content/doc/advanced/config-cheat-sheet.en-us.md index 5e976174fb19..aa9eb7e0caee 100644 --- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md +++ b/docs/content/doc/advanced/config-cheat-sheet.en-us.md @@ -519,6 +519,7 @@ relation to port exhaustion. - `NO_REPLY_ADDRESS`: **noreply.DOMAIN** Value for the domain part of the user's email address in the git log if user has set KeepEmailPrivate to true. DOMAIN resolves to the value in server.DOMAIN. The user's email will be replaced with a concatenation of the user name in lower case, "@" and NO_REPLY_ADDRESS. - `USER_DELETE_WITH_COMMENTS_MAX_TIME`: **0** Minimum amount of time a user must exist before comments are kept when the user is deleted. +- `VALID_SITE_URL_SCHEMES`: **http, https**: Valid site url schemes for user profiles ### Service - Expore (`service.explore`) diff --git a/modules/setting/service.go b/modules/setting/service.go index 41e834e8e61e..bd70c7e6ebe5 100644 --- a/modules/setting/service.go +++ b/modules/setting/service.go @@ -6,6 +6,7 @@ package setting import ( "regexp" + "strings" "time" "code.gitea.io/gitea/modules/log" @@ -55,6 +56,7 @@ var Service struct { AutoWatchOnChanges bool DefaultOrgMemberVisible bool UserDeleteWithCommentsMaxTime time.Duration + ValidSiteURLSchemes []string // OpenID settings EnableOpenIDSignIn bool @@ -120,6 +122,16 @@ func newService() { Service.DefaultOrgVisibilityMode = structs.VisibilityModes[Service.DefaultOrgVisibility] Service.DefaultOrgMemberVisible = sec.Key("DEFAULT_ORG_MEMBER_VISIBLE").MustBool() Service.UserDeleteWithCommentsMaxTime = sec.Key("USER_DELETE_WITH_COMMENTS_MAX_TIME").MustDuration(0) + sec.Key("VALID_SITE_URL_SCHEMES").MustString("http,https") + Service.ValidSiteURLSchemes = sec.Key("VALID_SITE_URL_SCHEMES").Strings(",") + schemes := make([]string, len(Service.ValidSiteURLSchemes)) + for _, scheme := range Service.ValidSiteURLSchemes { + scheme = strings.ToLower(strings.TrimSpace(scheme)) + if scheme != "" { + schemes = append(schemes, scheme) + } + } + Service.ValidSiteURLSchemes = schemes if err := Cfg.Section("service.explore").MapTo(&Service.Explore); err != nil { log.Fatal("Failed to map service.explore settings: %v", err) diff --git a/modules/validation/binding.go b/modules/validation/binding.go index 4cef48daf32d..5d5c64611f29 100644 --- a/modules/validation/binding.go +++ b/modules/validation/binding.go @@ -55,6 +55,7 @@ func CheckGitRefAdditionalRulesValid(name string) bool { func AddBindingRules() { addGitRefNameBindingRule() addValidURLBindingRule() + addValidSiteURLBindingRule() addGlobPatternRule() addRegexPatternRule() addGlobOrRegexPatternRule() @@ -102,6 +103,24 @@ func addValidURLBindingRule() { }) } +func addValidSiteURLBindingRule() { + // URL validation rule + binding.AddRule(&binding.Rule{ + IsMatch: func(rule string) bool { + return strings.HasPrefix(rule, "ValidSiteUrl") + }, + IsValid: func(errs binding.Errors, name string, val interface{}) (bool, binding.Errors) { + str := fmt.Sprintf("%v", val) + if len(str) != 0 && !IsValidSiteURL(str) { + errs.Add([]string{name}, binding.ERR_URL, "Url") + return false, errs + } + + return true, errs + }, + }) +} + func addGlobPatternRule() { binding.AddRule(&binding.Rule{ IsMatch: func(rule string) bool { diff --git a/modules/validation/helpers.go b/modules/validation/helpers.go index c22e667a2ebf..343261aac5b5 100644 --- a/modules/validation/helpers.go +++ b/modules/validation/helpers.go @@ -52,6 +52,25 @@ func IsValidURL(uri string) bool { return true } +// IsValidSiteURL checks if URL is valid +func IsValidSiteURL(uri string) bool { + u, err := url.ParseRequestURI(uri) + if err != nil { + return false + } + + if !validPort(portOnly(u.Host)) { + return false + } + + for _, scheme := range setting.Service.ValidSiteURLSchemes { + if scheme == u.Scheme { + return true + } + } + return false +} + // IsAPIURL checks if URL is current Gitea instance API URL func IsAPIURL(uri string) bool { return strings.HasPrefix(strings.ToLower(uri), strings.ToLower(setting.AppURL+"api")) diff --git a/services/forms/user_form.go b/services/forms/user_form.go index 2c065dc5116a..903a625da01e 100644 --- a/services/forms/user_form.go +++ b/services/forms/user_form.go @@ -226,7 +226,7 @@ type UpdateProfileForm struct { Name string `binding:"AlphaDashDot;MaxSize(40)"` FullName string `binding:"MaxSize(100)"` KeepEmailPrivate bool - Website string `binding:"ValidUrl;MaxSize(255)"` + Website string `binding:"ValidSiteUrl;MaxSize(255)"` Location string `binding:"MaxSize(50)"` Language string Description string `binding:"MaxSize(255)"` From 62a4879e84c7474dd72ab0eb4c54923f2690510c Mon Sep 17 00:00:00 2001 From: zeripath Date: Sat, 26 Jun 2021 00:11:33 +0100 Subject: [PATCH 08/12] Improve efficiency in FindRenderizableReferenceNumeric and getReferences (#16251) * Fuzzer finds an NPE due to incorrect URLPrefix The Fuzzer is running on a non-repo urlprefix which is incorrect for RenderRaw * Make FindRenderizableReferenceNumeric and getReferences more efficient Signed-off-by: Andrew Thornton Co-authored-by: techknowlogick --- modules/references/references.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/modules/references/references.go b/modules/references/references.go index 106e66b47bb5..ef859abcc79b 100644 --- a/modules/references/references.go +++ b/modules/references/references.go @@ -5,6 +5,7 @@ package references import ( + "bytes" "net/url" "regexp" "strconv" @@ -14,6 +15,8 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/markup/mdstripper" "code.gitea.io/gitea/modules/setting" + + "github.com/yuin/goldmark/util" ) var ( @@ -321,7 +324,7 @@ func FindRenderizableReferenceNumeric(content string, prOnly bool) (bool, *Rende return false, nil } } - r := getCrossReference([]byte(content), match[2], match[3], false, prOnly) + r := getCrossReference(util.StringToReadOnlyBytes(content), match[2], match[3], false, prOnly) if r == nil { return false, nil } @@ -465,18 +468,17 @@ func findAllIssueReferencesBytes(content []byte, links []string) []*rawReference } func getCrossReference(content []byte, start, end int, fromLink bool, prOnly bool) *rawReference { - refid := string(content[start:end]) - sep := strings.IndexAny(refid, "#!") + sep := bytes.IndexAny(content[start:end], "#!") if sep < 0 { return nil } - isPull := refid[sep] == '!' + isPull := content[start+sep] == '!' if prOnly && !isPull { return nil } - repo := refid[:sep] - issue := refid[sep+1:] - index, err := strconv.ParseInt(issue, 10, 64) + repo := string(content[start : start+sep]) + issue := string(content[start+sep+1 : end]) + index, err := strconv.ParseInt(string(issue), 10, 64) if err != nil { return nil } From 622f1e764c6230023cc1944ad727cd2ad1544b68 Mon Sep 17 00:00:00 2001 From: John Olheiser Date: Fri, 25 Jun 2021 23:16:36 -0500 Subject: [PATCH 09/12] Add better errors for disabled account recovery (#15117) Signed-off-by: jolheiser Co-authored-by: Andrew Thornton Co-authored-by: 6543 <6543@obermui.de> --- options/locale/locale_en-US.ini | 4 ++-- routers/web/user/auth.go | 1 + templates/user/auth/forgot_passwd.tmpl | 8 +++++++- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index a809f49eebae..4a79ffa7eb25 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -303,7 +303,8 @@ openid_connect_desc = The chosen OpenID URI is unknown. Associate it with a new openid_register_title = Create new account openid_register_desc = The chosen OpenID URI is unknown. Associate it with a new account here. openid_signin_desc = Enter your OpenID URI. For example: https://anne.me, bob.openid.org.cn or gnusocial.net/carry. -disable_forgot_password_mail = Account recovery is disabled. Please contact your site administrator. +disable_forgot_password_mail = Account recovery is disabled because no email is set up. Please contact your site administrator. +disable_forgot_password_mail_admin = Account recovery is only available when email is set up. Please set up email to enable account recovery. email_domain_blacklisted = You cannot register with your email address. authorize_application = Authorize Application authorize_redirect_notice = You will be redirected to %s if you authorize this application. @@ -312,7 +313,6 @@ authorize_application_description = If you grant the access, it will be able to authorize_title = Authorize "%s" to access your account? authorization_failed = Authorization failed authorization_failed_desc = The authorization failed because we detected an invalid request. Please contact the maintainer of the app you've tried to authorize. -disable_forgot_password_mail = Account recovery is disabled. Please contact your site administrator. sspi_auth_failed = SSPI authentication failed password_pwned = The password you chose is on a list of stolen passwords previously exposed in public data breaches. Please try again with a different password. password_pwned_err = Could not complete request to HaveIBeenPwned diff --git a/routers/web/user/auth.go b/routers/web/user/auth.go index 827b7cdef065..6b4beff0e081 100644 --- a/routers/web/user/auth.go +++ b/routers/web/user/auth.go @@ -1478,6 +1478,7 @@ func ForgotPasswd(ctx *context.Context) { ctx.Data["Title"] = ctx.Tr("auth.forgot_password_title") if setting.MailService == nil { + log.Warn(ctx.Tr("auth.disable_forgot_password_mail_admin")) ctx.Data["IsResetDisable"] = true ctx.HTML(http.StatusOK, tplForgotPassword) return diff --git a/templates/user/auth/forgot_passwd.tmpl b/templates/user/auth/forgot_passwd.tmpl index 241deeed4a79..2ff7acb97de4 100644 --- a/templates/user/auth/forgot_passwd.tmpl +++ b/templates/user/auth/forgot_passwd.tmpl @@ -22,7 +22,13 @@
    {{else if .IsResetDisable}} -

    {{.i18n.Tr "auth.disable_forgot_password_mail"}}

    +

    + {{if $.IsAdmin}} + {{.i18n.Tr "auth.disable_forgot_password_mail_admin"}} + {{else}} + {{.i18n.Tr "auth.disable_forgot_password_mail"}} + {{end}} +

    {{else if .ResendLimited}}

    {{.i18n.Tr "auth.resent_limit_prompt"}}

    {{end}} From e673e42f7efafb184ffbe84f6998087713d8e373 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Sat, 26 Jun 2021 11:13:51 +0200 Subject: [PATCH 10/12] Fixed issues not updated by commits (#16254) `UpdateIssuesCommit` may get called with fewer commits because of `FeedMaxCommitNum` and therefore may miss some commands. --- services/repository/push.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/services/repository/push.go b/services/repository/push.go index f031073b2e0e..dcb3bc779fc9 100644 --- a/services/repository/push.go +++ b/services/repository/push.go @@ -193,16 +193,17 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error { } commits = repo_module.ListToPushCommits(l) + + if err := repofiles.UpdateIssuesCommit(pusher, repo, commits.Commits, refName); err != nil { + log.Error("updateIssuesCommit: %v", err) + } + if len(commits.Commits) > setting.UI.FeedMaxCommitNum { commits.Commits = commits.Commits[:setting.UI.FeedMaxCommitNum] } commits.CompareURL = repo.ComposeCompareURL(opts.OldCommitID, opts.NewCommitID) notification.NotifyPushCommits(pusher, repo, opts, commits) - if err := repofiles.UpdateIssuesCommit(pusher, repo, commits.Commits, refName); err != nil { - log.Error("updateIssuesCommit: %v", err) - } - if err = models.RemoveDeletedBranch(repo.ID, branch); err != nil { log.Error("models.RemoveDeletedBranch %s/%s failed: %v", repo.ID, branch, err) } From e3c626834b34fae7728ee7869ed73ee4d1b26a26 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 26 Jun 2021 19:28:55 +0800 Subject: [PATCH 11/12] Let package git depend on setting but not opposite (#15241) * Let package git depend on setting but not opposite * private some package variables --- contrib/pr/checkout.go | 3 +- integrations/git_test.go | 6 +-- integrations/integration_test.go | 3 +- integrations/lfs_getobject_test.go | 13 ++++--- integrations/migration-test/migration_test.go | 3 +- models/migrations/migrations_test.go | 3 +- modules/git/command.go | 6 +-- modules/git/git.go | 33 ++++++++++++++++ modules/git/lfs.go | 37 ++++++++++++++++++ modules/git/repo_commit.go | 14 +++---- modules/setting/git.go | 38 ++----------------- modules/setting/lfs.go | 22 ----------- routers/api/v1/repo/commits.go | 4 +- routers/init.go | 4 +- routers/web/repo/branch.go | 7 ++-- routers/web/repo/commit.go | 6 +-- routers/web/repo/wiki.go | 3 +- 17 files changed, 113 insertions(+), 92 deletions(-) create mode 100644 modules/git/lfs.go diff --git a/contrib/pr/checkout.go b/contrib/pr/checkout.go index 9ce84f762cec..902c9ea62347 100644 --- a/contrib/pr/checkout.go +++ b/contrib/pr/checkout.go @@ -26,6 +26,7 @@ import ( "time" "code.gitea.io/gitea/models" + gitea_git "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/markup/external" "code.gitea.io/gitea/modules/setting" @@ -79,7 +80,7 @@ func runPR() { setting.RunUser = curUser.Username log.Printf("[PR] Loading fixtures data ...\n") - setting.CheckLFSVersion() + gitea_git.CheckLFSVersion() //models.LoadConfigs() /* setting.Database.Type = "sqlite3" diff --git a/integrations/git_test.go b/integrations/git_test.go index 13a60076a7e9..a9848eaa4c30 100644 --- a/integrations/git_test.go +++ b/integrations/git_test.go @@ -143,7 +143,7 @@ func standardCommitAndPushTest(t *testing.T, dstPath string) (little, big string func lfsCommitAndPushTest(t *testing.T, dstPath string) (littleLFS, bigLFS string) { t.Run("LFS", func(t *testing.T) { defer PrintCurrentTest(t)() - setting.CheckLFSVersion() + git.CheckLFSVersion() if !setting.LFS.StartServer { t.Skip() return @@ -213,7 +213,7 @@ func rawTest(t *testing.T, ctx *APITestContext, little, big, littleLFS, bigLFS s resp := session.MakeRequestNilResponseRecorder(t, req, http.StatusOK) assert.Equal(t, littleSize, resp.Length) - setting.CheckLFSVersion() + git.CheckLFSVersion() if setting.LFS.StartServer { req = NewRequest(t, "GET", path.Join("/", username, reponame, "/raw/branch/master/", littleLFS)) resp := session.MakeRequest(t, req, http.StatusOK) @@ -255,7 +255,7 @@ func mediaTest(t *testing.T, ctx *APITestContext, little, big, littleLFS, bigLFS resp := session.MakeRequestNilResponseRecorder(t, req, http.StatusOK) assert.Equal(t, littleSize, resp.Length) - setting.CheckLFSVersion() + git.CheckLFSVersion() if setting.LFS.StartServer { req = NewRequest(t, "GET", path.Join("/", username, reponame, "/media/branch/master/", littleLFS)) resp = session.MakeRequestNilResponseRecorder(t, req, http.StatusOK) diff --git a/integrations/integration_test.go b/integrations/integration_test.go index d755977d1ab1..8a008ac62176 100644 --- a/integrations/integration_test.go +++ b/integrations/integration_test.go @@ -26,6 +26,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/base" + "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/graceful" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/queue" @@ -162,7 +163,7 @@ func initIntegrationTest() { setting.SetCustomPathAndConf("", "", "") setting.NewContext() util.RemoveAll(models.LocalCopyPath()) - setting.CheckLFSVersion() + git.CheckLFSVersion() setting.InitDBConfig() if err := storage.Init(); err != nil { fmt.Printf("Init storage failed: %v", err) diff --git a/integrations/lfs_getobject_test.go b/integrations/lfs_getobject_test.go index 337a93567a49..c99500f469ea 100644 --- a/integrations/lfs_getobject_test.go +++ b/integrations/lfs_getobject_test.go @@ -13,6 +13,7 @@ import ( "testing" "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/lfs" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/routers/web" @@ -81,7 +82,7 @@ func checkResponseTestContentEncoding(t *testing.T, content *[]byte, resp *httpt func TestGetLFSSmall(t *testing.T) { defer prepareTestEnv(t)() - setting.CheckLFSVersion() + git.CheckLFSVersion() if !setting.LFS.StartServer { t.Skip() return @@ -94,7 +95,7 @@ func TestGetLFSSmall(t *testing.T) { func TestGetLFSLarge(t *testing.T) { defer prepareTestEnv(t)() - setting.CheckLFSVersion() + git.CheckLFSVersion() if !setting.LFS.StartServer { t.Skip() return @@ -110,7 +111,7 @@ func TestGetLFSLarge(t *testing.T) { func TestGetLFSGzip(t *testing.T) { defer prepareTestEnv(t)() - setting.CheckLFSVersion() + git.CheckLFSVersion() if !setting.LFS.StartServer { t.Skip() return @@ -131,7 +132,7 @@ func TestGetLFSGzip(t *testing.T) { func TestGetLFSZip(t *testing.T) { defer prepareTestEnv(t)() - setting.CheckLFSVersion() + git.CheckLFSVersion() if !setting.LFS.StartServer { t.Skip() return @@ -154,7 +155,7 @@ func TestGetLFSZip(t *testing.T) { func TestGetLFSRangeNo(t *testing.T) { defer prepareTestEnv(t)() - setting.CheckLFSVersion() + git.CheckLFSVersion() if !setting.LFS.StartServer { t.Skip() return @@ -167,7 +168,7 @@ func TestGetLFSRangeNo(t *testing.T) { func TestGetLFSRange(t *testing.T) { defer prepareTestEnv(t)() - setting.CheckLFSVersion() + git.CheckLFSVersion() if !setting.LFS.StartServer { t.Skip() return diff --git a/integrations/migration-test/migration_test.go b/integrations/migration-test/migration_test.go index 852c0b737c2d..209ff5a058f4 100644 --- a/integrations/migration-test/migration_test.go +++ b/integrations/migration-test/migration_test.go @@ -23,6 +23,7 @@ import ( "code.gitea.io/gitea/models/migrations" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/charset" + "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" @@ -61,7 +62,7 @@ func initMigrationTest(t *testing.T) func() { assert.NoError(t, util.RemoveAll(setting.RepoRootPath)) assert.NoError(t, util.CopyDir(path.Join(filepath.Dir(setting.AppPath), "integrations/gitea-repositories-meta"), setting.RepoRootPath)) - setting.CheckLFSVersion() + git.CheckLFSVersion() setting.InitDBConfig() setting.NewLogServices(true) return deferFn diff --git a/models/migrations/migrations_test.go b/models/migrations/migrations_test.go index 641d972b8b37..26066580d897 100644 --- a/models/migrations/migrations_test.go +++ b/models/migrations/migrations_test.go @@ -16,6 +16,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/base" + "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/util" @@ -55,7 +56,7 @@ func TestMain(m *testing.M) { setting.SetCustomPathAndConf("", "", "") setting.NewContext() - setting.CheckLFSVersion() + git.CheckLFSVersion() setting.InitDBConfig() setting.NewLogServices(true) diff --git a/modules/git/command.go b/modules/git/command.go index 2e375fd4f9f9..127e95ecfb84 100644 --- a/modules/git/command.go +++ b/modules/git/command.go @@ -23,8 +23,8 @@ var ( // GlobalCommandArgs global command args for external package setting GlobalCommandArgs []string - // DefaultCommandExecutionTimeout default command execution timeout duration - DefaultCommandExecutionTimeout = 360 * time.Second + // defaultCommandExecutionTimeout default command execution timeout duration + defaultCommandExecutionTimeout = 360 * time.Second ) // DefaultLocale is the default LC_ALL to run git commands in. @@ -111,7 +111,7 @@ func (c *Command) RunInDirTimeoutEnvFullPipeline(env []string, timeout time.Dura // it pipes stdout and stderr to given io.Writer and passes in an io.Reader as stdin. Between cmd.Start and cmd.Wait the passed in function is run. func (c *Command) RunInDirTimeoutEnvFullPipelineFunc(env []string, timeout time.Duration, dir string, stdout, stderr io.Writer, stdin io.Reader, fn func(context.Context, context.CancelFunc) error) error { if timeout == -1 { - timeout = DefaultCommandExecutionTimeout + timeout = defaultCommandExecutionTimeout } if len(dir) == 0 { diff --git a/modules/git/git.go b/modules/git/git.go index ce1b15c95351..ef6ec0c2bf32 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -14,6 +14,7 @@ import ( "time" "code.gitea.io/gitea/modules/process" + "code.gitea.io/gitea/modules/setting" "github.com/hashicorp/go-version" ) @@ -106,10 +107,42 @@ func SetExecutablePath(path string) error { return nil } +// VersionInfo returns git version information +func VersionInfo() string { + var format = "Git Version: %s" + var args = []interface{}{gitVersion.Original()} + // Since git wire protocol has been released from git v2.18 + if setting.Git.EnableAutoGitWireProtocol && CheckGitVersionAtLeast("2.18") == nil { + format += ", Wire Protocol %s Enabled" + args = append(args, "Version 2") // for focus color + } + + return fmt.Sprintf(format, args...) +} + // Init initializes git module func Init(ctx context.Context) error { DefaultContext = ctx + defaultCommandExecutionTimeout = time.Duration(setting.Git.Timeout.Default) * time.Second + + if err := SetExecutablePath(setting.Git.Path); err != nil { + return err + } + + // force cleanup args + GlobalCommandArgs = []string{} + + if CheckGitVersionAtLeast("2.9") == nil { + // Explicitly disable credential helper, otherwise Git credentials might leak + GlobalCommandArgs = append(GlobalCommandArgs, "-c", "credential.helper=") + } + + // Since git wire protocol has been released from git v2.18 + if setting.Git.EnableAutoGitWireProtocol && CheckGitVersionAtLeast("2.18") == nil { + GlobalCommandArgs = append(GlobalCommandArgs, "-c", "protocol.version=2") + } + // Save current git version on init to gitVersion otherwise it would require an RWMutex if err := LoadGitVersion(); err != nil { return err diff --git a/modules/git/lfs.go b/modules/git/lfs.go new file mode 100644 index 000000000000..79049c98245a --- /dev/null +++ b/modules/git/lfs.go @@ -0,0 +1,37 @@ +// Copyright 2021 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package git + +import ( + "sync" + + logger "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/setting" +) + +var once sync.Once + +// CheckLFSVersion will check lfs version, if not satisfied, then disable it. +func CheckLFSVersion() { + if setting.LFS.StartServer { + //Disable LFS client hooks if installed for the current OS user + //Needs at least git v2.1.2 + + err := LoadGitVersion() + if err != nil { + logger.Fatal("Error retrieving git version: %v", err) + } + + if CheckGitVersionAtLeast("2.1.2") != nil { + setting.LFS.StartServer = false + logger.Error("LFS server support needs at least Git v2.1.2") + } else { + once.Do(func() { + GlobalCommandArgs = append(GlobalCommandArgs, "-c", "filter.lfs.required=", + "-c", "filter.lfs.smudge=", "-c", "filter.lfs.clean=") + }) + } + } +} diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index 664a7445dd9d..815aa141e532 100644 --- a/modules/git/repo_commit.go +++ b/modules/git/repo_commit.go @@ -12,6 +12,8 @@ import ( "io/ioutil" "strconv" "strings" + + "code.gitea.io/gitea/modules/setting" ) // GetBranchCommitID returns last commit ID string of given branch. @@ -85,12 +87,6 @@ func (repo *Repository) GetCommitByPath(relpath string) (*Commit, error) { return commits.Front().Value.(*Commit), nil } -// CommitsRangeSize the default commits range size -var CommitsRangeSize = 50 - -// BranchesRangeSize the default branches range size -var BranchesRangeSize = 20 - func (repo *Repository) commitsByRange(id SHA1, page, pageSize int) (*list.List, error) { stdout, err := NewCommand("log", id.String(), "--skip="+strconv.Itoa((page-1)*pageSize), "--max-count="+strconv.Itoa(pageSize), prettyLogFormat).RunInDirBytes(repo.Path) @@ -206,7 +202,7 @@ func (repo *Repository) FileCommitsCount(revision, file string) (int64, error) { // CommitsByFileAndRange return the commits according revison file and the page func (repo *Repository) CommitsByFileAndRange(revision, file string, page int) (*list.List, error) { - skip := (page - 1) * CommitsRangeSize + skip := (page - 1) * setting.Git.CommitsRangeSize stdoutReader, stdoutWriter := io.Pipe() defer func() { @@ -216,7 +212,7 @@ func (repo *Repository) CommitsByFileAndRange(revision, file string, page int) ( go func() { stderr := strings.Builder{} err := NewCommand("log", revision, "--follow", - "--max-count="+strconv.Itoa(CommitsRangeSize*page), + "--max-count="+strconv.Itoa(setting.Git.CommitsRangeSize*page), prettyLogFormat, "--", file). RunInDirPipeline(repo.Path, stdoutWriter, &stderr) if err != nil { @@ -247,7 +243,7 @@ func (repo *Repository) CommitsByFileAndRange(revision, file string, page int) ( // CommitsByFileAndRangeNoFollow return the commits according revison file and the page func (repo *Repository) CommitsByFileAndRangeNoFollow(revision, file string, page int) (*list.List, error) { stdout, err := NewCommand("log", revision, "--skip="+strconv.Itoa((page-1)*50), - "--max-count="+strconv.Itoa(CommitsRangeSize), prettyLogFormat, "--", file).RunInDirBytes(repo.Path) + "--max-count="+strconv.Itoa(setting.Git.CommitsRangeSize), prettyLogFormat, "--", file).RunInDirBytes(repo.Path) if err != nil { return nil, err } diff --git a/modules/setting/git.go b/modules/setting/git.go index 308d94894ba7..7383996cb972 100644 --- a/modules/setting/git.go +++ b/modules/setting/git.go @@ -7,7 +7,6 @@ package setting import ( "time" - "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" ) @@ -19,8 +18,8 @@ var ( MaxGitDiffLines int MaxGitDiffLineCharacters int MaxGitDiffFiles int - CommitsRangeSize int - BranchesRangeSize int + CommitsRangeSize int // CommitsRangeSize the default commits range size + BranchesRangeSize int // BranchesRangeSize the default branches range size VerbosePush bool VerbosePushDelay time.Duration GCArgs []string `ini:"GC_ARGS" delim:" "` @@ -54,7 +53,7 @@ var ( Pull int GC int `ini:"GC"` }{ - Default: int(git.DefaultCommandExecutionTimeout / time.Second), + Default: 360, Migrate: 600, Mirror: 300, Clone: 300, @@ -68,35 +67,4 @@ func newGit() { if err := Cfg.Section("git").MapTo(&Git); err != nil { log.Fatal("Failed to map Git settings: %v", err) } - if err := git.SetExecutablePath(Git.Path); err != nil { - log.Fatal("Failed to initialize Git settings: %v", err) - } - git.DefaultCommandExecutionTimeout = time.Duration(Git.Timeout.Default) * time.Second - - version, err := git.LocalVersion() - if err != nil { - log.Fatal("Error retrieving git version: %v", err) - } - - // force cleanup args - git.GlobalCommandArgs = []string{} - - if git.CheckGitVersionAtLeast("2.9") == nil { - // Explicitly disable credential helper, otherwise Git credentials might leak - git.GlobalCommandArgs = append(git.GlobalCommandArgs, "-c", "credential.helper=") - } - - var format = "Git Version: %s" - var args = []interface{}{version.Original()} - // Since git wire protocol has been released from git v2.18 - if Git.EnableAutoGitWireProtocol && git.CheckGitVersionAtLeast("2.18") == nil { - git.GlobalCommandArgs = append(git.GlobalCommandArgs, "-c", "protocol.version=2") - format += ", Wire Protocol %s Enabled" - args = append(args, "Version 2") // for focus color - } - - git.CommitsRangeSize = Git.CommitsRangeSize - git.BranchesRangeSize = Git.BranchesRangeSize - - log.Info(format, args...) } diff --git a/modules/setting/lfs.go b/modules/setting/lfs.go index 8b9224b86a6c..a4bbd3c3ff37 100644 --- a/modules/setting/lfs.go +++ b/modules/setting/lfs.go @@ -9,7 +9,6 @@ import ( "time" "code.gitea.io/gitea/modules/generate" - "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" ini "gopkg.in/ini.v1" @@ -67,24 +66,3 @@ func newLFSService() { } } } - -// CheckLFSVersion will check lfs version, if not satisfied, then disable it. -func CheckLFSVersion() { - if LFS.StartServer { - //Disable LFS client hooks if installed for the current OS user - //Needs at least git v2.1.2 - - err := git.LoadGitVersion() - if err != nil { - log.Fatal("Error retrieving git version: %v", err) - } - - if git.CheckGitVersionAtLeast("2.1.2") != nil { - LFS.StartServer = false - log.Error("LFS server support needs at least Git v2.1.2") - } else { - git.GlobalCommandArgs = append(git.GlobalCommandArgs, "-c", "filter.lfs.required=", - "-c", "filter.lfs.smudge=", "-c", "filter.lfs.clean=") - } - } -} diff --git a/routers/api/v1/repo/commits.go b/routers/api/v1/repo/commits.go index a16cca0f4e14..9a0fd1d0b6f1 100644 --- a/routers/api/v1/repo/commits.go +++ b/routers/api/v1/repo/commits.go @@ -143,8 +143,8 @@ func GetAllCommits(ctx *context.APIContext) { listOptions.Page = 1 } - if listOptions.PageSize > git.CommitsRangeSize { - listOptions.PageSize = git.CommitsRangeSize + if listOptions.PageSize > setting.Git.CommitsRangeSize { + listOptions.PageSize = setting.Git.CommitsRangeSize } sha := ctx.Query("sha") diff --git a/routers/init.go b/routers/init.go index e52e547517f0..cc9f70321453 100644 --- a/routers/init.go +++ b/routers/init.go @@ -69,7 +69,9 @@ func GlobalInit(ctx context.Context) { if err := git.Init(ctx); err != nil { log.Fatal("Git module init failed: %v", err) } - setting.CheckLFSVersion() + log.Info(git.VersionInfo()) + + git.CheckLFSVersion() log.Trace("AppPath: %s", setting.AppPath) log.Trace("AppWorkPath: %s", setting.AppWorkPath) log.Trace("Custom path: %s", setting.CustomPath) diff --git a/routers/web/repo/branch.go b/routers/web/repo/branch.go index 4625b1a272fc..da72940144fe 100644 --- a/routers/web/repo/branch.go +++ b/routers/web/repo/branch.go @@ -18,6 +18,7 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/repofiles" repo_module "code.gitea.io/gitea/modules/repository" + "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/routers/utils" @@ -62,8 +63,8 @@ func Branches(ctx *context.Context) { } limit := ctx.QueryInt("limit") - if limit <= 0 || limit > git.BranchesRangeSize { - limit = git.BranchesRangeSize + if limit <= 0 || limit > setting.Git.BranchesRangeSize { + limit = setting.Git.BranchesRangeSize } skip := (page - 1) * limit @@ -73,7 +74,7 @@ func Branches(ctx *context.Context) { return } ctx.Data["Branches"] = branches - pager := context.NewPagination(int(branchesCount), git.BranchesRangeSize, page, 5) + pager := context.NewPagination(int(branchesCount), setting.Git.BranchesRangeSize, page, 5) pager.SetDefaultParams(ctx) ctx.Data["Page"] = pager diff --git a/routers/web/repo/commit.go b/routers/web/repo/commit.go index 3e6148bcbbc7..45ef22f498be 100644 --- a/routers/web/repo/commit.go +++ b/routers/web/repo/commit.go @@ -63,7 +63,7 @@ func Commits(ctx *context.Context) { pageSize := ctx.QueryInt("limit") if pageSize <= 0 { - pageSize = git.CommitsRangeSize + pageSize = setting.Git.CommitsRangeSize } // Both `git log branchName` and `git log commitId` work. @@ -82,7 +82,7 @@ func Commits(ctx *context.Context) { ctx.Data["CommitCount"] = commitsCount ctx.Data["Branch"] = ctx.Repo.BranchName - pager := context.NewPagination(int(commitsCount), git.CommitsRangeSize, page, 5) + pager := context.NewPagination(int(commitsCount), setting.Git.CommitsRangeSize, page, 5) pager.SetDefaultParams(ctx) ctx.Data["Page"] = pager @@ -250,7 +250,7 @@ func FileHistory(ctx *context.Context) { ctx.Data["CommitCount"] = commitsCount ctx.Data["Branch"] = branchName - pager := context.NewPagination(int(commitsCount), git.CommitsRangeSize, page, 5) + pager := context.NewPagination(int(commitsCount), setting.Git.CommitsRangeSize, page, 5) pager.SetDefaultParams(ctx) ctx.Data["Page"] = pager diff --git a/routers/web/repo/wiki.go b/routers/web/repo/wiki.go index cceb8451e58f..5271fe9b4ad9 100644 --- a/routers/web/repo/wiki.go +++ b/routers/web/repo/wiki.go @@ -21,6 +21,7 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/markup/markdown" + "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" @@ -316,7 +317,7 @@ func renderRevisionPage(ctx *context.Context) (*git.Repository, *git.TreeEntry) ctx.Data["Commits"] = commitsHistory - pager := context.NewPagination(int(commitsCount), git.CommitsRangeSize, page, 5) + pager := context.NewPagination(int(commitsCount), setting.Git.CommitsRangeSize, page, 5) pager.SetDefaultParams(ctx) ctx.Data["Page"] = pager From 19ac575d572af655ab691f829d0b4de38a1f10be Mon Sep 17 00:00:00 2001 From: zeripath Date: Sat, 26 Jun 2021 13:47:56 +0100 Subject: [PATCH 12/12] Limit stdout tracelog to actual stdout (#16258) Related #16243 Signed-off-by: Andrew Thornton --- modules/git/command.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/modules/git/command.go b/modules/git/command.go index 127e95ecfb84..d83c42fdc218 100644 --- a/modules/git/command.go +++ b/modules/git/command.go @@ -199,7 +199,11 @@ func (c *Command) RunInDirTimeoutEnv(env []string, timeout time.Duration, dir st return nil, ConcatenateError(err, stderr.String()) } if stdout.Len() > 0 && log.IsTrace() { - log.Trace("Stdout:\n %s", stdout.Bytes()[:1024]) + tracelen := stdout.Len() + if tracelen > 1024 { + tracelen = 1024 + } + log.Trace("Stdout:\n %s", stdout.Bytes()[:tracelen]) } return stdout.Bytes(), nil }