123123213weqw opened a new pull request, #4472:
URL: https://github.com/apache/rocketmq-dashboard/pull/4472
### Which Issue(s) This PR Fixes
No issue: both are small hardening gaps found while reading the LLM
configuration code. Happy to file one if you would rather track it.
### Brief Description
Two fixes with one theme: the AI configuration must not hand out its
provider credentials, and a provider name must mean the same thing when it is
registered as when it is looked up.
1. **`LlmProperties.toString()` printed the provider token.** Any log line,
exception message or debugging output that holds the configuration leaked the
credential. The field carries `@ToString.Exclude` now.
2. **`AgentProviderRegistry` normalized the engine only on lookup.**
`forEngine` trims and lowercases the requested engine, but the registry was
built from the raw string each provider reported, so a provider reporting
`Claude` registered under `Claude` and could never be resolved by
`forEngine("claude")`. Registration now normalizes the same way, and rejects an
empty or duplicate engine with a clear message rather than silently keeping
whichever entry `Collectors.toMap` happened to keep.
Both are revived from branches whose pull requests were closed when
`rocketmq-studio` was folded into `master` and the base branch was deleted, and
each keeps its red/green pair.
### Scope note on the rest of the AI cluster
I looked at the other closed AI pull requests before opening this, and
deliberately did **not** resubmit three of them, because `master` has moved
past them:
- `#3120` (guard a null cluster in capability resolution) patched
`ops/ai/tool/CapabilityResolver`, which has since moved to
`ops/ai/tool/catalog/CapabilityResolver` and been **redesigned** — the current
class resolves through `InstanceProviderRegistry`/`InstanceResolver` and
already handles a missing instance and a null vendor. The old patch would only
add a stale second `@Component` of the same name, which makes the Spring
context fail to load.
- `#2898` (tolerate malformed tool names) and `#2845` (bound the catalog
major version) patched `ToolAccessPolicy` and `ToolCatalog.majorVersion`, both
of which no longer exist after the tool layer was split into
`catalog`/`contract`/`core`/`filter`/`handler`/`service`/`support`.
The remaining applicable AI fix is #4357, submitted separately as #4470.
### How Did You Test This Change?
```
cd server && mvn -B -ntp test
[INFO] Tests run: 2430, Failures: 0, Errors: 0, Skipped: 0
mvn -B -ntp test -Dtest="LlmPropertiesTest,AgentProviderRegistryTest"
[INFO] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0
```
The 17 errors reported next to the full run are the `@SpringBootTest`
classes that need a reachable MySQL 8; they fail identically on the untouched
baseline.
Test coverage:
- `LlmPropertiesTest` — `toString()` does not contain the configured token.
- `AgentProviderRegistryTest` — a provider reporting a differently-cased
engine is still resolved by `forEngine`, and an empty or duplicate engine is
rejected at construction.
### Checklist
- [x] One coherent change; unrelated modifications are not bundled in
- [x] Commit subject follows Conventional Commits (`fix:`)
- [x] Tests added or updated for non-trivial changes, test methods named
`...Test`
- [x] New UI text has both Chinese and English entries under `web/src/i18n/`
— not applicable, no UI text added
- [x] Architecture constraints stay green (`mvn test` runs the ArchUnit
checks)
- [x] New source files carry the ASF license header — `LlmPropertiesTest`
carries the ASF header
- [x] Documentation touched where behaviour changed — none describes the
registration contract
--
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]
