[history server] Add redirect capability to enter_cluster - #5086
[history server] Add redirect capability to enter_cluster#5086KunWuLuan wants to merge 1 commit into
Conversation
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).
![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.
| return false | ||
| } | ||
| return u.Scheme == "" && u.Host == "" | ||
| } |
There was a problem hiding this comment.
Open redirect validation bypass
High Severity
isSafeRedirectPath only rejects // and /\ as prefixes, so path-traversal plus a mid-string backslash (for example after one query decode of %5c) still passes. Browsers can normalize that into a protocol-relative Location, enabling an open redirect despite the safety check. Go 1.26’s http.Redirect hardening only encodes leading backslashes, so it does not close this gap.
Reviewed by Cursor Bugbot for commit d5089e4. Configure here.
| if !isSafeRedirectPath(redirect) { | ||
| logrus.Warnf("Rejecting unsafe redirect target: %q", redirect) | ||
| r2.WriteErrorString(http.StatusBadRequest, fmt.Sprintf("invalid redirect target: %q (must be a site-local path starting with '/')", redirect)) | ||
| return |
There was a problem hiding this comment.
Cookies set on rejected redirect
Low Severity
Unsafe redirect values are rejected with 400 only after cluster cookies are already written. Other error paths in this handler return before SetCookie, so a failed request can still change client cluster context.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit d5089e4. Configure here.
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