Implement webhook branch filter (#7791)

* Fix validate() function to handle errors in embedded anon structs

* Implement webhook branch filter

See #2025, #3998.

Author: WGH-

go.mod

@@ -47,6 +47,7 @@ require (
 	github.com/go-sql-driver/mysql v1.4.1
 	github.com/go-swagger/go-swagger v0.20.1
 	github.com/go-xorm/xorm v0.7.7-0.20190822154023-17592d96b35b
+	github.com/gobwas/glob v0.2.3
 	github.com/gogits/chardet v0.0.0-20150115103509-2404f7772561
 	github.com/gogs/cron v0.0.0-20171120032916-9f6c956d3e14
 	github.com/google/go-github/v24 v24.0.1

go.sum

@@ -236,6 +236,8 @@ github.com/go-xorm/sqlfiddle v0.0.0-20180821085327-62ce714f951a/go.mod h1:56xuuq
 github.com/go-xorm/xorm v0.7.6/go.mod h1:nqz2TAsuOHWH2yk4FYWtacCGgdbrcdZ5mF1XadqEHls=
 github.com/go-xorm/xorm v0.7.7-0.20190822154023-17592d96b35b h1:Y0hWUheXDHpIs7BWtJcykO4d1VOsVDKg1PsP5YJwxxM=
 github.com/go-xorm/xorm v0.7.7-0.20190822154023-17592d96b35b/go.mod h1:nqz2TAsuOHWH2yk4FYWtacCGgdbrcdZ5mF1XadqEHls=
+github.com/gobwas/glob v0.2.3 h1:A4xDbljILXROh+kObIiy5kIaPYD8e96x1tgBhUI5J+Y=
+github.com/gobwas/glob v0.2.3/go.mod h1:d3Ez4x06l9bZtSvzIay5+Yzi0fmZzPgnTbPcKjJAkT8=
 github.com/gogits/chardet v0.0.0-20150115103509-2404f7772561 h1:deE7ritpK04PgtpyVOS2TYcQEld9qLCD5b5EbVNOuLA=
 github.com/gogits/chardet v0.0.0-20150115103509-2404f7772561/go.mod h1:YgYOrVn3Nj9Tq0EvjmFbphRytDj7JNRoWSStJZWDJTQ=
 github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ=

models/fixtures/webhook.yml

@@ -22,3 +22,10 @@
   content_type: 1 # json
   events: '{"push_only":false,"send_everything":false,"choose_events":false,"events":{"create":false,"push":true,"pull_request":true}}'
   is_active: true
+-
+  id: 4
+  repo_id: 2
+  url: www.example.com/url4
+  content_type: 1 # json
+  events: '{"push_only":true,"branch_filter":"{master,feature*}"}'
+  is_active: true

models/webhook.go

@@ -19,12 +19,14 @@ import (
 	"strings"
 	"time"
 
+	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
 	api "code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/sync"
 	"code.gitea.io/gitea/modules/timeutil"
 
+	"github.com/gobwas/glob"
 	gouuid "github.com/satori/go.uuid"
 	"github.com/unknwon/com"
 )
@@ -84,9 +86,10 @@ type HookEvents struct {
 
 // HookEvent represents events that will delivery hook.
 type HookEvent struct {
-	PushOnly       bool `json:"push_only"`
-	SendEverything bool `json:"send_everything"`
-	ChooseEvents   bool `json:"choose_events"`
+	PushOnly       bool   `json:"push_only"`
+	SendEverything bool   `json:"send_everything"`
+	ChooseEvents   bool   `json:"choose_events"`
+	BranchFilter   string `json:"branch_filter"`
 
 	HookEvents `json:"events"`
 }
@@ -256,6 +259,21 @@ func (w *Webhook) EventsArray() []string {
 	return events
 }
 
+func (w *Webhook) checkBranch(branch string) bool {
+	if w.BranchFilter == "" || w.BranchFilter == "*" {
+		return true
+	}
+
+	g, err := glob.Compile(w.BranchFilter)
+	if err != nil {
+		// should not really happen as BranchFilter is validated
+		log.Error("CheckBranch failed: %s", err)
+		return false
+	}
+
+	return g.Match(branch)
+}
+
 // CreateWebhook creates a new web hook.
 func CreateWebhook(w *Webhook) error {
 	return createWebhook(x, w)
@@ -651,6 +669,25 @@ func PrepareWebhook(w *Webhook, repo *Repository, event HookEventType, p api.Pay
 	return prepareWebhook(x, w, repo, event, p)
 }
 
+// getPayloadBranch returns branch for hook event, if applicable.
+func getPayloadBranch(p api.Payloader) string {
+	switch pp := p.(type) {
+	case *api.CreatePayload:
+		if pp.RefType == "branch" {
+			return pp.Ref
+		}
+	case *api.DeletePayload:
+		if pp.RefType == "branch" {
+			return pp.Ref
+		}
+	case *api.PushPayload:
+		if strings.HasPrefix(pp.Ref, git.BranchPrefix) {
+			return pp.Ref[len(git.BranchPrefix):]
+		}
+	}
+	return ""
+}
+
 func prepareWebhook(e Engine, w *Webhook, repo *Repository, event HookEventType, p api.Payloader) error {
 	for _, e := range w.eventCheckers() {
 		if event == e.typ {
@@ -660,6 +697,15 @@ func prepareWebhook(e Engine, w *Webhook, repo *Repository, event HookEventType,
 		}
 	}
 
+	// If payload has no associated branch (e.g. it's a new tag, issue, etc.),
+	// branch filter has no effect.
+	if branch := getPayloadBranch(p); branch != "" {
+		if !w.checkBranch(branch) {
+			log.Info("Branch %q doesn't match branch filter %q, skipping", branch, w.BranchFilter)
+			return nil
+		}
+	}
+
 	var payloader api.Payloader
 	var err error
 	// Use separate objects so modifications won't be made on payload on non-Gogs/Gitea type hooks.

models/webhook_test.go

@@ -270,6 +270,40 @@ func TestPrepareWebhooks(t *testing.T) {
 	}
 }
 
+func TestPrepareWebhooksBranchFilterMatch(t *testing.T) {
+	assert.NoError(t, PrepareTestDatabase())
+
+	repo := AssertExistsAndLoadBean(t, &Repository{ID: 2}).(*Repository)
+	hookTasks := []*HookTask{
+		{RepoID: repo.ID, HookID: 4, EventType: HookEventPush},
+	}
+	for _, hookTask := range hookTasks {
+		AssertNotExistsBean(t, hookTask)
+	}
+	// this test also ensures that * doesn't handle / in any special way (like shell would)
+	assert.NoError(t, PrepareWebhooks(repo, HookEventPush, &api.PushPayload{Ref: "refs/heads/feature/7791"}))
+	for _, hookTask := range hookTasks {
+		AssertExistsAndLoadBean(t, hookTask)
+	}
+}
+
+func TestPrepareWebhooksBranchFilterNoMatch(t *testing.T) {
+	assert.NoError(t, PrepareTestDatabase())
+
+	repo := AssertExistsAndLoadBean(t, &Repository{ID: 2}).(*Repository)
+	hookTasks := []*HookTask{
+		{RepoID: repo.ID, HookID: 4, EventType: HookEventPush},
+	}
+	for _, hookTask := range hookTasks {
+		AssertNotExistsBean(t, hookTask)
+	}
+	assert.NoError(t, PrepareWebhooks(repo, HookEventPush, &api.PushPayload{Ref: "refs/heads/fix_weird_bug"}))
+
+	for _, hookTask := range hookTasks {
+		AssertNotExistsBean(t, hookTask)
+	}
+}
+
 // TODO TestHookTask_deliver
 
 // TODO TestDeliverHooks

modules/auth/auth.go

@@ -310,6 +310,10 @@ func validate(errs binding.Errors, data map[string]interface{}, f Form, l macaro
 	}
 
 	data["HasError"] = true
+	// If the field with name errs[0].FieldNames[0] is not found in form
+	// somehow, some code later on will panic on Data["ErrorMsg"].(string).
+	// So initialize it to some default.
+	data["ErrorMsg"] = l.Tr("form.unknown_error")
 	AssignForm(f, data)
 
 	typ := reflect.TypeOf(f)
@@ -320,16 +324,9 @@ func validate(errs binding.Errors, data map[string]interface{}, f Form, l macaro
 		val = val.Elem()
 	}
 
-	for i := 0; i < typ.NumField(); i++ {
-		field := typ.Field(i)
-
+	if field, ok := typ.FieldByName(errs[0].FieldNames[0]); ok {
 		fieldName := field.Tag.Get("form")
-		// Allow ignored fields in the struct
-		if fieldName == "-" {
-			continue
-		}
-
-		if errs[0].FieldNames[0] == field.Name {
+		if fieldName != "-" {
 			data["Err_"+field.Name] = true
 
 			trName := field.Tag.Get("locale")
@@ -360,6 +357,8 @@ func validate(errs binding.Errors, data map[string]interface{}, f Form, l macaro
 				data["ErrorMsg"] = trName + l.Tr("form.url_error")
 			case binding.ERR_INCLUDE:
 				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)
 			default:
 				data["ErrorMsg"] = l.Tr("form.unknown_error") + " " + errs[0].Classification
 			}

modules/auth/repo_form.go

@@ -185,6 +185,7 @@ type WebhookForm struct {
 	PullRequest  bool
 	Repository   bool
 	Active       bool
+	BranchFilter string `binding:"GlobPattern"`
 }
 
 // PushOnly if the hook will be triggered when push

modules/structs/hook.go

@@ -40,17 +40,19 @@ type CreateHookOption struct {
 	// enum: gitea,gogs,slack,discord
 	Type string `json:"type" binding:"Required"`
 	// required: true
-	Config map[string]string `json:"config" binding:"Required"`
-	Events []string          `json:"events"`
+	Config       map[string]string `json:"config" binding:"Required"`
+	Events       []string          `json:"events"`
+	BranchFilter string            `json:"branch_filter" binding:"GlobPattern"`
 	// default: false
 	Active bool `json:"active"`
 }
 
 // EditHookOption options when modify one hook
 type EditHookOption struct {
-	Config map[string]string `json:"config"`
-	Events []string          `json:"events"`
-	Active *bool             `json:"active"`
+	Config       map[string]string `json:"config"`
+	Events       []string          `json:"events"`
+	BranchFilter string            `json:"branch_filter" binding:"GlobPattern"`
+	Active       *bool             `json:"active"`
 }
 
 // Payloader payload is some part of one hook

modules/validation/binding.go

@@ -10,11 +10,15 @@ import (
 	"strings"
 
 	"gitea.com/macaron/binding"
+	"github.com/gobwas/glob"
 )
 
 const (
 	// ErrGitRefName is git reference name error
 	ErrGitRefName = "GitRefNameError"
+
+	// ErrGlobPattern is returned when glob pattern is invalid
+	ErrGlobPattern = "GlobPattern"
 )
 
 var (
@@ -28,6 +32,7 @@ var (
 func AddBindingRules() {
 	addGitRefNameBindingRule()
 	addValidURLBindingRule()
+	addGlobPatternRule()
 }
 
 func addGitRefNameBindingRule() {
@@ -82,6 +87,26 @@ func addValidURLBindingRule() {
 	})
 }
 
+func addGlobPatternRule() {
+	binding.AddRule(&binding.Rule{
+		IsMatch: func(rule string) bool {
+			return rule == "GlobPattern"
+		},
+		IsValid: func(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 portOnly(hostport string) string {
 	colon := strings.IndexByte(hostport, ':')
 	if colon == -1 {

modules/validation/binding_test.go

@@ -26,8 +26,9 @@ type (
 	}
 
 	TestForm struct {
-		BranchName string `form:"BranchName" binding:"GitRefName"`
-		URL        string `form:"ValidUrl" binding:"ValidUrl"`
+		BranchName  string `form:"BranchName" binding:"GitRefName"`
+		URL         string `form:"ValidUrl" binding:"ValidUrl"`
+		GlobPattern string `form:"GlobPattern" binding:"GlobPattern"`
 	}
 )
 

modules/validation/glob_pattern_test.go

@@ -0,0 +1,62 @@
+// Copyright 2019 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 (
+	"testing"
+
+	"gitea.com/macaron/binding"
+	"github.com/gobwas/glob"
+)
+
+func getGlobPatternErrorString(pattern string) string {
+	// It would be unwise to rely on that glob
+	// compilation errors don't ever change.
+	if _, err := glob.Compile(pattern); err != nil {
+		return err.Error()
+	}
+	return ""
+}
+
+var globValidationTestCases = []validationTestCase{
+	{
+		description: "Empty glob pattern",
+		data: TestForm{
+			GlobPattern: "",
+		},
+		expectedErrors: binding.Errors{},
+	},
+	{
+		description: "Valid glob",
+		data: TestForm{
+			GlobPattern: "{master,release*}",
+		},
+		expectedErrors: binding.Errors{},
+	},
+
+	{
+		description: "Invalid glob",
+		data: TestForm{
+			GlobPattern: "[a-",
+		},
+		expectedErrors: binding.Errors{
+			binding.Error{
+				FieldNames:     []string{"GlobPattern"},
+				Classification: ErrGlobPattern,
+				Message:        getGlobPatternErrorString("[a-"),
+			},
+		},
+	},
+}
+
+func Test_GlobPatternValidation(t *testing.T) {
+	AddBindingRules()
+
+	for _, testCase := range globValidationTestCases {
+		t.Run(testCase.description, func(t *testing.T) {
+			performValidationTest(t, testCase)
+		})
+	}
+}

options/locale/locale_en-US.ini

@@ -300,6 +300,7 @@ max_size_error = ` must contain at most %s characters.`
 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.`
 unknown_error = Unknown error:
 captcha_incorrect = The CAPTCHA code is incorrect.
 password_not_match = The passwords do not match.
@@ -1258,6 +1259,8 @@ settings.event_pull_request = Pull Request
 settings.event_pull_request_desc = Pull request opened, closed, reopened, edited, approved, rejected, review comment, assigned, unassigned, label updated, label cleared or synchronized.
 settings.event_push = Push
 settings.event_push_desc = Git push to a repository.
+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 <code>*</code>, events for all branches are reported. See <a href="https://godoc.org/github.com/gobwas/glob#Compile">github.com/gobwas/glob</a> documentation for syntax. Examples: <code>master</code>, <code>{master,release*}</code>.
 settings.event_repository = Repository
 settings.event_repository_desc = Repository created or deleted.
 settings.active = Active

routers/api/v1/utils/hook.go

@@ -112,6 +112,7 @@ func addHook(ctx *context.APIContext, form *api.CreateHookOption, orgID, repoID
 				Repository:   com.IsSliceContainsStr(form.Events, string(models.HookEventRepository)),
 				Release:      com.IsSliceContainsStr(form.Events, string(models.HookEventRelease)),
 			},
+			BranchFilter: form.BranchFilter,
 		},
 		IsActive:     form.Active,
 		HookTaskType: models.ToHookTaskType(form.Type),
@@ -236,6 +237,7 @@ func editHook(ctx *context.APIContext, form *api.EditHookOption, w *models.Webho
 	w.PullRequest = com.IsSliceContainsStr(form.Events, string(models.HookEventPullRequest))
 	w.Repository = com.IsSliceContainsStr(form.Events, string(models.HookEventRepository))
 	w.Release = com.IsSliceContainsStr(form.Events, string(models.HookEventRelease))
+	w.BranchFilter = form.BranchFilter
 
 	if err := w.UpdateEvent(); err != nil {
 		ctx.Error(500, "UpdateEvent", err)