Wang1rrr opened a new pull request, #4797: URL: https://github.com/apache/rocketmq-dashboard/pull/4797
### Which Issue(s) This PR Fixes No issue - this came out of a review of which endpoints return a plaintext secret and whether they leave an audit trail. ### Brief Description `CloudCredentialService` and `AclService` audit every mutation, but neither audited the one read that hands the caller a plaintext secret: - `CloudCredentialService.getCredential` recorded nothing, while create/update/delete record `CREATE_/UPDATE_/DELETE_CLOUD_CREDENTIAL`. - `AclService.getUserCredentials` recorded nothing, while `createUser` / `updateUser` / `deleteUser` / `createRule` / `deleteRule` / `upsertPlainAccessConfig` all call `recordAudit`. Those two endpoints are the complete set of secret-returning endpoints in the server module, and both are already admin-only (`AuthInterceptor.isAdminOnlyGetPath` covers the `/api/acl/users/` and `/api/cloud-credentials/` prefixes). So this is not a privilege gap - it is the loss of the only durable record that a disclosure happened. `OperationAuditService` stamps the acting operator onto the row, and with no row a credential read was indistinguishable from no activity at all. The change records `REVEAL_CLOUD_CREDENTIAL` and `REVEAL_ACL_USER_CREDENTIALS`, naming the credential but never the secret. `AclService.getUserCredentials` is restructured from two early returns into a single exit so the audit covers both the Apache repository path and the Tencent vendor path; the Tencent branch returned directly and would otherwise have missed the call. `recordAudit` keeps swallowing sink failures, so an audit outage still cannot fail an operation that already succeeded. ### How Did You Test This Change? Added cases to `AclServiceTest` and `CloudCredentialServiceTest` asserting the reveal row is written on both the Apache and the Tencent path, and that the recorded detail names the credential without carrying the secret. ``` cd server mvn -B -Dtest=AclServiceTest,CloudCredentialServiceTest test [INFO] You have 0 Checkstyle violations. [INFO] Tests run: 80, Failures: 0, Errors: 0, Skipped: 0 -- in org.apache.rocketmq.studio.instance.acl.AclServiceTest [INFO] Tests run: 18, Failures: 0, Errors: 0, Skipped: 0 -- in org.apache.rocketmq.studio.provider.credential.CloudCredentialServiceTest [INFO] Tests run: 98, Failures: 0, Errors: 0, Skipped: 0 [INFO] BUILD SUCCESS ``` Backend only: no UI text, no new source files, no docs surface changed. ### Checklist - [x] One coherent change; unrelated modifications are not bundled in - [x] Commit subject follows Conventional Commits (`feat:` / `fix:` / `refactor:` / `chore:` / `docs:` / `perf:`) - [x] Tests added or updated for non-trivial changes, test methods named `...Test` - [ ] New UI text has both Chinese and English entries under `web/src/i18n/` - N/A, no UI text - [ ] Architecture constraints stay green (`mvn test` runs the ArchUnit checks) - no new types or packages introduced - [ ] New source files carry the ASF license header - N/A, no new files - [ ] Documentation touched where behaviour changed - N/A, the audit vocabulary is defined in code -- 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]
