123123213weqw opened a new pull request, #5724:
URL: https://github.com/apache/rocketmq-dashboard/pull/5724
## Problem / Evidence
`getGeneralSettings` redacts `dingtalkWebhook`/`smsWebhook` to the literal
`"******"` for non-admin sessions
(`SettingsService.redactNotificationWebhooks`), but the write side was never
taught about that marker:
- the console pre-fills the settings form with what it loaded and every save
echoes it back (`GeneralSettingsTab.persistPreference`/`mergeAndSave` re-read
the current record and post `buildPayload({...base, ...patch})`), and
- `saveGeneralSettings` preserves a blank `apiKey`/`dingtalkSigningSecret`
but has **no handling for the webhook marker**, so `"******"` is persisted
straight to the settings store.
Result: any save from a non-admin session — even one that only changes the
theme or the session timeout — silently overwrites both stored webhook URLs
with six asterisks, and every subsequent DingTalk/SMS notification fails
against a URL that is literally `******`. (Previously reported as #5004 /
#5000, which went stale and was auto-closed without landing.)
## Root cause / Fix
The redaction was not round-trip safe: the GET redacts, the client echoes,
and the POST does not recognise the marker the way it already recognises a
blank secret.
In `saveGeneralSettings`, when an incoming webhook equals
`REDACTED_NOTIFICATION_WEBHOOK`, restore the stored value instead of persisting
the marker — the same shape as the existing blank-secret preservation branches,
applied to both webhooks.
## Tests
- New
`saveGeneralSettingsShouldKeepExistingWebhooksWhenTheRedactedMarkerIsEchoedTest`
— fails on the pre-fix code (the marker was persisted verbatim).
- New `saveGeneralSettingsShouldReplaceAWebhookThatWasActuallyRetypedTest` —
pins that a genuinely retyped URL still replaces the stored one and a blank
still clears it, so the marker cannot lock edits out.
- `mvn test -Dtest='SettingsServiceTest,SettingsControllerTest'` → 68/68
passed.
## Risk
Low. One writer-side branch, no schema or API change. A deployment whose
stored webhook is already the literal `"******"` (corrupted earlier) keeps it
until an operator retypes a real URL — that retype path is explicitly tested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
