From aac03229753408933fa65d0441ea6fe8044a29e8 Mon Sep 17 00:00:00 2001
From: kolaente <k@knt.li>
Date: Sat, 6 Jun 2026 21:21:13 +0200
Subject: [PATCH] refactor(webhooks): mask write-only credentials in the model
 so create/update never echo them
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Webhook.ReadAll already cleared the secret and basic-auth from responses,
but Create and Update did not, so the v2 handler patched the gap with a
maskWebhookCredentials helper. Centralize the masking in the model via a
maskCredentials helper called after every DB write (ReadAll, Create,
Update) and drop the v2 handler helper.

The credentials are client-provided, not server-generated: the DB row
keeps them and outgoing deliveries reload + HMAC-sign from the DB copy,
so clearing the returned in-memory struct is correct write-only handling.

Webhook is a shared model, so v1's create/update responses also stop
echoing the submitted secret/auth — intended, and approved by the
maintainer.
---
 pkg/models/webhooks.go        | 25 ++++++++++++++++++++++---
 pkg/routes/api/v2/webhooks.go | 12 ------------
 2 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/pkg/models/webhooks.go b/pkg/models/webhooks.go
index 3587bbc3f..e6353606f 100644
--- a/pkg/models/webhooks.go
+++ b/pkg/models/webhooks.go
@@ -79,6 +79,17 @@ func (w *Webhook) TableName() string {
 	return "webhooks"
 }
 
+// maskCredentials clears the write-only secret and basic-auth fields so they are
+// never echoed back in a response. The client already submitted these values and
+// the DB row keeps them (outgoing deliveries reload and sign from the DB copy);
+// only the in-memory struct returned to the caller is cleared. Always call this
+// after the DB write, never before.
+func (w *Webhook) maskCredentials() {
+	w.Secret = ""
+	w.BasicAuthUser = ""
+	w.BasicAuthPassword = ""
+}
+
 var availableWebhookEvents map[string]bool
 var availableWebhookEventsLock *sync.Mutex
 var userDirectedWebhookEvents map[string]bool
@@ -183,6 +194,11 @@ func (w *Webhook) Create(s *xorm.Session, a web.Auth) (err error) {
 	}
 
 	w.CreatedBy, err = user.GetUserByID(s, a.GetID())
+	if err != nil {
+		return err
+	}
+
+	w.maskCredentials()
 	return
 }
 
@@ -234,9 +250,7 @@ func (w *Webhook) ReadAll(s *xorm.Session, a web.Auth, _ string, page int, perPa
 	}
 
 	for _, webhook := range ws {
-		webhook.Secret = ""
-		webhook.BasicAuthUser = ""
-		webhook.BasicAuthPassword = ""
+		webhook.maskCredentials()
 		if createdBy, has := users[webhook.CreatedByID]; has {
 			webhook.CreatedBy = createdBy
 		}
@@ -268,6 +282,11 @@ func (w *Webhook) Update(s *xorm.Session, _ web.Auth) (err error) {
 	_, err = s.Where("id = ?", w.ID).
 		Cols("events").
 		Update(w)
+	if err != nil {
+		return err
+	}
+
+	w.maskCredentials()
 	return
 }
 
diff --git a/pkg/routes/api/v2/webhooks.go b/pkg/routes/api/v2/webhooks.go
index d0d581fa3..4f614a27a 100644
--- a/pkg/routes/api/v2/webhooks.go
+++ b/pkg/routes/api/v2/webhooks.go
@@ -116,7 +116,6 @@ func webhooksCreate(ctx context.Context, in *struct {
 	if err := handler.DoCreate(ctx, &in.Body, a); err != nil {
 		return nil, translateDomainError(err)
 	}
-	maskWebhookCredentials(&in.Body)
 	return &singleBody[models.Webhook]{Body: &in.Body}, nil
 }
 
@@ -137,7 +136,6 @@ func webhooksUpdate(ctx context.Context, in *struct {
 	if err := handler.DoUpdate(ctx, &in.Body, a); err != nil {
 		return nil, translateDomainError(err)
 	}
-	maskWebhookCredentials(&in.Body)
 	return &singleBody[models.Webhook]{Body: &in.Body}, nil
 }
 
@@ -154,13 +152,3 @@ func webhooksDelete(ctx context.Context, in *struct {
 	}
 	return &emptyBody{}, nil
 }
-
-// maskWebhookCredentials clears the write-only secret and auth fields so a
-// create/update response never echoes them back, mirroring how ReadAll masks
-// them. The fields are tagged writeOnly in the schema, but that is docs-only —
-// Huma does not strip them at runtime, so we clear them here.
-func maskWebhookCredentials(w *models.Webhook) {
-	w.Secret = ""
-	w.BasicAuthUser = ""
-	w.BasicAuthPassword = ""
-}