unbridled-41 opened a new pull request, #4003:
URL: https://github.com/apache/rocketmq-dashboard/pull/4003
Fixes #TBD-ISSUE.
## Problem / Evidence
`GET /api/query-history/messages/{id}/results` returns the stored
query-result snapshot for any record id with no ownership check. Record ids are
sequential, so any authenticated user can enumerate
`/api/query-history/messages/1../results` and replay another user's stored
snapshots, which contain the topics, msgIds, tags, message keys, queue offsets,
broker names and — notably — the `bornHost`/`storeHost` addresses of the
clusters those users operate (`QueryHistoryService.buildResultSnapshot`, lines
109-124).
Every other read in the same service scopes to the authenticated user —
`listMessageQueries` (`QueryHistoryService.java:184`), `listTraceQueries`
(`:204`), and `summarize` (`:221-234`) all filter `queried_by =
currentUsernameOrSystem()`, and the owner is stamped at insert (`:93`). The
by-id results read added later (`selectById`, `:135`) is the only read that
skips the boundary. The scoping contract was established by the merged fix for
#2265 ("Query-history reads and summaries are scoped to the current
authenticated user by default"); the results endpoint did not exist yet when
that fix landed and never picked the boundary up.
## Root cause / Fix
`getMessageQueryResults` fetched the row by id alone. Fix: read the row
through the same owner-scoped `QueryWrapper` the other reads use (`.eq("id",
id).eq("queried_by", currentUsernameOrSystem())`) and keep returning 404 for
both missing and foreign records, so record existence is not leaked either. No
admin bypass is added, matching the list paths.
## Priority & scoring
PRIORITY 72 = impact 28 (authenticated cross-user disclosure of other
operators' query metadata and internal broker/producer host addresses) + blast
radius 12 (one endpoint, but all multi-user deployments with login required) +
reproducibility 18 (deterministic, sequential ids, one request per record) +
maintenance value 14 (aligns the last unscoped read with the service's own
established boundary; one-line fix).
FIX_CONFIDENCE 95 (the scoping pattern is already implemented and tested in
the same class).
## Tests
`mvn -B -ntp test
-Dtest='QueryHistoryServiceTest,QueryHistoryControllerTest'`:
- Before the fix, `getMessageQueryResultsHidesRecordsOwnedByOtherUsers`
failed because the foreign-owned record (owner `bob`, current user `alice`) was
returned instead of 404 — the defect reproduced.
- After the fix: QueryHistoryServiceTest **12/12** (10 pre-existing + 2 new,
including the owner-scoped wrapper SQL-segment assertion mirroring the existing
`queried_by` captor pattern) and QueryHistoryControllerTest 4/4.
- Full backend suite (`mvn -B -ntp test`): **Tests run: 2037, Failures: 3**
— exactly the pristine-baseline set (AuthCorsIntegrationTest ×2,
AliyunInstanceProviderTest ×1, identical messages); zero new failures; 2035
baseline + 2 new tests.
## Risk
Low. The normal UI flow only replays records from the requesting user's own
history list, which is already owner-scoped, so no legitimate path loses
access. Foreign or missing ids now return the same 404 as before for missing
ids. Data contents, endpoints and response shapes are unchanged.
--
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]
