RockteMQ-AI commented on code in PR #2552:
URL:
https://github.com/apache/rocketmq-dashboard/pull/2552#discussion_r3848864051
##########
server/src/main/java/org/apache/rocketmq/studio/cluster/broker/ClusterController.java:
##########
@@ -69,6 +70,12 @@ public Result<ClusterConfigUpdateResultVO>
updateClusterConfig(@Valid @RequestBo
return Result.ok(clusterService.updateClusterConfig(command));
}
+ @PostMapping("/config/preview")
Review Comment:
**[Critical]** Missing authorization check on preview endpoint.
The `/clusters/config/preview` endpoint does not have any `@PreAuthorize` or
similar security annotation. While this is a read-only operation, it exposes
sensitive internal configuration details (broker addresses, current config
values, cluster topology) to potentially unauthenticated users.
**Recommendation:** Add the same authorization annotation as the
`updateClusterConfig` endpoint to ensure only authorized users can preview
cluster configurations.
##########
server/src/main/java/org/apache/rocketmq/studio/cluster/broker/ClusterService.java:
##########
@@ -157,6 +161,26 @@ public void requireProxy(String clusterId, String addr) {
requireProxy(cluster, addr);
}
+ public ClusterConfigPreviewVO previewClusterConfig(UpdateConfigDTO
command) {
Review Comment:
**[Info]** Potential edge case in queue number validation
The `requireMatchingDefaultQueueNums()` method validates that
`writeQueueNums` and `readQueueNums` match when both are provided. However, if
only one is set, the validation passes. Both map to the same broker property
`defaultTopicQueueNums`. Consider adding a comment explaining this behavior or
validating that both must be set together.
--
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]
