From 39d398066d8c644fe452f74704e34ada6c7ab61e Mon Sep 17 00:00:00 2001 From: naiba <29243953+naiba@users.noreply.github.com> Date: Sat, 20 Jun 2026 01:54:50 +0000 Subject: [PATCH] fix(api): redact third-party credentials in ddns/notification list GET /api/v1/ddns and /api/v1/notification returned full objects with plaintext credentials (Cloudflare/TencentCloud secrets, webhook URLs with embedded bot tokens, Authorization headers). Redact these fields in the list responses. Since the frontend edit form repopulates from the list endpoint, the update handlers now treat an empty submitted credential as "no change" and preserve the stored value, preventing accidental secret wipes. Ref: GHSA-ww5p-j6cj-6mqq --- .../controller/credential_redaction_test.go | 174 ++++++++++++++++++ cmd/dashboard/controller/ddns.go | 17 +- cmd/dashboard/controller/notification.go | 22 ++- service/singleton/testhelpers.go | 2 + 4 files changed, 210 insertions(+), 5 deletions(-) create mode 100644 cmd/dashboard/controller/credential_redaction_test.go diff --git a/cmd/dashboard/controller/credential_redaction_test.go b/cmd/dashboard/controller/credential_redaction_test.go new file mode 100644 index 00000000..bb4ff01d --- /dev/null +++ b/cmd/dashboard/controller/credential_redaction_test.go @@ -0,0 +1,174 @@ +package controller + +import ( + "testing" + + "github.com/gin-gonic/gin" + "github.com/stretchr/testify/require" + + "github.com/nezhahq/nezha/model" + "github.com/nezhahq/nezha/service/singleton" +) + +func TestListDDNS_RedactsCredentials(t *testing.T) { + defer setupTenancyTest(t)() + + p := model.DDNSProfile{ + Common: model.Common{UserID: 10}, + Name: "cf", + Provider: "cloudflare", + AccessID: "id", + AccessSecret: "super-secret-token", + WebhookHeaders: `{"Authorization":"Bearer xxx"}`, + } + require.NoError(t, singleton.DB.Create(&p).Error) + singleton.DDNSShared.InsertForTest(&p) + + c := ctxAs(10, model.RoleAdmin) + out, err := listDDNS(c) + require.NoError(t, err) + require.Len(t, out, 1) + require.Empty(t, out[0].AccessSecret, "access_secret must be redacted in list response") + require.Empty(t, out[0].WebhookHeaders, "webhook_headers must be redacted in list response") + require.Equal(t, "id", out[0].AccessID, "non-secret fields must be preserved") + + var stored model.DDNSProfile + require.NoError(t, singleton.DB.First(&stored, p.ID).Error) + require.Equal(t, "super-secret-token", stored.AccessSecret, "redaction must not mutate stored data") +} + +func TestListNotification_RedactsCredentials(t *testing.T) { + defer setupTenancyTest(t)() + + n := model.Notification{ + Common: model.Common{UserID: 10}, + Name: "slack", + URL: "https://hooks.slack.com/services/T/B/secret", + RequestHeader: `{"Authorization":"Bearer xxx"}`, + RequestBody: `{"text":"#NEZHA#"}`, + } + require.NoError(t, singleton.DB.Create(&n).Error) + singleton.NotificationShared.InsertForTest(&n) + + c := ctxAs(10, model.RoleAdmin) + out, err := listNotification(c) + require.NoError(t, err) + require.Len(t, out, 1) + require.Empty(t, out[0].URL, "url must be redacted in list response") + require.Empty(t, out[0].RequestHeader, "request_header must be redacted in list response") + require.Empty(t, out[0].RequestBody, "request_body must be redacted in list response") + require.Equal(t, "slack", out[0].Name, "non-secret fields must be preserved") + + var stored model.Notification + require.NoError(t, singleton.DB.First(&stored, n.ID).Error) + require.Equal(t, "https://hooks.slack.com/services/T/B/secret", stored.URL, + "redaction must not mutate stored data") +} + +func TestUpdateDDNS_EmptySecretPreservesStored(t *testing.T) { + defer setupTenancyTest(t)() + + existing := model.DDNSProfile{ + Common: model.Common{UserID: 10}, + Name: "cf", + Provider: "webhook", + AccessID: "id", + AccessSecret: "keep-me", + WebhookURL: "http://127.0.0.1/", + WebhookMethod: 1, + WebhookHeaders: `{"X-Token":"keep-header"}`, + } + require.NoError(t, singleton.DB.Create(&existing).Error) + singleton.DDNSShared.InsertForTest(&existing) + + c := ctxAsMemberWithBody(10, map[string]any{ + "name": "cf-renamed", + "provider": "webhook", + "access_id": "id", + "access_secret": "", + "webhook_url": "http://127.0.0.1/", + "webhook_method": 1, + "webhook_request_type": 1, + "webhook_headers": "", + "max_retries": 3, + }) + c.Params = gin.Params{{Key: "id", Value: itoa(existing.ID)}} + _, err := updateDDNS(c) + require.NoError(t, err) + + var after model.DDNSProfile + require.NoError(t, singleton.DB.First(&after, existing.ID).Error) + require.Equal(t, "cf-renamed", after.Name, "non-secret edits must apply") + require.Equal(t, "keep-me", after.AccessSecret, "empty submitted secret must preserve stored value") + require.Equal(t, `{"X-Token":"keep-header"}`, after.WebhookHeaders, + "empty submitted headers must preserve stored value") +} + +func TestUpdateDDNS_NonEmptySecretOverwrites(t *testing.T) { + defer setupTenancyTest(t)() + + existing := model.DDNSProfile{ + Common: model.Common{UserID: 10}, + Name: "cf", + Provider: "webhook", + AccessSecret: "old", + WebhookURL: "http://127.0.0.1/", + WebhookMethod: 1, + } + require.NoError(t, singleton.DB.Create(&existing).Error) + singleton.DDNSShared.InsertForTest(&existing) + + c := ctxAsMemberWithBody(10, map[string]any{ + "name": "cf", + "provider": "webhook", + "access_secret": "new-secret", + "webhook_url": "http://127.0.0.1/", + "webhook_method": 1, + "webhook_request_type": 1, + "max_retries": 3, + }) + c.Params = gin.Params{{Key: "id", Value: itoa(existing.ID)}} + _, err := updateDDNS(c) + require.NoError(t, err) + + var after model.DDNSProfile + require.NoError(t, singleton.DB.First(&after, existing.ID).Error) + require.Equal(t, "new-secret", after.AccessSecret, "non-empty submitted secret must overwrite") +} + +func TestUpdateNotification_EmptyFieldsPreserveStored(t *testing.T) { + defer setupTenancyTest(t)() + + existing := model.Notification{ + Common: model.Common{UserID: 10}, + Name: "slack", + URL: "https://hooks.slack.com/services/keep", + RequestMethod: model.NotificationRequestMethodGET, + RequestType: model.NotificationRequestTypeJSON, + RequestHeader: `{"Authorization":"keep"}`, + RequestBody: "", + } + require.NoError(t, singleton.DB.Create(&existing).Error) + singleton.NotificationShared.InsertForTest(&existing) + + c := ctxAsMemberWithBody(10, map[string]any{ + "name": "slack-renamed", + "url": "", + "request_method": model.NotificationRequestMethodGET, + "request_type": model.NotificationRequestTypeJSON, + "request_header": "", + "request_body": "", + "skip_check": true, + }) + c.Params = gin.Params{{Key: "id", Value: itoa(existing.ID)}} + _, err := updateNotification(c) + require.NoError(t, err) + + var after model.Notification + require.NoError(t, singleton.DB.First(&after, existing.ID).Error) + require.Equal(t, "slack-renamed", after.Name, "non-secret edits must apply") + require.Equal(t, "https://hooks.slack.com/services/keep", after.URL, + "empty submitted url must preserve stored value") + require.Equal(t, `{"Authorization":"keep"}`, after.RequestHeader, + "empty submitted request_header must preserve stored value") +} diff --git a/cmd/dashboard/controller/ddns.go b/cmd/dashboard/controller/ddns.go index 5b204a47..a2a2c20e 100644 --- a/cmd/dashboard/controller/ddns.go +++ b/cmd/dashboard/controller/ddns.go @@ -30,6 +30,13 @@ func listDDNS(c *gin.Context) ([]*model.DDNSProfile, error) { return nil, err } + // 列表端点不回显写入态凭据：ddnsProfiles 是 copier 复制出的副本，置零安全， + // 不影响 singleton 内原始数据。 + for _, p := range ddnsProfiles { + p.AccessSecret = "" + p.WebhookHeaders = "" + } + return ddnsProfiles, nil } @@ -137,12 +144,18 @@ func updateDDNS(c *gin.Context) (any, error) { p.Provider = df.Provider p.Domains = df.Domains p.AccessID = df.AccessID - p.AccessSecret = df.AccessSecret p.WebhookURL = df.WebhookURL p.WebhookMethod = df.WebhookMethod p.WebhookRequestType = df.WebhookRequestType p.WebhookRequestBody = df.WebhookRequestBody - p.WebhookHeaders = df.WebhookHeaders + + // 凭据在列表接口已脱敏，前端无法回填；空值视为"不修改"，保留旧值避免误清空。 + if df.AccessSecret != "" { + p.AccessSecret = df.AccessSecret + } + if df.WebhookHeaders != "" { + p.WebhookHeaders = df.WebhookHeaders + } for n, domain := range p.Domains { // IDN to ASCII diff --git a/cmd/dashboard/controller/notification.go b/cmd/dashboard/controller/notification.go index 774ecf22..43403e34 100644 --- a/cmd/dashboard/controller/notification.go +++ b/cmd/dashboard/controller/notification.go @@ -29,6 +29,14 @@ func listNotification(c *gin.Context) ([]*model.Notification, error) { if err := copier.Copy(¬ifications, &slist); err != nil { return nil, err } + + // 列表端点不回显写入态凭据：notifications 是 copier 复制出的副本，置零安全， + // 不影响 singleton 内原始数据。 + for _, n := range notifications { + n.URL = "" + n.RequestHeader = "" + n.RequestBody = "" + } return notifications, nil } @@ -117,14 +125,22 @@ func updateNotification(c *gin.Context) (any, error) { n.Name = nf.Name n.RequestMethod = nf.RequestMethod n.RequestType = nf.RequestType - n.RequestHeader = nf.RequestHeader - n.RequestBody = nf.RequestBody - n.URL = nf.URL verifyTLS := nf.VerifyTLS n.VerifyTLS = &verifyTLS formatMetricUnits := nf.FormatMetricUnits n.FormatMetricUnits = &formatMetricUnits + // 凭据在列表接口已脱敏，前端无法回填；空值视为"不修改"，保留旧值避免误清空。 + if nf.URL != "" { + n.URL = nf.URL + } + if nf.RequestHeader != "" { + n.RequestHeader = nf.RequestHeader + } + if nf.RequestBody != "" { + n.RequestBody = nf.RequestBody + } + ns := model.NotificationServerBundle{ Notification: &n, Server: nil, diff --git a/service/singleton/testhelpers.go b/service/singleton/testhelpers.go index 459a531a..eb60b9c6 100644 --- a/service/singleton/testhelpers.go +++ b/service/singleton/testhelpers.go @@ -43,6 +43,7 @@ func (c *DDNSClass) InsertForTest(p *model.DDNSProfile) { c.listMu.Lock() c.list[p.ID] = p c.listMu.Unlock() + c.sortList() } // NewEmptyNotificationClassForTest 构造空 NotificationClass。 @@ -62,4 +63,5 @@ func (c *NotificationClass) InsertForTest(n *model.Notification) { c.listMu.Lock() c.list[n.ID] = n c.listMu.Unlock() + c.sortList() }