unbridled-41 opened a new pull request, #5436:
URL: https://github.com/apache/rocketmq-dashboard/pull/5436
## Problem / Evidence
"Clear key" in the AI assistant settings shows the success toast, but the
stored provider key is written straight back — the secret the operator removed
stays persisted and in use, and the next page load reports it as configured.
- `ops/ai/LlmConfigService.java:109-144` — `saveConfig` normalizes the
request (`normalizeWithStoredApiKey`, which correctly marks `clearApiKey=true`
and blanks the key) but then rebuilds the settings VO from scratch; the builder
copies `apiKey` and **never** `clearApiKey`, so it reverts to its default
`false`.
- `settings/SettingsService.java:134-138` — the merge reads exactly that
combination as "keep the stored key":
```java
if (settings.isClearApiKey()) { settings.setApiKey(""); }
else if (!StringUtils.hasText(settings.getApiKey()) && currentSettings !=
null) {
settings.setApiKey(currentSettings.getApiKey()); // the cleared key
comes back
}
```
- Call path: `web/src/pages/settings/AiAssistantTab.tsx:235-245`
(`saveLlmConfig({ apiKey: undefined, clearApiKey: true })` + success toast) →
`POST /api/llm/config` → `LlmConfigService.saveConfig` →
`SettingsService.saveGeneralSettings` → `MybatisPlusSettingsRepository` (the
key lands in `rmq_settings.json`).
- Why the tests missed it:
`LlmConfigServiceTest.saveConfigShouldClearStoredApiKeyWhenRequested` asserts
only the throwaway VO's blank `apiKey` while `SettingsService` is mocked, so
the merge that restores the key never runs.
## Root cause / Fix
The clear intent was lost when the persisted settings VO was rebuilt. Carry
it through:
```java
.clearApiKey(normalized.isClearApiKey())
```
`SettingsService` already handles the flag (it blanks the key and resets the
flag), so this is the only missing link. An ordinary save is unaffected: the
flag stays false and a blank key still means "keep the stored key", which is
what the form relies on when the operator edits only the model.
## Priority & scoring
PRIORITY 65 = 影响 25 (a credential the administrator explicitly removed
remains stored and in use — a security-relevant, silently wrong outcome) + 波及 8
(LLM settings write path) + 可复现 20 (deterministic UI action) + 维护价值 12 (one
flag at an untested seam).
FIX_CONFIDENCE 95: the flag is already normalized and already honoured by
the merge; the regression test now asserts the value that actually reaches
persistence.
## Tests
```
cd server && mvn -o test
-Dtest='LlmConfigServiceTest,SettingsServiceTest,SettingsControllerTest,LlmControllerTest'
```
- Red before the fix: `saveConfigShouldClearStoredApiKeyWhenRequested:314`
fails on the new `isClearApiKey()` assertion.
- Green after: `Tests run: 101, Failures: 0, Errors: 0, Skipped: 0`.
- New/strengthened regressions: the clear request must reach
`saveGeneralSettings` with `clearApiKey=true` and a blank key; an ordinary save
must reach it with `clearApiKey=false` and the submitted key, so the "blank
means keep" merge still works.
- `mvn -o checkstyle:check` passes.
## Risk
Low. Only a flag on the settings VO changes; the merge and the repository
are untouched, and the ordinary-save path is pinned by the added test.
--
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]
