From 4f18b06e782b31eb09dadb1416e92eb9c8f9ea2a Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Wed, 13 May 2026 20:35:59 +0530 Subject: [PATCH 1/5] feat: add session observability GET endpoints for sandboxes Signed-off-by: Abhinav Singh --- pkg/router/session_manager_test.go | 4 + pkg/store/interface.go | 2 + pkg/store/store_redis.go | 51 ++++ pkg/store/store_redis_test.go | 46 +++- pkg/store/store_valkey.go | 70 +++++- pkg/store/store_valkey_test.go | 80 +++++- pkg/workloadmanager/auth_test.go | 4 +- pkg/workloadmanager/client_cache_test.go | 2 +- .../garbage_collection_test.go | 3 + pkg/workloadmanager/handlers.go | 57 +++++ pkg/workloadmanager/handlers_test.go | 230 ++++++++++++++++++ pkg/workloadmanager/informers_test.go | 12 +- pkg/workloadmanager/sandbox_helper_test.go | 16 +- pkg/workloadmanager/server.go | 5 + pkg/workloadmanager/workload_builder.go | 2 +- 15 files changed, 556 insertions(+), 28 deletions(-) diff --git a/pkg/router/session_manager_test.go b/pkg/router/session_manager_test.go index 8740813b..23e24403 100644 --- a/pkg/router/session_manager_test.go +++ b/pkg/router/session_manager_test.go @@ -86,6 +86,10 @@ func (f *fakeStoreClient) ListExpiredSandboxes(_ context.Context, _ time.Time, _ return nil, nil } +func (f *fakeStoreClient) ListSandboxesByKind(_ context.Context, _ string) ([]*types.SandboxInfo, error) { + return nil, nil +} + func (f *fakeStoreClient) ListInactiveSandboxes(_ context.Context, _ time.Time, _ int64) ([]*types.SandboxInfo, error) { return nil, nil } diff --git a/pkg/store/interface.go b/pkg/store/interface.go index 72e74749..50137eb4 100644 --- a/pkg/store/interface.go +++ b/pkg/store/interface.go @@ -34,6 +34,8 @@ type Store interface { UpdateSandbox(ctx context.Context, sandboxStore *types.SandboxInfo) error // DeleteSandboxBySessionID delete sandbox by session ID DeleteSandboxBySessionID(ctx context.Context, sessionID string) error + // ListSandboxesByKind returns all active sandboxes matching the given kind. + ListSandboxesByKind(ctx context.Context, kind string) ([]*types.SandboxInfo, error) // ListExpiredSandboxes returns up to limit sandboxes with ExpiresAt before the given time ListExpiredSandboxes(ctx context.Context, before time.Time, limit int64) ([]*types.SandboxInfo, error) // ListInactiveSandboxes returns up to limit sandboxes with last-activity time before the given time diff --git a/pkg/store/store_redis.go b/pkg/store/store_redis.go index 13880f05..05e48720 100644 --- a/pkg/store/store_redis.go +++ b/pkg/store/store_redis.go @@ -143,6 +143,57 @@ func (rs *redisStore) GetSandboxBySessionID(ctx context.Context, sessionID strin return &sandboxRedis, nil } +// ListSandboxesByKind returns all active sandboxes matching the given kind. +// Uses SCAN to prevent blocking the Redis instance on large datasets. +func (rs *redisStore) ListSandboxesByKind(ctx context.Context, kind string) ([]*types.SandboxInfo, error) { + allSandboxes := make([]*types.SandboxInfo, 0) // Initialize as empty array, not nil + var cursor uint64 + matchPattern := rs.sessionPrefix + "*" + seenKeys := make(map[string]bool) + + for { + keys, nextCursor, err := rs.cli.Scan(ctx, cursor, matchPattern, 100).Result() + if err != nil { + return nil, fmt.Errorf("ListSandboxesByKind scan failed: %w", err) + } + + if len(keys) > 0 { + sessionIDs := make([]string, 0, len(keys)) + for _, key := range keys { + if key == rs.expiryIndexKey || key == rs.lastActivityIndexKey { + continue + } + if seenKeys[key] { + continue + } + seenKeys[key] = true + sessionIDs = append(sessionIDs, strings.TrimPrefix(key, rs.sessionPrefix)) + } + + if len(sessionIDs) > 0 { + // Batch load the fetched keys + sandboxes, err := rs.loadSandboxesBySessionIDs(ctx, sessionIDs) + if err != nil { + return nil, err + } + + // Filter by requested kind + for _, sb := range sandboxes { + if sb.Kind == kind { + allSandboxes = append(allSandboxes, sb) + } + } + } + } + + cursor = nextCursor + if cursor == 0 { + break // Scan complete + } + } + return allSandboxes, nil +} + func (rs *redisStore) StoreSandbox(ctx context.Context, sandboxRedis *types.SandboxInfo) error { if sandboxRedis == nil { return errors.New("StoreSandbox: sandbox is nil") diff --git a/pkg/store/store_redis_test.go b/pkg/store/store_redis_test.go index fd72071d..7bcb05a9 100644 --- a/pkg/store/store_redis_test.go +++ b/pkg/store/store_redis_test.go @@ -67,8 +67,8 @@ func newTestRedisClient(t *testing.T) (*redisStore, *miniredis.Miniredis) { rs := &redisStore{ cli: redisv9.NewClient(&redisv9.Options{Addr: mr.Addr()}), sessionPrefix: "session:", - expiryIndexKey: "sandbox:expiry", - lastActivityIndexKey: "sandbox:last_activity", + expiryIndexKey: "session:expiry", + lastActivityIndexKey: "session:last_activity", } return rs, mr } @@ -344,3 +344,45 @@ func TestUpdateSandboxLastActivity(t *testing.T) { t.Fatalf("unexpected lastActivity score after update: got %v, want %v", score, newLastActivity.Unix()) } } + +func TestRedisStore_ListSandboxesByKind(t *testing.T) { + ctx := context.Background() + c, _ := newTestRedisClient(t) + + now := time.Now().UTC() + sb1 := newTestSandbox("sb-1", "sess-1", now.Add(1*time.Hour)) + sb1.Kind = types.AgentRuntimeKind + + sb2 := newTestSandbox("sb-2", "sess-2", now.Add(1*time.Hour)) + sb2.Kind = types.CodeInterpreterKind + + sb3 := newTestSandbox("sb-3", "sess-3", now.Add(1*time.Hour)) + sb3.Kind = types.AgentRuntimeKind + + assert.NoError(t, c.StoreSandbox(ctx, sb1)) + assert.NoError(t, c.StoreSandbox(ctx, sb2)) + assert.NoError(t, c.StoreSandbox(ctx, sb3)) + + // List AgentRuntimeKind + list, err := c.ListSandboxesByKind(ctx, types.AgentRuntimeKind) + assert.NoError(t, err) + assert.Len(t, list, 2) + + ids := map[string]bool{} + for _, sb := range list { + ids[sb.SessionID] = true + } + assert.True(t, ids["sess-1"]) + assert.True(t, ids["sess-3"]) + + // List CodeInterpreterKind + list2, err := c.ListSandboxesByKind(ctx, types.CodeInterpreterKind) + assert.NoError(t, err) + assert.Len(t, list2, 1) + assert.Equal(t, "sess-2", list2[0].SessionID) + + // List unknown kind + list3, err := c.ListSandboxesByKind(ctx, "unknown-kind") + assert.NoError(t, err) + assert.Len(t, list3, 0) +} diff --git a/pkg/store/store_valkey.go b/pkg/store/store_valkey.go index 5e68b20d..46b20e93 100644 --- a/pkg/store/store_valkey.go +++ b/pkg/store/store_valkey.go @@ -111,19 +111,26 @@ func (vs *valkeyStore) loadSandboxesBySessionIDs(ctx context.Context, sessionIDs sessionIDKeys = append(sessionIDKeys, vs.sessionKey(sessionID)) } // MGet should in same slot - stingSliceResults, err := vs.cli.Do(ctx, vs.cli.B().Mget().Key(sessionIDKeys...).Build()).AsStrSlice() + valkeyResults, err := vs.cli.Do(ctx, vs.cli.B().Mget().Key(sessionIDKeys...).Build()).ToArray() if err != nil { return nil, fmt.Errorf("loadSandboxesBySessionIDs: Valkey MGet sandboxes failed: %w", err) } - if len(stingSliceResults) > len(sessionIDKeys) { - return nil, fmt.Errorf("unexpected MGet result size: %d, param size: %d", len(stingSliceResults), len(sessionIDKeys)) + if len(valkeyResults) != len(sessionIDKeys) { + return nil, fmt.Errorf("unexpected MGet result size: %d, expected: %d", len(valkeyResults), len(sessionIDKeys)) } - sandboxResults := make([]*types.SandboxInfo, 0, len(stingSliceResults)) - for i, sandboxObjString := range stingSliceResults { + sandboxResults := make([]*types.SandboxInfo, 0, len(valkeyResults)) + for i, msg := range valkeyResults { + if msg.IsNil() { + // key does not exist, ignore + continue + } + sandboxObjString, err := msg.ToString() + if err != nil { + return nil, fmt.Errorf("parse sandbox string failed: %w, index: %v, sessionID: %v", err, i, sessionIDs[i]) + } if len(sandboxObjString) == 0 { - // sandboxObjString is empty while sessionKey not exist, ignore continue } var sandboxRedis types.SandboxInfo @@ -168,6 +175,57 @@ func (vs *valkeyStore) GetSandboxBySessionID(ctx context.Context, sessionID stri return &sandboxRedis, nil } +// ListSandboxesByKind returns all active sandboxes matching the given kind. +// Uses SCAN to prevent blocking the Valkey instance on large datasets. +func (vs *valkeyStore) ListSandboxesByKind(ctx context.Context, kind string) ([]*types.SandboxInfo, error) { + allSandboxes := make([]*types.SandboxInfo, 0) // Initialize as empty array, not nil + cursor := uint64(0) + matchPattern := vs.sessionPrefix + "*" + seenKeys := make(map[string]bool) + + for { + scanRes, err := vs.cli.Do(ctx, vs.cli.B().Scan().Cursor(cursor).Match(matchPattern).Count(100).Build()).AsScanEntry() + if err != nil { + return nil, fmt.Errorf("ListSandboxesByKind scan failed: %w", err) + } + + if len(scanRes.Elements) > 0 { + sessionIDs := make([]string, 0, len(scanRes.Elements)) + for _, key := range scanRes.Elements { + if key == vs.expiryIndexKey || key == vs.lastActivityIndexKey { + continue + } + if seenKeys[key] { + continue + } + seenKeys[key] = true + sessionIDs = append(sessionIDs, strings.TrimPrefix(key, vs.sessionPrefix)) + } + + if len(sessionIDs) > 0 { + // Batch load the fetched keys + sandboxes, err := vs.loadSandboxesBySessionIDs(ctx, sessionIDs) + if err != nil { + return nil, err + } + + // Filter by requested kind + for _, sb := range sandboxes { + if sb.Kind == kind { + allSandboxes = append(allSandboxes, sb) + } + } + } + } + + cursor = scanRes.Cursor + if cursor == 0 { + break // Scan complete + } + } + return allSandboxes, nil +} + // StoreSandbox store sandbox into storage func (vs *valkeyStore) StoreSandbox(ctx context.Context, sandboxStore *types.SandboxInfo) error { if sandboxStore == nil { diff --git a/pkg/store/store_valkey_test.go b/pkg/store/store_valkey_test.go index 122d91f6..4a456dc2 100644 --- a/pkg/store/store_valkey_test.go +++ b/pkg/store/store_valkey_test.go @@ -135,8 +135,8 @@ func newValkeyTestClient(t *testing.T) (*valkeyStore, *miniredis.Miniredis) { rs := &valkeyStore{ cli: client, sessionPrefix: "session:", - expiryIndexKey: "sandbox:expiry", - lastActivityIndexKey: "sandbox:last_activity", + expiryIndexKey: "session:expiry", + lastActivityIndexKey: "session:last_activity", } return rs, mr } @@ -360,3 +360,79 @@ func TestValkeyStore_UpdateSandboxLastActivity(t *testing.T) { assert.Error(t, err) assert.True(t, errors.Is(err, ErrNotFound)) } + +func TestValkeyStore_ListSandboxesByKind(t *testing.T) { + ctx := context.Background() + c, _ := newValkeyTestClient(t) + + now := time.Now().UTC() + sb1 := newTestSandbox("sb-1", "sess-1", now.Add(1*time.Hour)) + sb1.Kind = types.AgentRuntimeKind + + sb2 := newTestSandbox("sb-2", "sess-2", now.Add(1*time.Hour)) + sb2.Kind = types.CodeInterpreterKind + + sb3 := newTestSandbox("sb-3", "sess-3", now.Add(1*time.Hour)) + sb3.Kind = types.AgentRuntimeKind + + assert.NoError(t, c.StoreSandbox(ctx, sb1)) + assert.NoError(t, c.StoreSandbox(ctx, sb2)) + assert.NoError(t, c.StoreSandbox(ctx, sb3)) + + // List AgentRuntimeKind + list, err := c.ListSandboxesByKind(ctx, types.AgentRuntimeKind) + assert.NoError(t, err) + assert.Len(t, list, 2) + + ids := map[string]bool{} + for _, sb := range list { + ids[sb.SessionID] = true + } + assert.True(t, ids["sess-1"]) + assert.True(t, ids["sess-3"]) + + // List CodeInterpreterKind + list2, err := c.ListSandboxesByKind(ctx, types.CodeInterpreterKind) + assert.NoError(t, err) + assert.Len(t, list2, 1) + assert.Equal(t, "sess-2", list2[0].SessionID) + + // List unknown kind + list3, err := c.ListSandboxesByKind(ctx, "unknown-kind") + assert.NoError(t, err) + assert.Len(t, list3, 0) +} + +// TestValkeyStore_LoadSandboxesBySessionIDs_OrphanedZSetEntry verifies that +// loadSandboxesBySessionIDs skips session IDs whose hash key has been evicted +// from Valkey (orphaned sorted-set entry) instead of aborting the entire batch. +func TestValkeyStore_LoadSandboxesBySessionIDs_OrphanedZSetEntry(t *testing.T) { + ctx := context.Background() + c, mr := newValkeyTestClient(t) + + now := time.Now().UTC().Truncate(time.Second) + + sb1 := newTestSandbox("sb-orphan", "sess-orphan", now.Add(-1*time.Hour)) + sb2 := newTestSandbox("sb-alive", "sess-alive", now.Add(-2*time.Hour)) + + if err := c.StoreSandbox(ctx, sb1); err != nil { + t.Fatalf("StoreSandbox sb1 error: %v", err) + } + if err := c.StoreSandbox(ctx, sb2); err != nil { + t.Fatalf("StoreSandbox sb2 error: %v", err) + } + + // Simulate Valkey evicting the hash key for sb1 while leaving its zset entry. + mr.Del(c.sessionKey("sess-orphan")) + + result, err := c.loadSandboxesBySessionIDs(ctx, []string{"sess-orphan", "sess-alive"}) + if err != nil { + t.Fatalf("expected no error with orphaned zset entry, got: %v", err) + } + if len(result) != 1 { + t.Fatalf("expected 1 sandbox (the non-evicted one), got %d", len(result)) + } + if result[0].SandboxID != "sb-alive" { + t.Fatalf("expected sb-alive, got %s", result[0].SandboxID) + } +} diff --git a/pkg/workloadmanager/auth_test.go b/pkg/workloadmanager/auth_test.go index 97a30cc0..34a3d574 100644 --- a/pkg/workloadmanager/auth_test.go +++ b/pkg/workloadmanager/auth_test.go @@ -31,7 +31,7 @@ import ( ) const ( - testToken = "test-token" + testToken = "test-token" testServiceAccount = "system:serviceaccount:default:test-sa" ) @@ -85,7 +85,7 @@ func TestAuthMiddleware_InvalidHeaderFormat(t *testing.T) { name: "no Bearer prefix", header: "token123", expectedBodyPart: "Invalid authorization header format", - }, + }, { name: "wrong prefix", header: "Basic token123", diff --git a/pkg/workloadmanager/client_cache_test.go b/pkg/workloadmanager/client_cache_test.go index e6d75e06..c0678d9f 100644 --- a/pkg/workloadmanager/client_cache_test.go +++ b/pkg/workloadmanager/client_cache_test.go @@ -28,7 +28,7 @@ import ( ) const ( - jwtHeader = `{"alg":"HS256","typ":"JWT"}` + jwtHeader = `{"alg":"HS256","typ":"JWT"}` testCacheKey = "default:test-sa" ) diff --git a/pkg/workloadmanager/garbage_collection_test.go b/pkg/workloadmanager/garbage_collection_test.go index 531e9908..e2d28036 100644 --- a/pkg/workloadmanager/garbage_collection_test.go +++ b/pkg/workloadmanager/garbage_collection_test.go @@ -46,6 +46,9 @@ func (nopStore) DeleteSandboxBySessionID(_ context.Context, _ string) error { r func (nopStore) ListExpiredSandboxes(_ context.Context, _ time.Time, _ int64) ([]*types.SandboxInfo, error) { return nil, nil } +func (nopStore) ListSandboxesByKind(_ context.Context, _ string) ([]*types.SandboxInfo, error) { + return nil, nil +} func (nopStore) ListInactiveSandboxes(_ context.Context, _ time.Time, _ int64) ([]*types.SandboxInfo, error) { return nil, nil } diff --git a/pkg/workloadmanager/handlers.go b/pkg/workloadmanager/handlers.go index 7d417c6e..87b5b6cb 100644 --- a/pkg/workloadmanager/handlers.go +++ b/pkg/workloadmanager/handlers.go @@ -308,6 +308,63 @@ func (s *Server) rollbackSandboxCreation(dynamicClient dynamic.Interface, sandbo } } +// handleGetSandbox handles requests to retrieve sandbox details by session ID +func (s *Server) handleGetSandbox(c *gin.Context, kind string) { + sessionID := c.Param("sessionId") + // Query sandbox from store + sandboxInfo, err := s.storeClient.GetSandboxBySessionID(c.Request.Context(), sessionID) + if err != nil { + if errors.Is(err, store.ErrNotFound) { + respondError(c, http.StatusNotFound, fmt.Sprintf("Session ID %s not found", sessionID)) + return + } + klog.Errorf("get sandbox from store by sessionID %s failed: %v", sessionID, err) + respondError(c, http.StatusInternalServerError, "internal server error") + return + } + + if sandboxInfo.Kind != kind { + respondError(c, http.StatusNotFound, fmt.Sprintf("Session ID %s not found for kind %s", sessionID, kind)) + return + } + + if s.config.EnableAuth { + _, userNamespace, _, _ := extractUserInfo(c) + if sandboxInfo.SandboxNamespace != userNamespace { + klog.Warningf("unauthorized GET attempt to session %s by user in namespace %s", sessionID, userNamespace) + respondError(c, http.StatusForbidden, "access denied to this session") + return + } + } + + respondJSON(c, http.StatusOK, sandboxInfo) +} + +// handleListSandboxes handles requests to list all active sandboxes by kind +func (s *Server) handleListSandboxes(c *gin.Context, kind string) { + sandboxes, err := s.storeClient.ListSandboxesByKind(c.Request.Context(), kind) + if err != nil { + klog.Errorf("list sandboxes of kind %s failed: %v", kind, err) + respondError(c, http.StatusInternalServerError, "internal server error") + return + } + + if !s.config.EnableAuth { + respondJSON(c, http.StatusOK, sandboxes) + return + } + + _, userNamespace, _, _ := extractUserInfo(c) + // Filter in-place to avoid a new allocation. + filtered := sandboxes[:0] + for _, sb := range sandboxes { + if sb.SandboxNamespace == userNamespace { + filtered = append(filtered, sb) + } + } + respondJSON(c, http.StatusOK, filtered) +} + // handleDeleteSandbox handles sandbox deletion requests func (s *Server) handleDeleteSandbox(c *gin.Context) { sessionID := c.Param("sessionId") diff --git a/pkg/workloadmanager/handlers_test.go b/pkg/workloadmanager/handlers_test.go index a71aa124..d1367ece 100644 --- a/pkg/workloadmanager/handlers_test.go +++ b/pkg/workloadmanager/handlers_test.go @@ -63,6 +63,9 @@ func (f *fakeStore) UpdateSandbox(_ context.Context, _ *types.SandboxInfo) error return f.updateErr } func (f *fakeStore) DeleteSandboxBySessionID(_ context.Context, _ string) error { return nil } +func (f *fakeStore) ListSandboxesByKind(_ context.Context, _ string) ([]*types.SandboxInfo, error) { + return nil, nil +} func (f *fakeStore) ListExpiredSandboxes(_ context.Context, _ time.Time, _ int64) ([]*types.SandboxInfo, error) { return nil, nil } @@ -476,6 +479,233 @@ func TestHandleSandboxCreate(t *testing.T) { } } +func TestHandleGetSandbox(t *testing.T) { + gin.SetMode(gin.TestMode) + + tests := []struct { + name string + sessionID string + storeErr error + storeResult *types.SandboxInfo + enableAuth bool + userNamespace string + expectStatus int + expectMessage string + }{ + { + name: "success without auth", + sessionID: "sess-123", + storeResult: &types.SandboxInfo{ + SessionID: "sess-123", + SandboxNamespace: "default", + Name: "sandbox-1", + Kind: types.AgentRuntimeKind, + }, + expectStatus: http.StatusOK, + }, + { + name: "not found", + sessionID: "sess-unknown", + storeErr: store.ErrNotFound, + expectStatus: http.StatusNotFound, + expectMessage: "Session ID sess-unknown not found", + }, + { + name: "internal store error", + sessionID: "sess-error", + storeErr: errors.New("db down"), + expectStatus: http.StatusInternalServerError, + expectMessage: "internal server error", + }, + { + name: "success with auth and matching namespace", + sessionID: "sess-auth-ok", + enableAuth: true, + userNamespace: "user-ns", + storeResult: &types.SandboxInfo{ + SessionID: "sess-auth-ok", + SandboxNamespace: "user-ns", + Kind: types.AgentRuntimeKind, + }, + expectStatus: http.StatusOK, + }, + { + name: "forbidden with auth and mismatched namespace", + sessionID: "sess-auth-fail", + enableAuth: true, + userNamespace: "hacker-ns", + storeResult: &types.SandboxInfo{ + SessionID: "sess-auth-fail", + SandboxNamespace: "victim-ns", + Kind: types.AgentRuntimeKind, + }, + expectStatus: http.StatusForbidden, + expectMessage: "access denied to this session", + }, + { + name: "wrong kind", + sessionID: "sess-wrong-kind", + storeResult: &types.SandboxInfo{ + SessionID: "sess-wrong-kind", + SandboxNamespace: "default", + Kind: types.CodeInterpreterKind, + }, + expectStatus: http.StatusNotFound, + expectMessage: "Session ID sess-wrong-kind not found for kind AgentRuntime", + }, + } + + for _, tt := range tests { + tc := tt + t.Run(tc.name, func(t *testing.T) { + fakeServer := newFakeServer() + fakeServer.config.EnableAuth = tc.enableAuth + + fakeStoreInst := &fakeStore{} + fakeServer.storeClient = fakeStoreInst + + patches := gomonkey.NewPatches() + defer patches.Reset() + + patches.ApplyMethod(reflect.TypeOf((*fakeStore)(nil)), "GetSandboxBySessionID", func(_ *fakeStore, _ context.Context, _ string) (*types.SandboxInfo, error) { + if tc.storeErr != nil { + return nil, tc.storeErr + } + return tc.storeResult, nil + }) + + if tc.enableAuth { + // Mock extractUserInfo to simulate the authenticated user's namespace + patches.ApplyFunc(extractUserInfo, func(_ *gin.Context) (string, string, string, string) { + return "mock-token", tc.userNamespace, "mock-project", "mock-sa" + }) + } + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + + req := httptest.NewRequest(http.MethodGet, "/sessions/"+tc.sessionID, nil) + c.Request = req + c.Params = gin.Params{{Key: "sessionId", Value: tc.sessionID}} + + fakeServer.handleGetSandbox(c, types.AgentRuntimeKind) + + require.Equal(t, tc.expectStatus, w.Code) + + if tc.expectStatus != http.StatusOK { + if tc.expectMessage != "" { + var errResp ErrorResponse + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &errResp)) + require.Equal(t, tc.expectMessage, errResp.Message) + } + return + } + + var resp types.SandboxInfo + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp)) + require.Equal(t, tc.storeResult.SessionID, resp.SessionID) + require.Equal(t, tc.storeResult.SandboxNamespace, resp.SandboxNamespace) + }) + } +} + +func TestHandleListSandboxes(t *testing.T) { + gin.SetMode(gin.TestMode) + + tests := []struct { + name string + kind string + storeErr error + storeResult []*types.SandboxInfo + enableAuth bool + userNamespace string + expectStatus int + expectMessage string + expectCount int + }{ + { + name: "success without auth", + kind: types.AgentRuntimeKind, + storeResult: []*types.SandboxInfo{ + {SessionID: "1", SandboxNamespace: "default", Kind: types.AgentRuntimeKind}, + {SessionID: "2", SandboxNamespace: "test-ns", Kind: types.AgentRuntimeKind}, + }, + expectStatus: http.StatusOK, + expectCount: 2, + }, + { + name: "internal store error", + kind: types.AgentRuntimeKind, + storeErr: errors.New("db down"), + expectStatus: http.StatusInternalServerError, + expectMessage: "internal server error", + }, + { + name: "success with auth filters by namespace", + kind: types.AgentRuntimeKind, + enableAuth: true, + userNamespace: "user-ns", + storeResult: []*types.SandboxInfo{ + {SessionID: "1", SandboxNamespace: "user-ns", Kind: types.AgentRuntimeKind}, + {SessionID: "2", SandboxNamespace: "other-ns", Kind: types.AgentRuntimeKind}, + }, + expectStatus: http.StatusOK, + expectCount: 1, + }, + } + + for _, tt := range tests { + tc := tt + t.Run(tc.name, func(t *testing.T) { + fakeServer := newFakeServer() + fakeServer.config.EnableAuth = tc.enableAuth + + fakeStoreInst := &fakeStore{} + fakeServer.storeClient = fakeStoreInst + + patches := gomonkey.NewPatches() + defer patches.Reset() + + patches.ApplyMethod(reflect.TypeOf((*fakeStore)(nil)), "ListSandboxesByKind", func(_ *fakeStore, _ context.Context, _ string) ([]*types.SandboxInfo, error) { + if tc.storeErr != nil { + return nil, tc.storeErr + } + return tc.storeResult, nil + }) + + if tc.enableAuth { + patches.ApplyFunc(extractUserInfo, func(_ *gin.Context) (string, string, string, string) { + return "mock-token", tc.userNamespace, "mock-project", "mock-sa" + }) + } + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + + req := httptest.NewRequest(http.MethodGet, "/sessions", nil) + c.Request = req + + fakeServer.handleListSandboxes(c, tc.kind) + + require.Equal(t, tc.expectStatus, w.Code) + + if tc.expectStatus != http.StatusOK { + var errResp ErrorResponse + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &errResp)) + require.Equal(t, tc.expectMessage, errResp.Message) + return + } + + var resp []*types.SandboxInfo + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp)) + require.Len(t, resp, tc.expectCount) + if tc.enableAuth && tc.expectCount > 0 { + require.Equal(t, tc.userNamespace, resp[0].SandboxNamespace) + } + }) + } +} + // This test verifies that the deleteSandbox handler correctly handles scenarios where the client disconnects (Context Cancellation) before the deletion operation completes. // // Key Points: diff --git a/pkg/workloadmanager/informers_test.go b/pkg/workloadmanager/informers_test.go index 993e914f..c3afa016 100644 --- a/pkg/workloadmanager/informers_test.go +++ b/pkg/workloadmanager/informers_test.go @@ -32,7 +32,7 @@ type neverSyncedInformer struct { cache.SharedIndexInformer } -func (n *neverSyncedInformer) HasSynced() bool { return false } +func (n *neverSyncedInformer) HasSynced() bool { return false } func (n *neverSyncedInformer) Run(stopCh <-chan struct{}) { <-stopCh } // alwaysSyncedInformer is a cache.SharedIndexInformer whose HasSynced always returns true. @@ -40,7 +40,7 @@ type alwaysSyncedInformer struct { cache.SharedIndexInformer } -func (a *alwaysSyncedInformer) HasSynced() bool { return true } +func (a *alwaysSyncedInformer) HasSynced() bool { return true } func (a *alwaysSyncedInformer) Run(stopCh <-chan struct{}) { <-stopCh } // runCanceled starts RunAndWaitForCacheSync in a goroutine, cancels the context @@ -69,10 +69,10 @@ func TestRunAndWaitForCacheSync_ContextCancellation(t *testing.T) { always := func() cache.SharedIndexInformer { return &alwaysSyncedInformer{} } tests := []struct { - name string - agentRuntime cache.SharedIndexInformer - codeInterpreter cache.SharedIndexInformer - pod cache.SharedIndexInformer + name string + agentRuntime cache.SharedIndexInformer + codeInterpreter cache.SharedIndexInformer + pod cache.SharedIndexInformer }{ { name: "AgentRuntimeInformer never syncs", diff --git a/pkg/workloadmanager/sandbox_helper_test.go b/pkg/workloadmanager/sandbox_helper_test.go index 5106678b..793108ad 100644 --- a/pkg/workloadmanager/sandbox_helper_test.go +++ b/pkg/workloadmanager/sandbox_helper_test.go @@ -346,7 +346,7 @@ func TestGetSandboxStatus_TableDriven(t *testing.T) { }, }, }, - expected: "ready", + expected: "ready", }, { name: "ready condition false without reason", @@ -360,7 +360,7 @@ func TestGetSandboxStatus_TableDriven(t *testing.T) { }, }, }, - expected: "not-ready", + expected: "not-ready", }, { name: "ready condition false with reason is not-ready", @@ -376,7 +376,7 @@ func TestGetSandboxStatus_TableDriven(t *testing.T) { }, }, }, - expected: "not-ready", + expected: "not-ready", }, { name: "ready condition unknown", @@ -390,7 +390,7 @@ func TestGetSandboxStatus_TableDriven(t *testing.T) { }, }, }, - expected: "not-ready", + expected: "not-ready", }, { name: "no conditions", @@ -399,7 +399,7 @@ func TestGetSandboxStatus_TableDriven(t *testing.T) { Conditions: []metav1.Condition{}, }, }, - expected: "not-ready", + expected: "not-ready", }, { name: "nil conditions", @@ -408,7 +408,7 @@ func TestGetSandboxStatus_TableDriven(t *testing.T) { Conditions: nil, }, }, - expected: "not-ready", + expected: "not-ready", }, { name: "other condition type", @@ -422,7 +422,7 @@ func TestGetSandboxStatus_TableDriven(t *testing.T) { }, }, }, - expected: "not-ready", + expected: "not-ready", }, { name: "multiple conditions with ready true", @@ -440,7 +440,7 @@ func TestGetSandboxStatus_TableDriven(t *testing.T) { }, }, }, - expected: "ready", + expected: "ready", }, } diff --git a/pkg/workloadmanager/server.go b/pkg/workloadmanager/server.go index 53b10dfb..571ea843 100644 --- a/pkg/workloadmanager/server.go +++ b/pkg/workloadmanager/server.go @@ -26,6 +26,7 @@ import ( "github.com/gin-gonic/gin" "k8s.io/klog/v2" + "github.com/volcano-sh/agentcube/pkg/common/types" "github.com/volcano-sh/agentcube/pkg/store" "golang.org/x/net/http2" "golang.org/x/net/http2/h2c" @@ -119,9 +120,13 @@ func (s *Server) setupRoutes() { // agent runtime management endpoints v1Group.POST("/agent-runtime", s.handleAgentRuntimeCreate) + v1Group.GET("/agent-runtime/sessions", func(c *gin.Context) { s.handleListSandboxes(c, types.AgentRuntimeKind) }) + v1Group.GET("/agent-runtime/sessions/:sessionId", func(c *gin.Context) { s.handleGetSandbox(c, types.AgentRuntimeKind) }) v1Group.DELETE("/agent-runtime/sessions/:sessionId", s.handleDeleteSandbox) // code interpreter management endpoints v1Group.POST("/code-interpreter", s.handleCodeInterpreterCreate) + v1Group.GET("/code-interpreter/sessions", func(c *gin.Context) { s.handleListSandboxes(c, types.CodeInterpreterKind) }) + v1Group.GET("/code-interpreter/sessions/:sessionId", func(c *gin.Context) { s.handleGetSandbox(c, types.CodeInterpreterKind) }) v1Group.DELETE("/code-interpreter/sessions/:sessionId", s.handleDeleteSandbox) } diff --git a/pkg/workloadmanager/workload_builder.go b/pkg/workloadmanager/workload_builder.go index 4abe59de..62412893 100644 --- a/pkg/workloadmanager/workload_builder.go +++ b/pkg/workloadmanager/workload_builder.go @@ -184,7 +184,7 @@ func buildSandboxObject(params *buildSandboxParams) *sandboxv1alpha1.Sandbox { Labels: map[string]string{ SessionIdLabelKey: params.sessionID, WorkloadNameLabelKey: params.workloadName, - "managed-by": "agentcube-workload-manager", + "managed-by": "agentcube-workload-manager", }, Annotations: map[string]string{ IdleTimeoutAnnotationKey: params.idleTimeout.String(), From f6bcafcc01baa1189341696788587b0fbc57466f Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Tue, 19 May 2026 16:10:08 +0530 Subject: [PATCH 2/5] test: add memory footprint note for seenKeys per review Signed-off-by: Abhinav Singh --- pkg/store/store_redis.go | 4 ++++ pkg/store/store_valkey.go | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/pkg/store/store_redis.go b/pkg/store/store_redis.go index 05e48720..149f6fe5 100644 --- a/pkg/store/store_redis.go +++ b/pkg/store/store_redis.go @@ -149,6 +149,10 @@ func (rs *redisStore) ListSandboxesByKind(ctx context.Context, kind string) ([]* allSandboxes := make([]*types.SandboxInfo, 0) // Initialize as empty array, not nil var cursor uint64 matchPattern := rs.sessionPrefix + "*" + // Note: seenKeys holds all scanned keys in memory to deduplicate results across all SCAN pages, + // as SCAN can return the same key multiple times. For extremely large datasets, this could + // have a memory impact, and a dedicated secondary index (e.g. Redis SET per kind) is recommended + // as a follow-up optimization. seenKeys := make(map[string]bool) for { diff --git a/pkg/store/store_valkey.go b/pkg/store/store_valkey.go index 46b20e93..9e56b6b9 100644 --- a/pkg/store/store_valkey.go +++ b/pkg/store/store_valkey.go @@ -181,6 +181,10 @@ func (vs *valkeyStore) ListSandboxesByKind(ctx context.Context, kind string) ([] allSandboxes := make([]*types.SandboxInfo, 0) // Initialize as empty array, not nil cursor := uint64(0) matchPattern := vs.sessionPrefix + "*" + // Note: seenKeys holds all scanned keys in memory to deduplicate results across all SCAN pages, + // as SCAN can return the same key multiple times. For extremely large datasets, this could + // have a memory impact, and a dedicated secondary index (e.g. Valkey SET per kind) is recommended + // as a follow-up optimization. seenKeys := make(map[string]bool) for { From 0eeb2c717adf5ed0275746f8cf243da5d72763e9 Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Tue, 19 May 2026 16:36:57 +0530 Subject: [PATCH 3/5] test: fix flaky RSA key comparison in TestGetPrivateKeyPEM Signed-off-by: Abhinav Singh --- pkg/router/jwt_test.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/pkg/router/jwt_test.go b/pkg/router/jwt_test.go index aef69981..dc09bb9f 100644 --- a/pkg/router/jwt_test.go +++ b/pkg/router/jwt_test.go @@ -183,7 +183,16 @@ func TestGetPrivateKeyPEM(t *testing.T) { privateKey, err := x509.ParsePKCS1PrivateKey(block.Bytes) assert.NoError(t, err) assert.NotNil(t, privateKey) - assert.Equal(t, manager.privateKey, privateKey) + // Compare keys mathematically instead of by object equality + assert.Equal(t, manager.privateKey.PublicKey.N, privateKey.PublicKey.N, "Public key N should match") + assert.Equal(t, manager.privateKey.PublicKey.E, privateKey.PublicKey.E, "Public key E should match") + assert.Equal(t, 0, manager.privateKey.D.Cmp(privateKey.D), "Private exponent D should match") + + // Compare primes + assert.Equal(t, len(manager.privateKey.Primes), len(privateKey.Primes), "Number of primes should match") + for i := range manager.privateKey.Primes { + assert.Equal(t, 0, manager.privateKey.Primes[i].Cmp(privateKey.Primes[i]), "Prime %d should match", i) + } } func TestLoadPrivateKeyPEM(t *testing.T) { From 92f070ca0a5cd6c815abc385b00a38144f1c4567 Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Tue, 19 May 2026 16:49:09 +0530 Subject: [PATCH 4/5] test: update TestHandleGetSandbox to expect 404 instead of 403 on namespace mismatch Signed-off-by: Abhinav Singh --- pkg/workloadmanager/handlers.go | 2 +- pkg/workloadmanager/handlers_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/workloadmanager/handlers.go b/pkg/workloadmanager/handlers.go index 87b5b6cb..efbb3954 100644 --- a/pkg/workloadmanager/handlers.go +++ b/pkg/workloadmanager/handlers.go @@ -332,7 +332,7 @@ func (s *Server) handleGetSandbox(c *gin.Context, kind string) { _, userNamespace, _, _ := extractUserInfo(c) if sandboxInfo.SandboxNamespace != userNamespace { klog.Warningf("unauthorized GET attempt to session %s by user in namespace %s", sessionID, userNamespace) - respondError(c, http.StatusForbidden, "access denied to this session") + respondError(c, http.StatusNotFound, fmt.Sprintf("Session ID %s not found", sessionID)) return } } diff --git a/pkg/workloadmanager/handlers_test.go b/pkg/workloadmanager/handlers_test.go index d1367ece..c108d74a 100644 --- a/pkg/workloadmanager/handlers_test.go +++ b/pkg/workloadmanager/handlers_test.go @@ -539,8 +539,8 @@ func TestHandleGetSandbox(t *testing.T) { SandboxNamespace: "victim-ns", Kind: types.AgentRuntimeKind, }, - expectStatus: http.StatusForbidden, - expectMessage: "access denied to this session", + expectStatus: http.StatusNotFound, + expectMessage: "Session ID sess-auth-fail not found", }, { name: "wrong kind", From 0b47b4f6873f42177da47b7d3677678d5eb8618e Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Mon, 25 May 2026 15:56:58 +0530 Subject: [PATCH 5/5] auth: enforce per-service-account ownership on GET session endpoints Signed-off-by: Abhinav Singh --- pkg/common/types/sandbox.go | 4 +++ pkg/workloadmanager/auth.go | 16 ++++++---- pkg/workloadmanager/handlers.go | 32 +++++++++++++++----- pkg/workloadmanager/handlers_test.go | 34 ++++++++++++++++++++-- pkg/workloadmanager/sandbox_helper.go | 6 ++-- pkg/workloadmanager/sandbox_helper_test.go | 4 +-- 6 files changed, 77 insertions(+), 19 deletions(-) diff --git a/pkg/common/types/sandbox.go b/pkg/common/types/sandbox.go index b829f780..e0b4db21 100644 --- a/pkg/common/types/sandbox.go +++ b/pkg/common/types/sandbox.go @@ -42,6 +42,10 @@ type SandboxInfo struct { // during ListInactiveSandboxes. It is intentionally excluded from JSON serialization. LastActivityAt time.Time `json:"-"` Status string `json:"status"` + // CreatedBy holds the Kubernetes service account name (without the namespace prefix) + // that created this sandbox. It is used by the GET handlers to enforce per-service-account + // ownership: only the creating SA can retrieve its own session entrypoints. + CreatedBy string `json:"createdBy,omitempty"` } type SandboxEntryPoint struct { diff --git a/pkg/workloadmanager/auth.go b/pkg/workloadmanager/auth.go index d501d86e..487a6cce 100644 --- a/pkg/workloadmanager/auth.go +++ b/pkg/workloadmanager/auth.go @@ -47,18 +47,24 @@ import ( // // POST /v1/sandboxes (CreateSandbox): // - Any authenticated user can create sandboxes -// - The creator's service account name is stored in the sandbox metadata +// - The creator's service account name is stamped into SandboxInfo.CreatedBy +// and persisted in the store at creation time // // GET /v1/sandboxes (ListSandboxes): -// - Users can only list sandboxes they created +// - Users can only list sandboxes they created (CreatedBy == serviceAccountName) +// - For backward compatibility, sandboxes without a CreatedBy field are filtered +// by namespace only (entries created before this field was introduced) // // GET /v1/sandboxes/{sandboxId} (GetSandbox): -// - Users can only access sandboxes they created -// - Access is checked via checkSandboxAccess() function +// - Users can only access sandboxes they created (CreatedBy == serviceAccountName) +// - Access is first checked by namespace, then by CreatedBy +// - Returns 404 (not 403) for unauthorized or missing sessions to prevent +// session ID enumeration attacks // // DELETE /v1/sandboxes/{sandboxId} (DeleteSandbox): // - Users can only delete sandboxes they created -// - Access is checked via checkSandboxAccess() function +// - Access is enforced by Kubernetes RBAC via the user's own dynamicClient; +// the Kubernetes API itself rejects cross-account deletes // // CONNECT /v1/sandboxes/{sandboxId} (Tunnel): // - Users can only establish tunnels to sandboxes they created diff --git a/pkg/workloadmanager/handlers.go b/pkg/workloadmanager/handlers.go index efbb3954..22236f21 100644 --- a/pkg/workloadmanager/handlers.go +++ b/pkg/workloadmanager/handlers.go @@ -125,6 +125,7 @@ func (s *Server) handleSandboxCreate(c *gin.Context, kind string) { namespace := sandbox.Namespace dynamicClient := s.k8sClient.dynamicClient + var createdBy string if s.config.EnableAuth { userDynamicClient, errExtractClient := s.extractUserK8sClient(c) if errExtractClient != nil { @@ -133,6 +134,7 @@ func (s *Server) handleSandboxCreate(c *gin.Context, kind string) { return } dynamicClient = userDynamicClient + _, _, _, createdBy = extractUserInfo(c) } // CRITICAL: Register watcher BEFORE creating sandbox @@ -141,7 +143,7 @@ func (s *Server) handleSandboxCreate(c *gin.Context, kind string) { // Ensure cleanup is called when function returns to prevent memory leak defer s.sandboxController.UnWatchSandbox(namespace, sandboxName) - response, err := s.createSandbox(c.Request.Context(), dynamicClient, sandbox, sandboxClaim, sandboxEntry, resultChan) + response, err := s.createSandbox(c.Request.Context(), dynamicClient, sandbox, sandboxClaim, sandboxEntry, resultChan, createdBy) if err != nil { // Client disconnected — abort with 499 so logs/metrics reflect the cancellation. if errors.Is(err, context.Canceled) { @@ -196,8 +198,8 @@ func (s *Server) createK8sResources(ctx context.Context, dynamicClient dynamic.I } // createSandbox performs sandbox creation and returns the response payload or an error with an HTTP status code. -func (s *Server) createSandbox(ctx context.Context, dynamicClient dynamic.Interface, sandbox *sandboxv1alpha1.Sandbox, sandboxClaim *extensionsv1alpha1.SandboxClaim, sandboxEntry *sandboxEntry, resultChan <-chan SandboxStatusUpdate) (*types.CreateSandboxResponse, error) { - placeholder := buildSandboxPlaceHolder(sandbox, sandboxEntry) +func (s *Server) createSandbox(ctx context.Context, dynamicClient dynamic.Interface, sandbox *sandboxv1alpha1.Sandbox, sandboxClaim *extensionsv1alpha1.SandboxClaim, sandboxEntry *sandboxEntry, resultChan <-chan SandboxStatusUpdate, createdBy string) (*types.CreateSandboxResponse, error) { + placeholder := buildSandboxPlaceHolder(sandbox, sandboxEntry, createdBy) if err := s.storeClient.StoreSandbox(ctx, placeholder); err != nil { if isContextError(err) { return nil, err @@ -261,7 +263,7 @@ func (s *Server) createSandbox(ctx context.Context, dynamicClient dynamic.Interf return nil, api.NewInternalError(fmt.Errorf("failed to verify sandbox %s/%s entrypoints: %w", sandbox.Namespace, sandbox.Name, err)) } - storeCacheInfo := buildSandboxInfo(createdSandbox, podIP, sandboxEntry) + storeCacheInfo := buildSandboxInfo(createdSandbox, podIP, sandboxEntry, createdBy) response := &types.CreateSandboxResponse{ Kind: storeCacheInfo.Kind, @@ -329,12 +331,19 @@ func (s *Server) handleGetSandbox(c *gin.Context, kind string) { } if s.config.EnableAuth { - _, userNamespace, _, _ := extractUserInfo(c) + _, userNamespace, _, serviceAccountName := extractUserInfo(c) if sandboxInfo.SandboxNamespace != userNamespace { klog.Warningf("unauthorized GET attempt to session %s by user in namespace %s", sessionID, userNamespace) respondError(c, http.StatusNotFound, fmt.Sprintf("Session ID %s not found", sessionID)) return } + // Enforce per-service-account ownership: only the creating SA can retrieve its session. + // Returns 404 (not 403) to prevent session ID enumeration. + if sandboxInfo.CreatedBy != "" && sandboxInfo.CreatedBy != serviceAccountName { + klog.Warningf("unauthorized GET attempt to session %s by service account %s (owner: %s)", sessionID, serviceAccountName, sandboxInfo.CreatedBy) + respondError(c, http.StatusNotFound, fmt.Sprintf("Session ID %s not found", sessionID)) + return + } } respondJSON(c, http.StatusOK, sandboxInfo) @@ -354,13 +363,20 @@ func (s *Server) handleListSandboxes(c *gin.Context, kind string) { return } - _, userNamespace, _, _ := extractUserInfo(c) + _, userNamespace, _, serviceAccountName := extractUserInfo(c) // Filter in-place to avoid a new allocation. + // When CreatedBy is set, enforce per-SA ownership (documented intent in auth.go). + // When CreatedBy is empty (legacy entries created before this field was added), + // fall back to namespace-scoped filtering so existing sessions remain accessible. filtered := sandboxes[:0] for _, sb := range sandboxes { - if sb.SandboxNamespace == userNamespace { - filtered = append(filtered, sb) + if sb.SandboxNamespace != userNamespace { + continue + } + if sb.CreatedBy != "" && sb.CreatedBy != serviceAccountName { + continue } + filtered = append(filtered, sb) } respondJSON(c, http.StatusOK, filtered) } diff --git a/pkg/workloadmanager/handlers_test.go b/pkg/workloadmanager/handlers_test.go index c108d74a..bba78f52 100644 --- a/pkg/workloadmanager/handlers_test.go +++ b/pkg/workloadmanager/handlers_test.go @@ -245,7 +245,7 @@ func TestServerCreateSandbox(t *testing.T) { claim = &extensionsv1alpha1.SandboxClaim{ObjectMeta: metav1.ObjectMeta{Name: sb.Name, Namespace: sb.Namespace}} } - resp, err := server.createSandbox(context.Background(), nil, sb, claim, makeEntry(), resultChan) + resp, err := server.createSandbox(context.Background(), nil, sb, claim, makeEntry(), resultChan, "") require.Equal(t, tt.expectCreateCalls, createCalls, "createSandbox call count") require.Equal(t, tt.expectClaimCalls, claimCalls, "createSandboxClaim call count") @@ -445,7 +445,7 @@ func TestHandleSandboxCreate(t *testing.T) { }) createCalls := 0 - patches.ApplyPrivateMethod(reflect.TypeOf(fakeServer), "createSandbox", func(_ *Server, _ context.Context, _ dynamic.Interface, _ *sandboxv1alpha1.Sandbox, _ *extensionsv1alpha1.SandboxClaim, _ *sandboxEntry, _ <-chan SandboxStatusUpdate) (*types.CreateSandboxResponse, error) { + patches.ApplyPrivateMethod(reflect.TypeOf(fakeServer), "createSandbox", func(_ *Server, _ context.Context, _ dynamic.Interface, _ *sandboxv1alpha1.Sandbox, _ *extensionsv1alpha1.SandboxClaim, _ *sandboxEntry, _ <-chan SandboxStatusUpdate, _ string) (*types.CreateSandboxResponse, error) { createCalls++ if tc.createErr != nil { return nil, tc.createErr @@ -542,6 +542,20 @@ func TestHandleGetSandbox(t *testing.T) { expectStatus: http.StatusNotFound, expectMessage: "Session ID sess-auth-fail not found", }, + { + name: "forbidden with auth matching namespace but different service account", + sessionID: "sess-wrong-sa", + enableAuth: true, + userNamespace: "user-ns", + storeResult: &types.SandboxInfo{ + SessionID: "sess-wrong-sa", + SandboxNamespace: "user-ns", + Kind: types.AgentRuntimeKind, + CreatedBy: "other-sa", + }, + expectStatus: http.StatusNotFound, + expectMessage: "Session ID sess-wrong-sa not found", + }, { name: "wrong kind", sessionID: "sess-wrong-kind", @@ -652,6 +666,22 @@ func TestHandleListSandboxes(t *testing.T) { expectStatus: http.StatusOK, expectCount: 1, }, + { + name: "success with auth filters by service account", + kind: types.AgentRuntimeKind, + enableAuth: true, + userNamespace: "user-ns", + storeResult: []*types.SandboxInfo{ + // Same namespace, created by the requesting SA — should be included + {SessionID: "1", SandboxNamespace: "user-ns", Kind: types.AgentRuntimeKind, CreatedBy: "mock-sa"}, + // Same namespace, different SA — should be excluded + {SessionID: "2", SandboxNamespace: "user-ns", Kind: types.AgentRuntimeKind, CreatedBy: "other-sa"}, + // Same namespace, no CreatedBy (legacy) — should be included (backward compat) + {SessionID: "3", SandboxNamespace: "user-ns", Kind: types.AgentRuntimeKind, CreatedBy: ""}, + }, + expectStatus: http.StatusOK, + expectCount: 2, + }, } for _, tt := range tests { diff --git a/pkg/workloadmanager/sandbox_helper.go b/pkg/workloadmanager/sandbox_helper.go index 322b5d8d..6de122ee 100644 --- a/pkg/workloadmanager/sandbox_helper.go +++ b/pkg/workloadmanager/sandbox_helper.go @@ -47,7 +47,7 @@ var sandboxEntrypointDial = func(ctx context.Context, endpoint string, timeout t return conn.Close() } -func buildSandboxPlaceHolder(sandboxCR *sandboxv1alpha1.Sandbox, entry *sandboxEntry) *types.SandboxInfo { +func buildSandboxPlaceHolder(sandboxCR *sandboxv1alpha1.Sandbox, entry *sandboxEntry, createdBy string) *types.SandboxInfo { var expiresAt time.Time if sandboxCR.Spec.Lifecycle.ShutdownTime != nil { expiresAt = sandboxCR.Spec.Lifecycle.ShutdownTime.Time @@ -66,10 +66,11 @@ func buildSandboxPlaceHolder(sandboxCR *sandboxv1alpha1.Sandbox, entry *sandboxE ExpiresAt: expiresAt, Status: "creating", IdleTimeout: metav1.Duration{Duration: idleTimeout}, + CreatedBy: createdBy, } } -func buildSandboxInfo(sandbox *sandboxv1alpha1.Sandbox, podIP string, entry *sandboxEntry) *types.SandboxInfo { +func buildSandboxInfo(sandbox *sandboxv1alpha1.Sandbox, podIP string, entry *sandboxEntry, createdBy string) *types.SandboxInfo { createdAt := sandbox.GetCreationTimestamp().Time expiresAt := createdAt.Add(DefaultSandboxTTL) if sandbox.Spec.Lifecycle.ShutdownTime != nil { @@ -98,6 +99,7 @@ func buildSandboxInfo(sandbox *sandboxv1alpha1.Sandbox, podIP string, entry *san ExpiresAt: expiresAt, Status: getSandboxStatus(sandbox), IdleTimeout: metav1.Duration{Duration: idleTimeout}, + CreatedBy: createdBy, } } diff --git a/pkg/workloadmanager/sandbox_helper_test.go b/pkg/workloadmanager/sandbox_helper_test.go index 793108ad..bc65a1d7 100644 --- a/pkg/workloadmanager/sandbox_helper_test.go +++ b/pkg/workloadmanager/sandbox_helper_test.go @@ -151,7 +151,7 @@ func TestBuildSandboxPlaceHolder_TableDriven(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { sandbox := tt.setupSandbox() - result := buildSandboxPlaceHolder(sandbox, tt.entry) + result := buildSandboxPlaceHolder(sandbox, tt.entry, "") tt.validate(t, result) }) } @@ -322,7 +322,7 @@ func TestBuildSandboxInfo_TableDriven(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { sandbox := tt.setupSandbox() - result := buildSandboxInfo(sandbox, tt.podIP, tt.entry) + result := buildSandboxInfo(sandbox, tt.podIP, tt.entry, "") tt.validateResult(t, result) }) }