From 401ec9efca5f456cfa47077bf5be73f545260bca Mon Sep 17 00:00:00 2001 From: Kausthubh J Rao <105716675+Exgene@users.noreply.github.com> Date: Thu, 2 Oct 2025 15:37:13 +0530 Subject: [PATCH 1/8] fix(webhook): prevent tag events from bypassing branch filters --- services/webhook/webhook.go | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/services/webhook/webhook.go b/services/webhook/webhook.go index 2b276207a1396..4210cf88a4e40 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) string { switch pp := p.(type) { case *api.CreatePayload: - if pp.RefType == "branch" { - return pp.Ref + switch pp.RefType { + case "branch": + return git.BranchPrefix + pp.Ref + case "tag": + return git.TagPrefix + pp.Ref } case *api.DeletePayload: - if pp.RefType == "branch" { - return pp.Ref + switch pp.RefType { + case "branch": + return git.BranchPrefix + pp.Ref + case "tag": + return git.TagPrefix + pp.Ref } case *api.PushPayload: - if strings.HasPrefix(pp.Ref, git.BranchPrefix) { - return pp.Ref[len(git.BranchPrefix):] - } + return pp.Ref } return "" } @@ -144,11 +147,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) + // Apply the filter directly to the ref name + if ref := getPayloadRef(p); ref != "" { + if !checkBranch(w, ref) { + log.Info("Ref %q doesn't match branch filter %q, skipping", ref, w.BranchFilter) return nil } } From 4d0c02bcbb2ff4635a0b7398345ecfec16c5c6cd Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 2 Oct 2025 18:49:53 +0800 Subject: [PATCH 2/8] use git.RefName --- services/webhook/webhook.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/services/webhook/webhook.go b/services/webhook/webhook.go index 4210cf88a4e40..73ebd62fddc90 100644 --- a/services/webhook/webhook.go +++ b/services/webhook/webhook.go @@ -46,24 +46,24 @@ func IsValidHookTaskType(name string) bool { var hookQueue *queue.WorkerPoolQueue[int64] // getPayloadRef returns the full ref name for hook event, if applicable. -func getPayloadRef(p api.Payloader) string { +func getPayloadRef(p api.Payloader) git.RefName { switch pp := p.(type) { case *api.CreatePayload: switch pp.RefType { case "branch": - return git.BranchPrefix + pp.Ref + return git.RefNameFromBranch(pp.Ref) case "tag": - return git.TagPrefix + pp.Ref + return git.RefNameFromTag(pp.Ref) } case *api.DeletePayload: switch pp.RefType { case "branch": - return git.BranchPrefix + pp.Ref + return git.RefNameFromBranch(pp.Ref) case "tag": - return git.TagPrefix + pp.Ref + return git.RefNameFromTag(pp.Ref) } case *api.PushPayload: - return pp.Ref + return git.RefName(pp.Ref) } return "" } @@ -149,6 +149,8 @@ func PrepareWebhook(ctx context.Context, w *webhook_model.Webhook, event webhook // Apply the filter directly to the ref name if ref := getPayloadRef(p); ref != "" { + // FIXME: here comes the problem, "ref" is the full ref name, but the filter + // But, "checkBranch" check it against "w.BranchFilter", does it make sense? if !checkBranch(w, ref) { log.Info("Ref %q doesn't match branch filter %q, skipping", ref, w.BranchFilter) return nil From b9484cfd944a01f44a51b37d93c045de8a286d19 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 2 Oct 2025 19:52:01 +0800 Subject: [PATCH 3/8] fix --- options/locale/locale_en-US.ini | 4 ++- services/webhook/webhook.go | 18 ++++++------ services/webhook/webhook_test.go | 28 +++++++++++++++++++ templates/repo/settings/webhook/settings.tmpl | 11 +++++++- web_src/css/base.css | 16 +++++++---- web_src/css/form.css | 1 + 6 files changed, 62 insertions(+), 16 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 73ebd62fddc90..cfabf8919f296 100644 --- a/services/webhook/webhook.go +++ b/services/webhook/webhook.go @@ -111,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) 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. @@ -147,12 +150,9 @@ func PrepareWebhook(ctx context.Context, w *webhook_model.Webhook, event webhook return nil } - // Apply the filter directly to the ref name + // Check the payload's git ref against the webhook's branch filter, if any. if ref := getPayloadRef(p); ref != "" { - // FIXME: here comes the problem, "ref" is the full ref name, but the filter - // But, "checkBranch" check it against "w.BranchFilter", does it make sense? - if !checkBranch(w, ref) { - log.Info("Ref %q doesn't match branch filter %q, skipping", ref, w.BranchFilter) + 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..0f5658b6171f8 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..e92030f2baeb2 100644 --- a/web_src/css/base.css +++ b/web_src/css/base.css @@ -101,6 +101,12 @@ samp, font-size: 0.95em; /* compensate for monospace fonts being usually slightly larger */ } +code { + padding: 2px 4px; + border-radius: .24em; + background-color: var(--color-label-bg); +} + b, strong, h1, @@ -177,6 +183,11 @@ table { border-collapse: collapse; } +ul { + margin: 0.5em 0; + padding: 0 1.5em; +} + button { cursor: pointer; } @@ -603,11 +614,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..2c9cad85b4235 100644 --- a/web_src/css/form.css +++ b/web_src/css/form.css @@ -218,6 +218,7 @@ textarea:focus, .form .help { color: var(--color-secondary-dark-5); + margin-top: 0.25em; padding-bottom: 0.6em; display: inline-block; text-wrap: balance; From d09fd4ea56eb73007eac078f68ec2039b23554da Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 2 Oct 2025 19:58:31 +0800 Subject: [PATCH 4/8] fix ui --- templates/repo/settings/webhook/settings.tmpl | 10 +++++----- web_src/css/base.css | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/templates/repo/settings/webhook/settings.tmpl b/templates/repo/settings/webhook/settings.tmpl index 0f5658b6171f8..1f71b9cfab7c4 100644 --- a/templates/repo/settings/webhook/settings.tmpl +++ b/templates/repo/settings/webhook/settings.tmpl @@ -48,11 +48,11 @@ {{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/*}
    • -
+
    +
  • main
    • +
  • {main,feature/*}
    • +
  • {refs/heads/feature/*,refs/tags/release/*}
    • +
diff --git a/web_src/css/base.css b/web_src/css/base.css index e92030f2baeb2..50725b747e662 100644 --- a/web_src/css/base.css +++ b/web_src/css/base.css @@ -102,7 +102,7 @@ samp, } code { - padding: 2px 4px; + padding: 1px 4px; border-radius: .24em; background-color: var(--color-label-bg); } From 7fbaf6ea5834381d44fb283059d3cfa8f282a42b Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 2 Oct 2025 20:01:06 +0800 Subject: [PATCH 5/8] remove unnecessary style --- web_src/css/repo/issue-list.css | 7 ------- 1 file changed, 7 deletions(-) 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; -} From d1373a75e6216c65ba407d2a448f13f5ed8484f9 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 2 Oct 2025 22:07:54 +0800 Subject: [PATCH 6/8] fine tune --- services/webhook/webhook.go | 5 +++-- web_src/css/base.css | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/services/webhook/webhook.go b/services/webhook/webhook.go index cfabf8919f296..a35997dda093a 100644 --- a/services/webhook/webhook.go +++ b/services/webhook/webhook.go @@ -119,7 +119,7 @@ func checkBranchFilter(branchFilter string, ref git.RefName) bool { 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 } @@ -150,8 +150,9 @@ func PrepareWebhook(ctx context.Context, w *webhook_model.Webhook, event webhook return nil } - // Check the payload's git ref against the webhook's branch filter, if any. + // 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/web_src/css/base.css b/web_src/css/base.css index 50725b747e662..f0bbc20c08e84 100644 --- a/web_src/css/base.css +++ b/web_src/css/base.css @@ -103,7 +103,7 @@ samp, code { padding: 1px 4px; - border-radius: .24em; + border-radius: var(--border-radius); background-color: var(--color-label-bg); } @@ -185,7 +185,7 @@ table { ul { margin: 0.5em 0; - padding: 0 1.5em; + padding: 0 0 0 1.5em; } button { From e9ac10657929cd6741063811216dc32f2c479daa Mon Sep 17 00:00:00 2001 From: silverwind Date: Thu, 2 Oct 2025 17:05:55 +0200 Subject: [PATCH 7/8] add label color to code --- web_src/css/base.css | 1 + 1 file changed, 1 insertion(+) diff --git a/web_src/css/base.css b/web_src/css/base.css index f0bbc20c08e84..9cef92019d08b 100644 --- a/web_src/css/base.css +++ b/web_src/css/base.css @@ -105,6 +105,7 @@ code { padding: 1px 4px; border-radius: var(--border-radius); background-color: var(--color-label-bg); + color: var(--color-label-text); } b, From 09ad009c8e4d8f81bde039d43e8b856599076c15 Mon Sep 17 00:00:00 2001 From: silverwind Date: Thu, 2 Oct 2025 17:47:54 +0200 Subject: [PATCH 8/8] override color for help --- web_src/css/form.css | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/web_src/css/form.css b/web_src/css/form.css index 2c9cad85b4235..757edf72974b9 100644 --- a/web_src/css/form.css +++ b/web_src/css/form.css @@ -224,6 +224,10 @@ textarea:focus, text-wrap: balance; } +.form .help code { + color: var(--color-text-light-1); +} + .m-captcha-style { width: 100%; height: 5em;