From aec2f51731a825ad938604d8ddb52543fe2d3bf6 Mon Sep 17 00:00:00 2001 From: Kausthubh J Rao <105716675+Exgene@users.noreply.github.com> Date: Fri, 3 Oct 2025 12:21:57 +0530 Subject: [PATCH] fix(webhook): prevent tag events from bypassing branch filters targets #35449 (#35567) Tag creation/deletion was triggering push webhooks even when branch filters were configured, causing unintended pipeline executions. This change modifies the branch filter logic to check the full ref name directly instead of first determining if it's a "branch" event. Fixes: Tag events now properly respect branch filters - Add getPayloadRef() function to extract full ref names - Update PrepareWebhook() to use direct ref matching - Prevents refs/tags/* from matching refs/heads/* filters Closes #35449 --------- Co-authored-by: wxiaoguang Co-authored-by: silverwind --- options/locale/locale_en-US.ini | 4 +- services/webhook/webhook.go | 45 ++++++++++--------- services/webhook/webhook_test.go | 28 ++++++++++++ templates/repo/settings/webhook/settings.tmpl | 11 ++++- web_src/css/base.css | 17 ++++--- web_src/css/form.css | 5 +++ web_src/css/repo/issue-list.css | 7 --- 7 files changed, 83 insertions(+), 34 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index b3eb7b1f4a4fd..be715fbf3c059 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -2433,7 +2433,9 @@ settings.event_workflow_job_desc = Gitea Actions Workflow job queued, waiting, i settings.event_package = Package settings.event_package_desc = Package created or deleted in 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 *, events for all branches are reported. See %[2]s documentation for syntax. Examples: master, {master,release*}. +settings.branch_filter_desc_1 = Branch (and ref name) allowlist for push, branch creation and branch deletion events, specified as glob pattern. If empty or *, events for all branches and tags are reported. +settings.branch_filter_desc_2 = Use refs/heads/ or refs/tags/ prefix to match full ref names. +settings.branch_filter_desc_doc = See %[2]s documentation for syntax. settings.authorization_header = Authorization Header settings.authorization_header_desc = Will be included as authorization header for requests when present. Examples: %s. settings.active = Active diff --git a/services/webhook/webhook.go b/services/webhook/webhook.go index 2b276207a1396..a35997dda093a 100644 --- a/services/webhook/webhook.go +++ b/services/webhook/webhook.go @@ -8,7 +8,6 @@ import ( "errors" "fmt" "net/http" - "strings" "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" @@ -46,21 +45,25 @@ func IsValidHookTaskType(name string) bool { // hookQueue is a global queue of web hooks var hookQueue *queue.WorkerPoolQueue[int64] -// getPayloadBranch returns branch for hook event, if applicable. -func getPayloadBranch(p api.Payloader) string { +// getPayloadRef returns the full ref name for hook event, if applicable. +func getPayloadRef(p api.Payloader) git.RefName { switch pp := p.(type) { case *api.CreatePayload: - if pp.RefType == "branch" { - return pp.Ref + switch pp.RefType { + case "branch": + return git.RefNameFromBranch(pp.Ref) + case "tag": + return git.RefNameFromTag(pp.Ref) } case *api.DeletePayload: - if pp.RefType == "branch" { - return pp.Ref + switch pp.RefType { + case "branch": + return git.RefNameFromBranch(pp.Ref) + case "tag": + return git.RefNameFromTag(pp.Ref) } case *api.PushPayload: - if strings.HasPrefix(pp.Ref, git.BranchPrefix) { - return pp.Ref[len(git.BranchPrefix):] - } + return git.RefName(pp.Ref) } return "" } @@ -108,19 +111,22 @@ func enqueueHookTask(taskID int64) error { return nil } -func checkBranch(w *webhook_model.Webhook, branch string) bool { - if w.BranchFilter == "" || w.BranchFilter == "*" { +func checkBranchFilter(branchFilter string, ref git.RefName) bool { + if branchFilter == "" || branchFilter == "*" || branchFilter == "**" { return true } - g, err := glob.Compile(w.BranchFilter) + g, err := glob.Compile(branchFilter) if err != nil { // should not really happen as BranchFilter is validated - log.Error("CheckBranch failed: %s", err) + log.Debug("checkBranchFilter failed to compile filer %q, err: %s", branchFilter, err) return false } - return g.Match(branch) + if ref.IsBranch() && g.Match(ref.BranchName()) { + return true + } + return g.Match(ref.String()) } // PrepareWebhook creates a hook task and enqueues it for processing. @@ -144,11 +150,10 @@ func PrepareWebhook(ctx context.Context, w *webhook_model.Webhook, event webhook return nil } - // 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 !checkBranch(w, branch) { - log.Info("Branch %q doesn't match branch filter %q, skipping", branch, w.BranchFilter) + // If payload has no associated branch (e.g. it's a new tag, issue, etc.), branch filter has no effect. + if ref := getPayloadRef(p); ref != "" { + // Check the payload's git ref against the webhook's branch filter. + if !checkBranchFilter(w.BranchFilter, ref) { return nil } } diff --git a/services/webhook/webhook_test.go b/services/webhook/webhook_test.go index 34bcfd3df9d9a..f4432cc3f1198 100644 --- a/services/webhook/webhook_test.go +++ b/services/webhook/webhook_test.go @@ -10,6 +10,7 @@ import ( "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" webhook_model "code.gitea.io/gitea/models/webhook" + "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/test" @@ -90,3 +91,30 @@ func TestWebhookUserMail(t *testing.T) { assert.Equal(t, user.GetPlaceholderEmail(), convert.ToUser(t.Context(), user, nil).Email) assert.Equal(t, user.Email, convert.ToUser(t.Context(), user, user).Email) } + +func TestCheckBranchFilter(t *testing.T) { + cases := []struct { + filter string + ref git.RefName + match bool + }{ + {"", "any-ref", true}, + {"*", "any-ref", true}, + {"**", "any-ref", true}, + + {"main", git.RefNameFromBranch("main"), true}, + {"main", git.RefNameFromTag("main"), false}, + + {"feature/*", git.RefNameFromBranch("feature"), false}, + {"feature/*", git.RefNameFromBranch("feature/foo"), true}, + {"feature/*", git.RefNameFromTag("feature/foo"), false}, + + {"{refs/heads/feature/*,refs/tags/release/*}", git.RefNameFromBranch("feature/foo"), true}, + {"{refs/heads/feature/*,refs/tags/release/*}", git.RefNameFromBranch("main"), false}, + {"{refs/heads/feature/*,refs/tags/release/*}", git.RefNameFromTag("release/bar"), true}, + {"{refs/heads/feature/*,refs/tags/release/*}", git.RefNameFromTag("dev"), false}, + } + for _, v := range cases { + assert.Equal(t, v.match, checkBranchFilter(v.filter, v.ref), "filter: %q ref: %q", v.filter, v.ref) + } +} diff --git a/templates/repo/settings/webhook/settings.tmpl b/templates/repo/settings/webhook/settings.tmpl index a8ad1d6c9e5cf..1f71b9cfab7c4 100644 --- a/templates/repo/settings/webhook/settings.tmpl +++ b/templates/repo/settings/webhook/settings.tmpl @@ -44,7 +44,16 @@
- {{ctx.Locale.Tr "repo.settings.branch_filter_desc" "https://pkg.go.dev/github.com/gobwas/glob#Compile" "github.com/gobwas/glob"}} + + {{ctx.Locale.Tr "repo.settings.branch_filter_desc_1"}} + {{ctx.Locale.Tr "repo.settings.branch_filter_desc_2"}} + {{ctx.Locale.Tr "repo.settings.branch_filter_desc_doc" "https://pkg.go.dev/github.com/gobwas/glob#Compile" "github.com/gobwas/glob"}} +
    +
  • main
  • +
  • {main,feature/*}
  • +
  • {refs/heads/feature/*,refs/tags/release/*}
  • +
+
diff --git a/web_src/css/base.css b/web_src/css/base.css index 6ff060f1f9e2f..9cef92019d08b 100644 --- a/web_src/css/base.css +++ b/web_src/css/base.css @@ -101,6 +101,13 @@ samp, font-size: 0.95em; /* compensate for monospace fonts being usually slightly larger */ } +code { + padding: 1px 4px; + border-radius: var(--border-radius); + background-color: var(--color-label-bg); + color: var(--color-label-text); +} + b, strong, h1, @@ -177,6 +184,11 @@ table { border-collapse: collapse; } +ul { + margin: 0.5em 0; + padding: 0 0 0 1.5em; +} + button { cursor: pointer; } @@ -603,11 +615,6 @@ img.ui.avatar, text-align: center; } -.ui .message > ul { - margin: 0; - padding: 0 1em; -} - .ui .header > i + .content { padding-left: 0.75rem; vertical-align: middle; diff --git a/web_src/css/form.css b/web_src/css/form.css index c51eba1bc9442..757edf72974b9 100644 --- a/web_src/css/form.css +++ b/web_src/css/form.css @@ -218,11 +218,16 @@ textarea:focus, .form .help { color: var(--color-secondary-dark-5); + margin-top: 0.25em; padding-bottom: 0.6em; display: inline-block; text-wrap: balance; } +.form .help code { + color: var(--color-text-light-1); +} + .m-captcha-style { width: 100%; height: 5em; diff --git a/web_src/css/repo/issue-list.css b/web_src/css/repo/issue-list.css index bf8ff00b7ef19..33888101b6d74 100644 --- a/web_src/css/repo/issue-list.css +++ b/web_src/css/repo/issue-list.css @@ -84,10 +84,3 @@ margin-right: 8px; text-align: left; } - -.label-filter-exclude-info code { - border: 1px solid var(--color-secondary); - border-radius: var(--border-radius); - padding: 1px 2px; - font-size: 11px; -}