From 12cfb4beb50255d6b0007ecb465e43daa90095ad Mon Sep 17 00:00:00 2001 From: Antonin Delpeuch Date: Tue, 27 Jan 2026 23:19:34 +0100 Subject: [PATCH] fix: allow test delivery for webhooks not enabled for push events (#11073) Webhooks not enabled for push events cannot be tested using the "Test delivery" button, because the built-in test payload corresponds to a push event and is therefore filtered out at delivery time if the webhook isn't configured to trigger for such events. This fixes it by delivering the payload for a push event regardless of the webhook's configuration. This has the downside of delivering a payload which isn't necessarily representative of what the webhook will deliver for real, but it would be a significant effort to implement test payloads for all possible event types. We leave this as a follow-up improvement. Fixes #7934. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11073 Reviewed-by: oliverpool Reviewed-by: Gusted Co-authored-by: Antonin Delpeuch Co-committed-by: Antonin Delpeuch --- routers/api/v1/repo/hook.go | 3 +-- routers/web/repo/setting/webhook.go | 2 +- services/webhook/webhook.go | 26 ++++++++++++++++++ services/webhook/webhook_test.go | 41 +++++++++++++++++++++++++++++ 4 files changed, 69 insertions(+), 3 deletions(-) diff --git a/routers/api/v1/repo/hook.go b/routers/api/v1/repo/hook.go index 5d277604b8..a0e28f0685 100644 --- a/routers/api/v1/repo/hook.go +++ b/routers/api/v1/repo/hook.go @@ -15,7 +15,6 @@ import ( "forgejo.org/modules/setting" api "forgejo.org/modules/structs" "forgejo.org/modules/web" - webhook_module "forgejo.org/modules/webhook" "forgejo.org/routers/api/v1/utils" "forgejo.org/services/context" "forgejo.org/services/convert" @@ -177,7 +176,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, &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 b2c0691d04..9bd4422556 100644 --- a/routers/web/repo/setting/webhook.go +++ b/routers/web/repo/setting/webhook.go @@ -436,7 +436,7 @@ func WebhookTest(ctx *context.Context) { Pusher: apiUser, Sender: apiUser, } - if err := webhook_service.PrepareWebhook(ctx, w, webhook_module.HookEventPush, p); err != nil { + if err := webhook_service.PrepareTestWebhook(ctx, w, p); err != nil { ctx.Flash.Error("PrepareWebhook: " + err.Error()) ctx.Status(http.StatusInternalServerError) } else { diff --git a/services/webhook/webhook.go b/services/webhook/webhook.go index 93a863ec49..f163971d46 100644 --- a/services/webhook/webhook.go +++ b/services/webhook/webhook.go @@ -209,6 +209,32 @@ func PrepareWebhook(ctx context.Context, w *webhook_model.Webhook, event webhook return enqueueHookTask(task.ID) } +// PrepareTestWebhook creates a hook task for test delivery and enqueues it for processing. +// It simulates a "push" event even though the webhook might not be enabled for that event. +func PrepareTestWebhook(ctx context.Context, w *webhook_model.Webhook, p api.Payloader) error { + // Skip sending if webhooks are disabled. + if setting.DisableWebhooks { + return errors.New("webhooks are globally disabled") + } + + payload, err := p.JSONPayload() + if err != nil { + return fmt.Errorf("JSONPayload for Test: %w", err) + } + + task, err := webhook_model.CreateHookTask(ctx, &webhook_model.HookTask{ + HookID: w.ID, + PayloadContent: string(payload), + EventType: webhook_module.HookEventPush, + PayloadVersion: 2, + }) + if err != nil { + return fmt.Errorf("CreateHookTask for Test: %w", err) + } + + return enqueueHookTask(task.ID) +} + // PrepareWebhooks adds new webhooks to task queue for given payload. func PrepareWebhooks(ctx context.Context, source EventSource, event webhook_module.HookEventType, p api.Payloader) error { owner := source.Owner diff --git a/services/webhook/webhook_test.go b/services/webhook/webhook_test.go index 15cb8f620c..40aa6c7f48 100644 --- a/services/webhook/webhook_test.go +++ b/services/webhook/webhook_test.go @@ -5,6 +5,8 @@ package webhook import ( "fmt" + "net/http" + "net/http/httptest" "testing" "forgejo.org/models/db" @@ -111,3 +113,42 @@ func TestWebhookUserMail(t *testing.T) { assert.Equal(t, user.GetPlaceholderEmail(), convert.ToUser(db.DefaultContext, user, nil).Email) assert.Equal(t, user.Email, convert.ToUser(db.DefaultContext, user, user).Email) } + +func TestDeliverTestPayloadWithoutPushEvent(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + done := make(chan struct{}, 1) + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "/webhook", r.URL.Path) + w.WriteHeader(200) + done <- struct{}{} + })) + t.Cleanup(s.Close) + + hookEvent := webhook_module.HookEvent{ChooseEvents: true, HookEvents: webhook_module.HookEvents{Release: true}} + hook := &webhook_model.Webhook{ + RepoID: 3, + URL: s.URL + "/webhook", + ContentType: webhook_model.ContentTypeJSON, + IsActive: true, + Type: webhook_module.GITEA, + HookEvent: &hookEvent, + } + require.NoError(t, webhook_model.CreateWebhook(t.Context(), hook, "")) + + // if we deliver this webhook for a push event, nothing happens because the webhook isn't configured to run on those events + require.NoError(t, PrepareWebhook(db.DefaultContext, hook, webhook_module.HookEventPush, &api.ReleasePayload{})) + unittest.AssertNotExistsBean(t, &webhook_model.HookTask{ + HookID: hook.ID, + }) + + // but if we deliver it as a testing payload, the check on event types is bypassed + // (so that webhooks can be tested regardless of the event types they are enabled for) + // See https://codeberg.org/forgejo/forgejo/issues/7934 + require.NoError(t, PrepareTestWebhook(db.DefaultContext, hook, &api.ReleasePayload{})) + + unittest.AssertExistsAndLoadBean(t, &webhook_model.HookTask{ + HookID: hook.ID, + EventType: webhook_module.HookEventPush, + }) +}