diff --git a/historyserver/pkg/historyserver/enter_cluster_test.go b/historyserver/pkg/historyserver/enter_cluster_test.go index 81a1b2f0932..5806f95313d 100644 --- a/historyserver/pkg/historyserver/enter_cluster_test.go +++ b/historyserver/pkg/historyserver/enter_cluster_test.go @@ -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()) + } + }) +} diff --git a/historyserver/pkg/historyserver/router.go b/historyserver/pkg/historyserver/router.go index b74282bf471..16ddf6d0ed0 100644 --- a/historyserver/pkg/historyserver/router.go +++ b/historyserver/pkg/historyserver/router.go @@ -10,6 +10,7 @@ import ( "math" "mime" "net/http" + "net/url" "path" "sort" "strconv" @@ -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 == "" +} + func routerRayClusterSet(s *ServerHandler) { ws := new(restful.WebService) defer restful.Add(ws) @@ -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 + } + http.Redirect(r2.ResponseWriter, r1.Request, redirect, http.StatusFound) + return + } + r2.WriteJson(map[string]interface{}{ "result": "success", "name": resolvedName, @@ -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) { @@ -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("")) }