From 7cd5748c1d6e6dffc87697f10c035d2a6aba73c7 Mon Sep 17 00:00:00 2001 From: WGH Date: Thu, 8 Aug 2019 03:51:45 +0300 Subject: [PATCH] Implement webhook branch filter See #2025, #3998. --- models/fixtures/webhook.yml | 7 +++ models/webhook.go | 51 +++++++++++++++- models/webhook_test.go | 33 +++++++++++ modules/auth/auth.go | 2 + modules/auth/repo_form.go | 1 + modules/structs/hook.go | 12 ++-- modules/validation/binding.go | 24 ++++++++ modules/validation/binding_test.go | 1 + modules/validation/regexp_test.go | 59 +++++++++++++++++++ options/locale/locale_en-US.ini | 3 + routers/api/v1/utils/hook.go | 2 + routers/repo/webhook.go | 1 + templates/repo/settings/webhook/settings.tmpl | 7 +++ templates/swagger/v1_json.tmpl | 8 +++ 14 files changed, 203 insertions(+), 8 deletions(-) create mode 100644 modules/validation/regexp_test.go diff --git a/models/fixtures/webhook.yml b/models/fixtures/webhook.yml index 11d7439cf4a8..02fddaa8fda5 100644 --- a/models/fixtures/webhook.yml +++ b/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|release)"}' + is_active: true diff --git a/models/webhook.go b/models/webhook.go index e3e11e59633f..261a40a4419d 100644 --- a/models/webhook.go +++ b/models/webhook.go @@ -16,9 +16,11 @@ import ( "net" "net/http" "net/url" + "regexp" "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" @@ -83,9 +85,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"` } @@ -255,6 +258,20 @@ func (w *Webhook) EventsArray() []string { return events } +func (w *Webhook) checkBranch(branch string) bool { + if w.BranchFilter == "" { + return true + } + pattern := "^" + w.BranchFilter + "$" + matched, err := regexp.MatchString(pattern, branch) + if err != nil { + // should not really happen as BranchFilter is validated + log.Error("CheckBranch failed: %s", err) + return false + } + return matched +} + // CreateWebhook creates a new web hook. func CreateWebhook(w *Webhook) error { return createWebhook(x, w) @@ -650,6 +667,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, bool) { + switch pp := p.(type) { + case *api.CreatePayload: + if pp.RefType == "branch" { + return pp.Ref, true + } + case *api.DeletePayload: + if pp.RefType == "branch" { + return pp.Ref, true + } + case *api.PushPayload: + if strings.HasPrefix(pp.Ref, git.BranchPrefix) { + return pp.Ref[len(git.BranchPrefix):], true + } + } + return "", false +} + func prepareWebhook(e Engine, w *Webhook, repo *Repository, event HookEventType, p api.Payloader) error { for _, e := range w.eventCheckers() { if event == e.typ { @@ -659,6 +695,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, ok := getPayloadBranch(p); ok { + 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. diff --git a/models/webhook_test.go b/models/webhook_test.go index d2fb3ea6ed2d..8b5a6a33e849 100644 --- a/models/webhook_test.go +++ b/models/webhook_test.go @@ -270,6 +270,39 @@ 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) + } + assert.NoError(t, PrepareWebhooks(repo, HookEventPush, &api.PushPayload{Ref: "refs/heads/master"})) + 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 diff --git a/modules/auth/auth.go b/modules/auth/auth.go index 95871909bb7d..7bc0bd539845 100644 --- a/modules/auth/auth.go +++ b/modules/auth/auth.go @@ -352,6 +352,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.ErrRegexp: + data["ErrorMsg"] = trName + l.Tr("form.regexp_error", errs[0].Message) default: data["ErrorMsg"] = l.Tr("form.unknown_error") + " " + errs[0].Classification } diff --git a/modules/auth/repo_form.go b/modules/auth/repo_form.go index 0333c3c92614..50f935d23f2d 100644 --- a/modules/auth/repo_form.go +++ b/modules/auth/repo_form.go @@ -182,6 +182,7 @@ type WebhookForm struct { PullRequest bool Repository bool Active bool + BranchFilter string `binding:"Regexp"` } // PushOnly if the hook will be triggered when push diff --git a/modules/structs/hook.go b/modules/structs/hook.go index 8dae578ec645..cefdd8d7e720 100644 --- a/modules/structs/hook.go +++ b/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:"Regexp"` // 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:"Regexp"` + Active *bool `json:"active"` } // Payloader payload is some part of one hook diff --git a/modules/validation/binding.go b/modules/validation/binding.go index 03b6f6276df2..593e7ed0b873 100644 --- a/modules/validation/binding.go +++ b/modules/validation/binding.go @@ -15,6 +15,9 @@ import ( const ( // ErrGitRefName is git reference name error ErrGitRefName = "GitRefNameError" + + // ErrRegexp is returned when regexp is invalid + ErrRegexp = "RegexpErr" ) var ( @@ -28,6 +31,7 @@ var ( func AddBindingRules() { addGitRefNameBindingRule() addValidURLBindingRule() + addRegexpRule() } func addGitRefNameBindingRule() { @@ -82,6 +86,26 @@ func addValidURLBindingRule() { }) } +func addRegexpRule() { + binding.AddRule(&binding.Rule{ + IsMatch: func(rule string) bool { + return strings.HasPrefix(rule, "Regexp") + }, + IsValid: func(errs binding.Errors, name string, val interface{}) (bool, binding.Errors) { + str := fmt.Sprintf("%v", val) + + if str != "" { + if _, err := regexp.Compile(str); err != nil { + errs.Add([]string{name}, ErrRegexp, err.Error()) + return false, errs + } + } + + return true, errs + }, + }) +} + func portOnly(hostport string) string { colon := strings.IndexByte(hostport, ':') if colon == -1 { diff --git a/modules/validation/binding_test.go b/modules/validation/binding_test.go index f55b09266dd2..4276969250cc 100644 --- a/modules/validation/binding_test.go +++ b/modules/validation/binding_test.go @@ -29,6 +29,7 @@ type ( TestForm struct { BranchName string `form:"BranchName" binding:"GitRefName"` URL string `form:"ValidUrl" binding:"ValidUrl"` + Regexp string `form:"Regexp" binding:"Regexp"` } ) diff --git a/modules/validation/regexp_test.go b/modules/validation/regexp_test.go new file mode 100644 index 000000000000..9a6f841d141c --- /dev/null +++ b/modules/validation/regexp_test.go @@ -0,0 +1,59 @@ +package validation + +import ( + "regexp" + "testing" + + "github.com/go-macaron/binding" +) + +func getRegexpErrorString(pattern string) string { + // It would be unwise to rely on that regexp + // compilation errors don't ever change across Go releases. + _, err := regexp.Compile(pattern) + if err != nil { + return err.Error() + } + return "" +} + +var regexpValidationTestCases = []validationTestCase{ + { + description: "Empty regexp", + data: TestForm{ + Regexp: "", + }, + expectedErrors: binding.Errors{}, + }, + { + description: "Valid regexp", + data: TestForm{ + Regexp: "(master|release)", + }, + expectedErrors: binding.Errors{}, + }, + + { + description: "Invalid regexp", + data: TestForm{ + Regexp: "master)(", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"Regexp"}, + Classification: ErrRegexp, + Message: getRegexpErrorString("master)("), + }, + }, + }, +} + +func Test_RegexpValidation(t *testing.T) { + AddBindingRules() + + for _, testCase := range regexpValidationTestCases { + t.Run(testCase.description, func(t *testing.T) { + performValidationTest(t, testCase) + }) + } +} diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 0df7cdf7e0d0..aeb5a640ccd0 100644 --- a/options/locale/locale_en-US.ini +++ b/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'.` +regexp_error = ` regular expression is invalid: %s.` unknown_error = Unknown error: captcha_incorrect = The CAPTCHA code is incorrect. password_not_match = The passwords do not match. @@ -1245,6 +1246,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 regular expression. If empty, events for all branches are reported. Regular expression is implicitly anchored, that is, partial match is not allowed. Add .* explicitly, if necessary. settings.event_repository = Repository settings.event_repository_desc = Repository created or deleted. settings.active = Active diff --git a/routers/api/v1/utils/hook.go b/routers/api/v1/utils/hook.go index 92846c5f2a3f..2faa35cd1ec2 100644 --- a/routers/api/v1/utils/hook.go +++ b/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) diff --git a/routers/repo/webhook.go b/routers/repo/webhook.go index 20a3a45c18ae..5f2b9ea306a0 100644 --- a/routers/repo/webhook.go +++ b/routers/repo/webhook.go @@ -145,6 +145,7 @@ func ParseHookEvent(form auth.WebhookForm) *models.HookEvent { PullRequest: form.PullRequest, Repository: form.Repository, }, + BranchFilter: form.BranchFilter, } } diff --git a/templates/repo/settings/webhook/settings.tmpl b/templates/repo/settings/webhook/settings.tmpl index 3f9f739775db..a852c15cedde 100644 --- a/templates/repo/settings/webhook/settings.tmpl +++ b/templates/repo/settings/webhook/settings.tmpl @@ -116,6 +116,13 @@ + +
+ + + {{.i18n.Tr "repo.settings.branch_filter_desc" | Str2html}} +
+
diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 4ae7f5a49ebf..144b85c8d536 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -7277,6 +7277,10 @@ "default": false, "x-go-name": "Active" }, + "branch_filter": { + "type": "string", + "x-go-name": "BranchFilter" + }, "config": { "type": "object", "additionalProperties": { @@ -7868,6 +7872,10 @@ "type": "boolean", "x-go-name": "Active" }, + "branch_filter": { + "type": "string", + "x-go-name": "BranchFilter" + }, "config": { "type": "object", "additionalProperties": {