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 <oliverpool@noreply.codeberg.org>
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: Antonin Delpeuch <antonin@delpeuch.eu>
Co-committed-by: Antonin Delpeuch <antonin@delpeuch.eu>

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,

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 {

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

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,
+	})
+}