From b6904c97302db0e05d94cb0231cd790a0d2e7af2 Mon Sep 17 00:00:00 2001 From: Harsh Satyajit Thakur Date: Wed, 15 Jul 2026 04:24:18 +1000 Subject: [PATCH] fix: make "test push webhook" always work (#38425) * fix #38309 * fix #26238 * fix #37886 --------- Co-authored-by: wxiaoguang --- options/locale/locale_en-US.json | 1 - routers/api/v1/repo/hook.go | 2 +- routers/web/repo/setting/webhook.go | 27 ++++++--------- services/webhook/webhook.go | 27 +++++++++++++++ services/webhook/webhook_test.go | 35 ++++++++++++++++++++ templates/repo/settings/webhook/history.tmpl | 18 ++++------ 6 files changed, 81 insertions(+), 29 deletions(-) diff --git a/options/locale/locale_en-US.json b/options/locale/locale_en-US.json index 9373aa83a9..91e6694076 100644 --- a/options/locale/locale_en-US.json +++ b/options/locale/locale_en-US.json @@ -2251,7 +2251,6 @@ "repo.settings.webhook_deletion_success": "The webhook has been removed.", "repo.settings.webhook.test_delivery": "Test Push Event", "repo.settings.webhook.test_delivery_desc": "Test this webhook with a fake push event.", - "repo.settings.webhook.test_delivery_desc_disabled": "To test this webhook with a fake event, activate it.", "repo.settings.webhook.request": "Request", "repo.settings.webhook.response": "Response", "repo.settings.webhook.headers": "Headers", diff --git a/routers/api/v1/repo/hook.go b/routers/api/v1/repo/hook.go index c247fc0039..b3154696b7 100644 --- a/routers/api/v1/repo/hook.go +++ b/routers/api/v1/repo/hook.go @@ -177,7 +177,7 @@ func TestHook(ctx *context.APIContext) { commit := convert.ToPayloadCommit(ctx, ctx.Repo.Repository, ctx.Repo.Commit) commitID := ctx.Repo.Commit.ID.String() - if err := webhook_service.PrepareWebhook(ctx, hook, webhook_module.HookEventPush, &api.PushPayload{ + if err := webhook_service.PrepareTestWebhook(ctx, hook, webhook_module.HookEventPush, &api.PushPayload{ Ref: ref, Before: commitID, After: commitID, diff --git a/routers/web/repo/setting/webhook.go b/routers/web/repo/setting/webhook.go index 4d3120618f..f128209f1b 100644 --- a/routers/web/repo/setting/webhook.go +++ b/routers/web/repo/setting/webhook.go @@ -664,19 +664,14 @@ func TestWebhook(ctx *context.Context) { return } - // Grab latest commit or fake one if it's empty repository. - // Note: in old code, the "ctx.Repo.Commit" is the last commit of the default branch. - // New code doesn't set that commit, so it always uses the fake commit to test webhook. - commit := ctx.Repo.Commit - if commit == nil { - ghost := user_model.NewGhostUser() - objectFormat := git.ObjectFormatFromName(ctx.Repo.Repository.ObjectFormatName) - commit = &git.Commit{ - ID: objectFormat.EmptyObjectID(), - Author: ghost.NewGitSig(), - Committer: ghost.NewGitSig(), - CommitMessage: git.CommitMessage{MessageRaw: "This is a fake commit"}, - } + // use a fake commit to test webhook + ghostUser := user_model.NewGhostUser() + objectFormat := git.ObjectFormatFromName(ctx.Repo.Repository.ObjectFormatName) + commit := &git.Commit{ + ID: objectFormat.EmptyObjectID(), + Author: ghostUser.NewGitSig(), + Committer: ghostUser.NewGitSig(), + CommitMessage: git.CommitMessage{MessageRaw: "This is a fake commit for webhook push test"}, } apiUser := convert.ToUserWithAccessMode(ctx, ctx.Doer, perm.AccessModeNone) @@ -697,7 +692,7 @@ func TestWebhook(ctx *context.Context) { commitID := commit.ID.String() p := &api.PushPayload{ - Ref: git.BranchPrefix + ctx.Repo.Repository.DefaultBranch, + Ref: git.RefNameFromBranch(ctx.Repo.Repository.DefaultBranch).String(), Before: commitID, After: commitID, CompareURL: setting.AppURL + ctx.Repo.Repository.ComposeCompareURL(commitID, commitID), @@ -708,8 +703,8 @@ func TestWebhook(ctx *context.Context) { Pusher: apiUser, Sender: apiUser, } - if err := webhook_service.PrepareWebhook(ctx, w, webhook_module.HookEventPush, p); err != nil { - ctx.Flash.Error("PrepareWebhook: " + err.Error()) + if err := webhook_service.PrepareTestWebhook(ctx, w, webhook_module.HookEventPush, p); err != nil { + ctx.Flash.Error("PrepareTestWebhook: " + err.Error()) ctx.Status(http.StatusInternalServerError) } else { ctx.Flash.Info(ctx.Tr("repo.settings.webhook.delivery.success")) diff --git a/services/webhook/webhook.go b/services/webhook/webhook.go index 5e8107772a..0a53f289aa 100644 --- a/services/webhook/webhook.go +++ b/services/webhook/webhook.go @@ -129,6 +129,33 @@ func checkBranchFilter(branchFilter string, ref git.RefName) bool { return g.Match(ref.String()) } +// PrepareTestWebhook always creates and enqueues a hook task for manual testing. +// Unlike PrepareWebhook, it ignores event subscriptions and branch filters so the +// Test Push Event control can verify delivery even when those gates would suppress +// a real event. +func PrepareTestWebhook(ctx context.Context, w *webhook_model.Webhook, event webhook_module.HookEventType, p api.Payloader) error { + if setting.DisableWebhooks { + return nil + } + + payload, err := p.JSONPayload() + if err != nil { + return fmt.Errorf("JSONPayload for %s: %w", event, err) + } + + task, err := webhook_model.CreateHookTask(ctx, &webhook_model.HookTask{ + HookID: w.ID, + PayloadContent: string(payload), + EventType: event, + PayloadVersion: 2, + }) + if err != nil { + return fmt.Errorf("CreateHookTask for %s: %w", event, err) + } + + return enqueueHookTask(task.ID) +} + // PrepareWebhook creates a hook task and enqueues it for processing. // The payload is saved as-is. The adjustments depending on the webhook type happen // right before delivery, in the [Deliver] method. diff --git a/services/webhook/webhook_test.go b/services/webhook/webhook_test.go index f20510c9a6..6ecf24337c 100644 --- a/services/webhook/webhook_test.go +++ b/services/webhook/webhook_test.go @@ -30,6 +30,7 @@ func TestWebhookService(t *testing.T) { t.Run("PrepareBranchFilterNoMatch", testWebhookPrepareBranchFilterNoMatch) t.Run("WebhookUserMail", testWebhookUserMail) t.Run("CheckBranchFilter", testWebhookCheckBranchFilter) + t.Run("PrepareTestWebhookIgnoresGates", testPrepareTestWebhookIgnoresGates) } func testWebhookGetSlackHook(t *testing.T) { @@ -132,3 +133,37 @@ func testWebhookCheckBranchFilter(t *testing.T) { assert.Equal(t, v.match, checkBranchFilter(v.filter, v.ref), "filter: %q ref: %q", v.filter, v.ref) } } + +func testPrepareTestWebhookIgnoresGates(t *testing.T) { + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + hook := &webhook_model.Webhook{ + RepoID: repo.ID, + URL: "http://localhost/gitea-webhook-test-prepare_test_webhook", + ContentType: webhook_model.ContentTypeJSON, + IsActive: true, + HookEvent: &webhook_module.HookEvent{ + ChooseEvents: true, + BranchFilter: "dev", + HookEvents: webhook_module.HookEvents{ + webhook_module.HookEventWorkflowRun: true, + }, + }, + } + require.NoError(t, hook.UpdateEvent()) + require.NoError(t, db.Insert(t.Context(), hook)) + + payload := &api.PushPayload{ + Ref: "refs/heads/master", + Commits: []*api.PayloadCommit{{}}, + } + hookTask := &webhook_model.HookTask{HookID: hook.ID, EventType: webhook_module.HookEventPush} + + // Real deliveries stay gated: no push event + branch filter mismatch => nothing queued. + unittest.AssertNotExistsBean(t, hookTask) + require.NoError(t, PrepareWebhook(t.Context(), hook, webhook_module.HookEventPush, payload)) + unittest.AssertNotExistsBean(t, hookTask) + + // Manual test delivery always queues so the endpoint can be verified. + require.NoError(t, PrepareTestWebhook(t.Context(), hook, webhook_module.HookEventPush, payload)) + unittest.AssertExistsAndLoadBean(t, hookTask) +} diff --git a/templates/repo/settings/webhook/history.tmpl b/templates/repo/settings/webhook/history.tmpl index d57913042b..7762fca85f 100644 --- a/templates/repo/settings/webhook/history.tmpl +++ b/templates/repo/settings/webhook/history.tmpl @@ -1,16 +1,12 @@ -{{$isNew:=or .PageIsSettingsHooksNew .PageIsAdminDefaultHooksNew .PageIsAdminSystemHooksNew}} {{if .PageIsSettingsHooksEdit}} -

- {{ctx.Locale.Tr "repo.settings.recent_deliveries"}} +

+ {{ctx.Locale.Tr "repo.settings.recent_deliveries"}} {{if .Permission.IsAdmin}} -
- - - - -
+ {{end}}