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 <wxiaoguang@gmail.com>
Co-authored-by: silverwind <me@silverwind.io>
This commit is contained in:
Kausthubh J Rao
2025-10-03 12:21:57 +05:30
committed by GitHub
parent efc48c36ff
commit c4532101a4
7 changed files with 83 additions and 34 deletions

View File

@@ -2434,7 +2434,9 @@ settings.event_workflow_job_desc = Gitea Actions Workflow job queued, waiting, i
settings.event_package = Package settings.event_package = Package
settings.event_package_desc = Package created or deleted in a repository. settings.event_package_desc = Package created or deleted in a repository.
settings.branch_filter = Branch filter 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="%[1]s">%[2]s</a> documentation for syntax. Examples: <code>master</code>, <code>{master,release*}</code>. 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 <code>*</code>, events for all branches and tags are reported.
settings.branch_filter_desc_2 = Use <code>refs/heads/</code> or <code>refs/tags/</code> prefix to match full ref names.
settings.branch_filter_desc_doc = See <a href="%[1]s">%[2]s</a> documentation for syntax.
settings.authorization_header = Authorization Header settings.authorization_header = Authorization Header
settings.authorization_header_desc = Will be included as authorization header for requests when present. Examples: %s. settings.authorization_header_desc = Will be included as authorization header for requests when present. Examples: %s.
settings.active = Active settings.active = Active

View File

@@ -8,7 +8,6 @@ import (
"errors" "errors"
"fmt" "fmt"
"net/http" "net/http"
"strings"
"code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/db"
repo_model "code.gitea.io/gitea/models/repo" 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 // hookQueue is a global queue of web hooks
var hookQueue *queue.WorkerPoolQueue[int64] var hookQueue *queue.WorkerPoolQueue[int64]
// getPayloadBranch returns branch for hook event, if applicable. // getPayloadRef returns the full ref name for hook event, if applicable.
func getPayloadBranch(p api.Payloader) string { func getPayloadRef(p api.Payloader) git.RefName {
switch pp := p.(type) { switch pp := p.(type) {
case *api.CreatePayload: case *api.CreatePayload:
if pp.RefType == "branch" { switch pp.RefType {
return pp.Ref case "branch":
return git.RefNameFromBranch(pp.Ref)
case "tag":
return git.RefNameFromTag(pp.Ref)
} }
case *api.DeletePayload: case *api.DeletePayload:
if pp.RefType == "branch" { switch pp.RefType {
return pp.Ref case "branch":
return git.RefNameFromBranch(pp.Ref)
case "tag":
return git.RefNameFromTag(pp.Ref)
} }
case *api.PushPayload: case *api.PushPayload:
if strings.HasPrefix(pp.Ref, git.BranchPrefix) { return git.RefName(pp.Ref)
return pp.Ref[len(git.BranchPrefix):]
}
} }
return "" return ""
} }
@@ -108,19 +111,22 @@ func enqueueHookTask(taskID int64) error {
return nil return nil
} }
func checkBranch(w *webhook_model.Webhook, branch string) bool { func checkBranchFilter(branchFilter string, ref git.RefName) bool {
if w.BranchFilter == "" || w.BranchFilter == "*" { if branchFilter == "" || branchFilter == "*" || branchFilter == "**" {
return true return true
} }
g, err := glob.Compile(w.BranchFilter) g, err := glob.Compile(branchFilter)
if err != nil { if err != nil {
// should not really happen as BranchFilter is validated // 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 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. // 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 return nil
} }
// If payload has no associated branch (e.g. it's a new tag, issue, etc.), // If payload has no associated branch (e.g. it's a new tag, issue, etc.), branch filter has no effect.
// branch filter has no effect. if ref := getPayloadRef(p); ref != "" {
if branch := getPayloadBranch(p); branch != "" { // Check the payload's git ref against the webhook's branch filter.
if !checkBranch(w, branch) { if !checkBranchFilter(w.BranchFilter, ref) {
log.Info("Branch %q doesn't match branch filter %q, skipping", branch, w.BranchFilter)
return nil return nil
} }
} }

View File

@@ -10,6 +10,7 @@ import (
"code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user" user_model "code.gitea.io/gitea/models/user"
webhook_model "code.gitea.io/gitea/models/webhook" webhook_model "code.gitea.io/gitea/models/webhook"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
api "code.gitea.io/gitea/modules/structs" api "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/test" "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.GetPlaceholderEmail(), convert.ToUser(t.Context(), user, nil).Email)
assert.Equal(t, user.Email, convert.ToUser(t.Context(), user, user).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)
}
}

View File

@@ -44,7 +44,16 @@
<div class="field"> <div class="field">
<label>{{ctx.Locale.Tr "repo.settings.branch_filter"}}</label> <label>{{ctx.Locale.Tr "repo.settings.branch_filter"}}</label>
<input name="branch_filter" type="text" value="{{or .Webhook.BranchFilter "*"}}"> <input name="branch_filter" type="text" value="{{or .Webhook.BranchFilter "*"}}">
<span class="help">{{ctx.Locale.Tr "repo.settings.branch_filter_desc" "https://pkg.go.dev/github.com/gobwas/glob#Compile" "github.com/gobwas/glob"}}</span> <span class="help">
{{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"}}
<ul>
<li><code>main</code></li>
<li><code>{main,feature/*}</code></li>
<li><code>{refs/heads/feature/*,refs/tags/release/*}</code></li>
</ul>
</span>
</div> </div>
<div class="field"> <div class="field">

View File

@@ -101,6 +101,13 @@ samp,
font-size: 0.95em; /* compensate for monospace fonts being usually slightly larger */ 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, b,
strong, strong,
h1, h1,
@@ -177,6 +184,11 @@ table {
border-collapse: collapse; border-collapse: collapse;
} }
ul {
margin: 0.5em 0;
padding: 0 0 0 1.5em;
}
button { button {
cursor: pointer; cursor: pointer;
} }
@@ -603,11 +615,6 @@ img.ui.avatar,
text-align: center; text-align: center;
} }
.ui .message > ul {
margin: 0;
padding: 0 1em;
}
.ui .header > i + .content { .ui .header > i + .content {
padding-left: 0.75rem; padding-left: 0.75rem;
vertical-align: middle; vertical-align: middle;

View File

@@ -218,11 +218,16 @@ textarea:focus,
.form .help { .form .help {
color: var(--color-secondary-dark-5); color: var(--color-secondary-dark-5);
margin-top: 0.25em;
padding-bottom: 0.6em; padding-bottom: 0.6em;
display: inline-block; display: inline-block;
text-wrap: balance; text-wrap: balance;
} }
.form .help code {
color: var(--color-text-light-1);
}
.m-captcha-style { .m-captcha-style {
width: 100%; width: 100%;
height: 5em; height: 5em;

View File

@@ -84,10 +84,3 @@
margin-right: 8px; margin-right: 8px;
text-align: left; 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;
}