Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 108 additions & 0 deletions historyserver/pkg/historyserver/enter_cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -555,3 +555,111 @@ func TestEnterClusterRayJobAndRayService(t *testing.T) {
}
})
}

func TestIsSafeRedirectPath(t *testing.T) {
cases := []struct {
target string
want bool
}{
{"/#/overview", true},
{"/", true},
{"/clusters?foo=bar", true},
{"", false},
{"relative/path", false},
{"//evil.com", false}, // protocol-relative
{"/\\evil.com", false}, // backslash variant
{"http://evil.com", false}, // absolute URL
{"https://evil.com/x", false}, // absolute URL
{"javascript:alert(1)", false}, // scheme, no leading slash
}
for _, tc := range cases {
if got := isSafeRedirectPath(tc.target); got != tc.want {
t.Errorf("isSafeRedirectPath(%q) = %v, want %v", tc.target, got, tc.want)
}
}
}

func TestEnterClusterRedirect(t *testing.T) {
restful.DefaultContainer = restful.NewContainer()

mockReader := &mockStorageReader{
clusters: []utils.ClusterInfo{
{
Namespace: "default",
Name: "cluster-a",
SessionName: "session_2026-04-22_10-00-00_000000_1",
OwnerKind: "rayjob",
OwnerName: "job-a",
},
},
}

scheme := runtime.NewScheme()
_ = rayv1.AddToScheme(scheme)
k8sClient := fake.NewClientBuilder().WithScheme(scheme).Build()
clientManager := &ClientManager{clients: []client.Client{k8sClient}}

handler := &ServerHandler{
maxClusters: 100,
reader: mockReader,
clientManager: clientManager,
}
fp := &fakeProcessor{
fn: func(ctx context.Context, info utils.ClusterInfo) (SessionStatus, *eventserver.SessionSnapshot, error) {
return SessionStatusProcessed, &eventserver.SessionSnapshot{}, nil
},
}
handler.sessionLoader = NewSessionLoader(fp, context.Background(), DefaultSessionProcessTimeout, DefaultSessionCacheSize, DefaultSessionCacheTTL)
routerRayClusterSet(handler)
container := restful.DefaultContainer

const validSession = "session_2026-04-22_10-00-00_000000_1"

t.Run("valid redirect returns 302 with cookies preserved", func(t *testing.T) {
req := httptest.NewRequest("GET", "/enter_cluster/default/raycluster/cluster-a/"+validSession+"?redirect=/%23/overview", nil)
resp := httptest.NewRecorder()
container.ServeHTTP(resp, req)

if resp.Code != http.StatusFound {
t.Fatalf("Expected status 302, got %d: %s", resp.Code, resp.Body.String())
}
if loc := resp.Header().Get("Location"); loc != "/#/overview" {
t.Errorf("Expected Location '/#/overview', got %q", loc)
}

// Cookies must still be set on the redirect response.
cookieMap := make(map[string]*http.Cookie)
for _, cookie := range resp.Result().Cookies() {
cookieMap[cookie.Name] = cookie
}
if c, ok := cookieMap[COOKIE_SESSION_NAME_KEY]; !ok || c.Value != validSession {
t.Errorf("Expected cookie %s to be %q, got %v", COOKIE_SESSION_NAME_KEY, validSession, c)
}
if c, ok := cookieMap[COOKIE_CLUSTER_NAME_KEY]; !ok || c.Value != "cluster-a" {
t.Errorf("Expected cookie %s to be 'cluster-a', got %v", COOKIE_CLUSTER_NAME_KEY, c)
}
})

t.Run("unsafe redirect is rejected with 400", func(t *testing.T) {
req := httptest.NewRequest("GET", "/enter_cluster/default/raycluster/cluster-a/"+validSession+"?redirect=https://evil.com", nil)
resp := httptest.NewRecorder()
container.ServeHTTP(resp, req)

if resp.Code != http.StatusBadRequest {
t.Fatalf("Expected status 400 for unsafe redirect, got %d: %s", resp.Code, resp.Body.String())
}
})

t.Run("no redirect param keeps JSON response", func(t *testing.T) {
req := httptest.NewRequest("GET", "/enter_cluster/default/raycluster/cluster-a/"+validSession, nil)
resp := httptest.NewRecorder()
container.ServeHTTP(resp, req)

if resp.Code != http.StatusOK {
t.Fatalf("Expected status 200, got %d: %s", resp.Code, resp.Body.String())
}
if !strings.Contains(resp.Body.String(), "\"result\"") {
t.Errorf("Expected JSON body with result field, got %q", resp.Body.String())
}
})
}
36 changes: 36 additions & 0 deletions historyserver/pkg/historyserver/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"math"
"mime"
"net/http"
"net/url"
"path"
"sort"
"strconv"
Expand Down Expand Up @@ -311,6 +312,25 @@ func routerLogical(s *ServerHandler) {

}

// isSafeRedirectPath reports whether target is a safe site-local redirect
// destination, guarding against open-redirect attacks. Only same-origin
// absolute paths are allowed: the value must start with a single "/", must not
// be protocol-relative ("//host") or a backslash variant ("/\host"), and must
// not carry a URL scheme or host component.
func isSafeRedirectPath(target string) bool {
if target == "" || target[0] != '/' {
return false
}
if strings.HasPrefix(target, "//") || strings.HasPrefix(target, "/\\") {
return false
}
u, err := url.Parse(target)
if err != nil {
return false
}
return u.Scheme == "" && u.Host == ""
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d5089e4. Configure here.


func routerRayClusterSet(s *ServerHandler) {
ws := new(restful.WebService)
defer restful.Add(ws)
Expand Down Expand Up @@ -363,6 +383,20 @@ func routerRayClusterSet(s *ServerHandler) {
http.SetCookie(r2, &http.Cookie{MaxAge: 600, Path: "/", Name: COOKIE_OWNER_KIND_KEY, Value: resolvedClusterInfo.OwnerKind})
http.SetCookie(r2, &http.Cookie{MaxAge: 600, Path: "/", Name: COOKIE_OWNER_NAME_KEY, Value: resolvedClusterInfo.OwnerName})

// When a "redirect" query parameter is supplied, respond with a 302 to it
// 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 != "" {
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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d5089e4. Configure here.

}
http.Redirect(r2.ResponseWriter, r1.Request, redirect, http.StatusFound)
return
}

r2.WriteJson(map[string]interface{}{
"result": "success",
"name": resolvedName,
Expand All @@ -381,6 +415,7 @@ func routerRayClusterSet(s *ServerHandler) {
Param(ws.PathParameter("namespace", "namespace")).
Param(ws.PathParameter("kind", "kind (raycluster, rayjob, or rayservice)")).
Param(ws.PathParameter("name", "name")).
Param(ws.QueryParameter("redirect", "optional site-local path (e.g. /#/overview); when set, respond with a 302 to it instead of JSON").DataType("string")).
Writes(""))

ws.Route(ws.GET("/{namespace}/{kind}/{name}/{session}").To(func(r1 *restful.Request, r2 *restful.Response) {
Expand All @@ -395,6 +430,7 @@ func routerRayClusterSet(s *ServerHandler) {
Param(ws.PathParameter("kind", "kind (raycluster, rayjob, or rayservice)")).
Param(ws.PathParameter("name", "name")).
Param(ws.PathParameter("session", "session")).
Param(ws.QueryParameter("redirect", "optional site-local path (e.g. /#/overview); when set, respond with a 302 to it instead of JSON").DataType("string")).
Writes(""))

}
Expand Down
Loading