fix(webhook): prevent tag events from bypassing branch filters targets (#35567) (#35577)

Backport #35567 by Exgene

Co-authored-by: Kausthubh J Rao <105716675+Exgene@users.noreply.github.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
Co-authored-by: silverwind <me@silverwind.io>
This commit is contained in:
Giteabot
2025-10-03 17:49:16 +00:00
committed by GitHub
co-authored by Kausthubh J Rao wxiaoguang silverwind
parent 6c8879b832
commit d94faf6d7e
7 changed files with 83 additions and 34 deletions
+25 -20
View File
@@ -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
}
}
+28
View File
@@ -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)
}
}