From ce0d1355cf8f83fb2fec6fa5a8777f540489e012 Mon Sep 17 00:00:00 2001 From: kolaente Date: Tue, 4 Aug 2026 13:21:38 +0200 Subject: [PATCH] fix(security): reject link shares in Webhook.ReadAll The guard on Webhook.CanRead is unreachable: no route exposes a read-one webhook, and DoReadAll never calls CanRead. Two paths were left open: - the v2 user-webhook list passes a.GetID() into Webhook.UserID, which is negative for a link share, so the w.UserID > 0 branch and its link share check were skipped and the request fell through to the project branch with project id 0, returning 404 instead of 403. - the project branch never rejected link shares at all, so any holder of a public share link could list the project's webhooks. target_url is a bearer secret for Slack, Discord, Teams and Zapier. Guard both by rejecting link shares at the top of ReadAll. --- pkg/models/link_sharing_test.go | 24 ++++++++++++++++++++++++ pkg/models/webhooks.go | 7 ++++++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/pkg/models/link_sharing_test.go b/pkg/models/link_sharing_test.go index 63a17952a..a8368cadc 100644 --- a/pkg/models/link_sharing_test.go +++ b/pkg/models/link_sharing_test.go @@ -419,4 +419,28 @@ func TestLinkSharing_CannotActAsCollidingUser(t *testing.T) { require.NoError(t, err) assert.False(t, can) }) + + t.Run("list the webhooks of the colliding user", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + share := &LinkSharing{ID: 1} + // the route fills UserID from the auth object + _, _, _, err := (&Webhook{UserID: share.GetID()}).ReadAll(s, share, "", 1, 50) + require.Error(t, err) + assert.True(t, IsErrGenericForbidden(err)) + }) + + t.Run("list the webhooks of the project the share points at", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + // link share 1 has read permission on project 1 + share := &LinkSharing{ID: 1, ProjectID: 1, Permission: PermissionRead} + _, _, _, err := (&Webhook{ProjectID: 1}).ReadAll(s, share, "", 1, 50) + require.Error(t, err) + assert.True(t, IsErrGenericForbidden(err)) + }) } diff --git a/pkg/models/webhooks.go b/pkg/models/webhooks.go index 2dfa65b5c..191696a9e 100644 --- a/pkg/models/webhooks.go +++ b/pkg/models/webhooks.go @@ -220,12 +220,17 @@ func (w *Webhook) Create(s *xorm.Session, a web.Auth) (err error) { // @Failure 500 {object} models.Message "Internal server error" // @Router /projects/{id}/webhooks [get] func (w *Webhook) ReadAll(s *xorm.Session, a web.Auth, _ string, page int, perPage int) (result interface{}, resultCount int, numberOfTotalItems int64, err error) { + // A link share can read its project, but webhook target_urls are secrets. + if _, is := a.(*LinkSharing); is { + return nil, 0, 0, ErrGenericForbidden{} + } + // w.UserID set selects the user-level list: a user may only see their own // webhooks. The project list (w.UserID == 0) delegates to the project's read // permission instead. var listCond builder.Cond if w.UserID > 0 { - if _, isShareAuth := a.(*LinkSharing); isShareAuth || w.UserID != a.GetID() { + if w.UserID != a.GetID() { return nil, 0, 0, ErrGenericForbidden{} } listCond = builder.Eq{"user_id": w.UserID}