From b2d7a6cba0477acb133e138cead00b993fd9ffd9 Mon Sep 17 00:00:00 2001 From: hachimitsu-pichi Date: Wed, 21 Jan 2026 14:38:48 +0900 Subject: [PATCH 1/7] fix(webhook): validate signature and body for DELETE webhook message API --- router/v3/router.go | 9 +++++---- router/v3/webhooks.go | 18 ++++++++++++++++++ router/v3/webhooks_test.go | 31 +++++++++++++++++++++++-------- 3 files changed, 46 insertions(+), 12 deletions(-) diff --git a/router/v3/router.go b/router/v3/router.go index a1b80c85a..aa7a3294e 100644 --- a/router/v3/router.go +++ b/router/v3/router.go @@ -292,10 +292,7 @@ func (h *Handlers) Setup(e *echo.Group) { apiWebhooksWID.GET("/icon", h.GetWebhookIcon, requires(permission.GetWebhook)) apiWebhooksWID.PUT("/icon", h.ChangeWebhookIcon, requires(permission.EditWebhook)) apiWebhooksWID.GET("/messages", h.GetWebhookMessages, requires(permission.GetWebhook)) - apiWebhooksWIDMessage := apiWebhooksWID.Group("/messages/:messageID", requires(permission.GetWebhook), retrieve.MessageID(), requiresMessageAccessPerm) - { - apiWebhooksWIDMessage.DELETE("", h.DeleteWebhookMessage, requires(permission.GetMessage)) - } + } } apiGroups := api.Group("/groups") @@ -422,6 +419,10 @@ func (h *Handlers) Setup(e *echo.Group) { apiNoAuth.POST("/login", h.Login, noLogin) apiNoAuth.POST("/logout", h.Logout) apiNoAuth.POST("/webhooks/:webhookID", h.PostWebhook, retrieve.WebhookID()) + apiWebhooks := apiNoAuth.Group("/messages/:messageID") + { + apiWebhooks.DELETE("", h.DeleteWebhookMessage, retrieve.MessageID(), retrieve.WebhookID()) + } apiNoAuth.POST("/qall/webhook", h.LiveKitWebhook) apiNoAuthPublic := apiNoAuth.Group("/public") { diff --git a/router/v3/webhooks.go b/router/v3/webhooks.go index 2ca09ddfb..43b22abde 100644 --- a/router/v3/webhooks.go +++ b/router/v3/webhooks.go @@ -254,6 +254,24 @@ func (h *Handlers) DeleteWebhookMessage(c echo.Context) error { messageID := getParamAsUUID(c, consts.ParamMessageID) botUserID := w.GetBotUserID() messageUserID := m.GetUserID() + + // Webhookシークレット確認 + body, err := io.ReadAll(c.Request().Body) + if err != nil { + return herror.InternalServerError(err) + } + if len(body) == 0 { + return herror.BadRequest("empty body") + } + if len(w.GetSecret()) > 0 { + sig, _ := hex.DecodeString(c.Request().Header.Get(consts.HeaderSignature)) + if len(sig) == 0 { + return herror.BadRequest("missing X-TRAQ-Signature header") + } + if subtle.ConstantTimeCompare(hmac.SHA1(body, w.GetSecret()), sig) != 1 { + return herror.BadRequest("X-TRAQ-Signature is wrong") + } + } if botUserID == messageUserID { if err := h.Repo.DeleteMessage(messageID); err != nil { diff --git a/router/v3/webhooks_test.go b/router/v3/webhooks_test.go index d7f175101..df4f93db0 100644 --- a/router/v3/webhooks_test.go +++ b/router/v3/webhooks_test.go @@ -551,21 +551,36 @@ func TestHandlers_DeleteWebhookMessage(t *testing.T) { wh := env.CreateWebhook(t, rand, user.GetID(), ch.ID) wh2 := env.CreateWebhook(t, rand, user.GetID(), ch.ID) message := env.CreateMessage(t, wh.GetBotUserID(), ch.ID, "test") - s := env.S(t, user.GetID()) - t.Run("not logged in", func(t *testing.T) { + calcHMACSHA1 := func(t *testing.T, message, secret string) string { + t.Helper() + mac := hmac.New(sha1.New, []byte(secret)) + _, _ = mac.Write([]byte(message)) + return hex.EncodeToString(mac.Sum(nil)) + } + + t.Run("bad request (no signature)", func(t *testing.T) { t.Parallel() e := env.R(t) - e.DELETE(path, wh.GetID(), message.GetID()). + e.PUT(path, wh.GetID(), message.GetID()). Expect(). - Status(http.StatusUnauthorized) + Status(http.StatusBadRequest) + }) + + t.Run("bad request (bad signature)", func(t *testing.T) { + t.Parallel() + e := env.R(t) + e.PUT(path, wh.GetID(), message.GetID()). + WithHeader("X-TRAQ-Signature", calcHMACSHA1(t, "test", wh.GetSecret())). + Expect(). + Status(http.StatusBadRequest) }) t.Run("not found webhook", func(t *testing.T) { t.Parallel() e := env.R(t) e.DELETE(path, uuid.Must(uuid.NewV4()), message.GetID()). - WithCookie(session.CookieName, s). + WithHeader("X-TRAQ-Signature", calcHMACSHA1(t, "", wh.GetSecret())). Expect(). Status(http.StatusNotFound) }) @@ -574,7 +589,7 @@ func TestHandlers_DeleteWebhookMessage(t *testing.T) { t.Parallel() e := env.R(t) e.DELETE(path, wh.GetID(), uuid.Must(uuid.NewV4())). - WithCookie(session.CookieName, s). + WithHeader("X-TRAQ-Signature", calcHMACSHA1(t, "", wh.GetSecret())). Expect(). Status(http.StatusNotFound) }) @@ -583,7 +598,7 @@ func TestHandlers_DeleteWebhookMessage(t *testing.T) { t.Parallel() e := env.R(t) e.DELETE(path, wh2.GetID(), message.GetID()). - WithCookie(session.CookieName, s). + WithHeader("X-TRAQ-Signature", calcHMACSHA1(t, "", wh.GetSecret())). Expect(). Status(http.StatusForbidden) }) @@ -592,7 +607,7 @@ func TestHandlers_DeleteWebhookMessage(t *testing.T) { t.Parallel() e := env.R(t) e.DELETE(path, wh.GetID(), message.GetID()). - WithCookie(session.CookieName, s). + WithHeader("X-TRAQ-Signature", calcHMACSHA1(t, "", wh.GetSecret())). Expect(). Status(http.StatusNoContent) From cbbd8be9a2a65636d5ea161122bb4042dd18e05e Mon Sep 17 00:00:00 2001 From: hachimitsu-pichi Date: Wed, 21 Jan 2026 14:51:57 +0900 Subject: [PATCH 2/7] fix(webhook): correct HTTP method for DeleteWebhookMessage test cases --- router/v3/webhooks.go | 2 +- router/v3/webhooks_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/router/v3/webhooks.go b/router/v3/webhooks.go index 43b22abde..55215f64c 100644 --- a/router/v3/webhooks.go +++ b/router/v3/webhooks.go @@ -254,7 +254,7 @@ func (h *Handlers) DeleteWebhookMessage(c echo.Context) error { messageID := getParamAsUUID(c, consts.ParamMessageID) botUserID := w.GetBotUserID() messageUserID := m.GetUserID() - + // Webhookシークレット確認 body, err := io.ReadAll(c.Request().Body) if err != nil { diff --git a/router/v3/webhooks_test.go b/router/v3/webhooks_test.go index df4f93db0..f010ce931 100644 --- a/router/v3/webhooks_test.go +++ b/router/v3/webhooks_test.go @@ -562,7 +562,7 @@ func TestHandlers_DeleteWebhookMessage(t *testing.T) { t.Run("bad request (no signature)", func(t *testing.T) { t.Parallel() e := env.R(t) - e.PUT(path, wh.GetID(), message.GetID()). + e.DELETE(path, wh.GetID(), message.GetID()). Expect(). Status(http.StatusBadRequest) }) @@ -570,7 +570,7 @@ func TestHandlers_DeleteWebhookMessage(t *testing.T) { t.Run("bad request (bad signature)", func(t *testing.T) { t.Parallel() e := env.R(t) - e.PUT(path, wh.GetID(), message.GetID()). + e.DELETE(path, wh.GetID(), message.GetID()). WithHeader("X-TRAQ-Signature", calcHMACSHA1(t, "test", wh.GetSecret())). Expect(). Status(http.StatusBadRequest) From a7f5042c2b78711f32b9bb50c0fc68728a75fd3f Mon Sep 17 00:00:00 2001 From: hachimitsu-pichi Date: Wed, 21 Jan 2026 15:10:19 +0900 Subject: [PATCH 3/7] fix(webhook): update DELETE webhook message test to use newly created message --- router/v3/webhooks_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/router/v3/webhooks_test.go b/router/v3/webhooks_test.go index f010ce931..01389b193 100644 --- a/router/v3/webhooks_test.go +++ b/router/v3/webhooks_test.go @@ -606,12 +606,13 @@ func TestHandlers_DeleteWebhookMessage(t *testing.T) { t.Run("success", func(t *testing.T) { t.Parallel() e := env.R(t) - e.DELETE(path, wh.GetID(), message.GetID()). + message2 := env.CreateMessage(t, wh.GetBotUserID(), ch.ID, "test") + e.DELETE(path, wh.GetID(), message2.GetID()). WithHeader("X-TRAQ-Signature", calcHMACSHA1(t, "", wh.GetSecret())). Expect(). Status(http.StatusNoContent) - _, err := env.Repository.GetMessageByID(message.GetID()) + _, err := env.Repository.GetMessageByID(message2.GetID()) assert.ErrorIs(t, err, repository.ErrNotFound) }) } From 098ff5fa619c4f14f7712266990df0e347a59f47 Mon Sep 17 00:00:00 2001 From: hachimitsu-pichi Date: Wed, 21 Jan 2026 15:31:02 +0900 Subject: [PATCH 4/7] fix(webhook): restructure webhook routes to include message deletion under specific webhook context --- router/v3/router.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/router/v3/router.go b/router/v3/router.go index aa7a3294e..ff3a2679a 100644 --- a/router/v3/router.go +++ b/router/v3/router.go @@ -418,10 +418,13 @@ func (h *Handlers) Setup(e *echo.Group) { } apiNoAuth.POST("/login", h.Login, noLogin) apiNoAuth.POST("/logout", h.Logout) - apiNoAuth.POST("/webhooks/:webhookID", h.PostWebhook, retrieve.WebhookID()) - apiWebhooks := apiNoAuth.Group("/messages/:messageID") + apiWebhooks := apiNoAuth.Group("/webhook/:webhookID", retrieve.WebhookID()) { - apiWebhooks.DELETE("", h.DeleteWebhookMessage, retrieve.MessageID(), retrieve.WebhookID()) + apiWebhooks.POST("", h.PostWebhook) + apiWebhooksMessages := apiWebhooks.Group("/messages/:messageID", retrieve.MessageID()) + { + apiWebhooksMessages.DELETE("", h.DeleteWebhookMessage) + } } apiNoAuth.POST("/qall/webhook", h.LiveKitWebhook) apiNoAuthPublic := apiNoAuth.Group("/public") From 3d745566af1edb587c54c7b41e61f893ae44c84e Mon Sep 17 00:00:00 2001 From: Eraxyso <130852025+Eraxyso@users.noreply.github.com> Date: Wed, 21 Jan 2026 06:45:37 +0000 Subject: [PATCH 5/7] fix(webhook): update signature validation for DELETE webhook message API --- router/v3/webhooks.go | 9 +-------- router/v3/webhooks_test.go | 2 +- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/router/v3/webhooks.go b/router/v3/webhooks.go index 55215f64c..bb6dfac61 100644 --- a/router/v3/webhooks.go +++ b/router/v3/webhooks.go @@ -256,19 +256,12 @@ func (h *Handlers) DeleteWebhookMessage(c echo.Context) error { messageUserID := m.GetUserID() // Webhookシークレット確認 - body, err := io.ReadAll(c.Request().Body) - if err != nil { - return herror.InternalServerError(err) - } - if len(body) == 0 { - return herror.BadRequest("empty body") - } if len(w.GetSecret()) > 0 { sig, _ := hex.DecodeString(c.Request().Header.Get(consts.HeaderSignature)) if len(sig) == 0 { return herror.BadRequest("missing X-TRAQ-Signature header") } - if subtle.ConstantTimeCompare(hmac.SHA1(body, w.GetSecret()), sig) != 1 { + if subtle.ConstantTimeCompare(hmac.SHA1("", w.GetSecret()), sig) != 1 { return herror.BadRequest("X-TRAQ-Signature is wrong") } } diff --git a/router/v3/webhooks_test.go b/router/v3/webhooks_test.go index 01389b193..51304985e 100644 --- a/router/v3/webhooks_test.go +++ b/router/v3/webhooks_test.go @@ -598,7 +598,7 @@ func TestHandlers_DeleteWebhookMessage(t *testing.T) { t.Parallel() e := env.R(t) e.DELETE(path, wh2.GetID(), message.GetID()). - WithHeader("X-TRAQ-Signature", calcHMACSHA1(t, "", wh.GetSecret())). + WithHeader("X-TRAQ-Signature", calcHMACSHA1(t, "", wh2.GetSecret())). Expect(). Status(http.StatusForbidden) }) From dd42c149ea6e93866ec31f4c146e6fa922dd1559 Mon Sep 17 00:00:00 2001 From: Eraxyso <130852025+Eraxyso@users.noreply.github.com> Date: Wed, 21 Jan 2026 06:48:05 +0000 Subject: [PATCH 6/7] fix(webhook): correct signature validation for DELETE webhook message API --- router/v3/webhooks.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/router/v3/webhooks.go b/router/v3/webhooks.go index bb6dfac61..42a55f292 100644 --- a/router/v3/webhooks.go +++ b/router/v3/webhooks.go @@ -261,7 +261,7 @@ func (h *Handlers) DeleteWebhookMessage(c echo.Context) error { if len(sig) == 0 { return herror.BadRequest("missing X-TRAQ-Signature header") } - if subtle.ConstantTimeCompare(hmac.SHA1("", w.GetSecret()), sig) != 1 { + if subtle.ConstantTimeCompare(hmac.SHA1([]byte{}, w.GetSecret()), sig) != 1 { return herror.BadRequest("X-TRAQ-Signature is wrong") } } From e63c352c0218a320e1f982025a9286ba16f54d41 Mon Sep 17 00:00:00 2001 From: hachimitsu-pichi Date: Wed, 21 Jan 2026 16:04:50 +0900 Subject: [PATCH 7/7] fix(webhook): correct endpoint path for webhook group to plural form --- router/v3/router.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/router/v3/router.go b/router/v3/router.go index ff3a2679a..cdc16262f 100644 --- a/router/v3/router.go +++ b/router/v3/router.go @@ -418,7 +418,7 @@ func (h *Handlers) Setup(e *echo.Group) { } apiNoAuth.POST("/login", h.Login, noLogin) apiNoAuth.POST("/logout", h.Logout) - apiWebhooks := apiNoAuth.Group("/webhook/:webhookID", retrieve.WebhookID()) + apiWebhooks := apiNoAuth.Group("/webhooks/:webhookID", retrieve.WebhookID()) { apiWebhooks.POST("", h.PostWebhook) apiWebhooksMessages := apiWebhooks.Group("/messages/:messageID", retrieve.MessageID())