[history server] Add redirect capability to enter_cluster - #5086
[history server] Add redirect capability to enter_cluster#5086KunWuLuan wants to merge 2 commits into
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
Reviewed by Cursor Bugbot for commit d5089e4. Configure here.
Add an optional `redirect` query parameter to the enter_cluster endpoint. When set, the handler responds with a 302 to the given path (Set-Cookie headers still ride along on the redirect) instead of the JSON body, so a single navigation both establishes the cluster context and lands on the dashboard, e.g. /enter_cluster/<ns>/<kind>/<name>/latest?redirect=/#/overview Redirect targets are validated by isSafeRedirectPath to prevent open redirects: only same-origin absolute paths are allowed. Behavior without the parameter is unchanged (still returns JSON).
d5089e4 to
5e454cf
Compare
|
I tried this PR, following docs here to start ray dashboard and proxy to history server However, when trying with: I'll get |
| if strings.HasPrefix(target, "//") || strings.HasPrefix(target, "/\\") { | ||
| return false | ||
| } |
There was a problem hiding this comment.
We may get something like redirect=/x/../\evil.com, which have \ in the middle. Could we use strings.Contains(target, "\\") here?
| // instead of JSON. The Set-Cookie headers above still ride along on the | ||
| // redirect response, so a single navigation both establishes the cluster | ||
| // context and lands on the dashboard (e.g. /enter_cluster/...?redirect=/#/overview). | ||
| if redirect := r1.QueryParameter("redirect"); redirect != "" { |
There was a problem hiding this comment.
Should we move this check above the Set-Cookie calls, so cookies aren't updated when the redirect target is invalid?
3 participants
Why
Entering a cluster today requires two steps: hit the enter_cluster endpoint to establish the cluster context (via Set-Cookie), then separately navigate to the dashboard. This makes it awkward to hand out a single link that both sets context and lands the user on a useful page.
What
Adds an optional redirect query parameter to the enter_cluster endpoint (both the .../{name} and .../{name}/{session} routes).
When redirect is set, the handler responds with a 302 to the given path instead of the JSON body. The Set-Cookie headers still ride along on the redirect response, so a single navigation both establishes the cluster context and lands on the target page.
Example:
/enter_cluster////latest?redirect=/#/overview
When redirect is not set, behavior is unchanged — the endpoint still returns the existing JSON response.
Security
Redirect targets are validated by a new isSafeRedirectPath helper to prevent open-redirect attacks. Only same-origin, site-local absolute paths are allowed:
Unsafe targets are logged and rejected with a 400.
Testing