From 8d53015b87c5aa16d26f4087a927ba0a5e51e342 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Wed, 20 May 2026 01:47:39 +0200 Subject: [PATCH 1/2] sec(api): restrict SourceIdentity to admin sessions in GET /api/config (closes #407) GET /api/config returned SourceIdentity (AWS account ID, Azure tenant ID / subscription ID, GCP project ID) to any authenticated user. Non-admin roles should not have access to the host cloud identity. Change getConfig to accept the request so it can call requireAdmin. Only populate SourceIdentity when the caller is an admin; non-admin responses get SourceIdentity: nil. SourceCloud (the cloud name string "aws"/"azure") is still returned to all authenticated users as it is needed by the dashboard for feature gating. Update all call sites (router wrapper, existing tests). Add regression tests confirming non-admin sessions receive a nil SourceIdentity. --- internal/api/handler_config.go | 23 ++++++---- internal/api/handler_config_test.go | 65 +++++++++++++++++++++++++++-- internal/api/router.go | 2 +- 3 files changed, 78 insertions(+), 12 deletions(-) diff --git a/internal/api/handler_config.go b/internal/api/handler_config.go index f4769dc6e..d589f235b 100644 --- a/internal/api/handler_config.go +++ b/internal/api/handler_config.go @@ -23,7 +23,7 @@ func sourceCloud() string { } // Configuration handlers -func (h *Handler) getConfig(ctx context.Context) (*ConfigResponse, error) { +func (h *Handler) getConfig(ctx context.Context, req *events.LambdaFunctionURLRequest) (*ConfigResponse, error) { globalCfg, err := h.config.GetGlobalConfig(ctx) if err != nil { return nil, err @@ -34,12 +34,21 @@ func (h *Handler) getConfig(ctx context.Context) (*ConfigResponse, error) { return nil, err } - return &ConfigResponse{ - Global: globalCfg, - Services: services, - SourceCloud: sourceCloud(), - SourceIdentity: h.resolveSourceIdentity(ctx), - }, nil + resp := &ConfigResponse{ + Global: globalCfg, + Services: services, + SourceCloud: sourceCloud(), + } + + // SourceIdentity contains the host cloud account ID (AWS account number, + // Azure tenant ID, etc.). Only expose it to admin sessions so that + // non-admin users cannot extract the cloud identity of the CUDly host + // account (issue #407). + if _, adminErr := h.requireAdmin(ctx, req); adminErr == nil { + resp.SourceIdentity = h.resolveSourceIdentity(ctx) + } + + return resp, nil } // preserveOmittedRecommendationFields merges persisted GlobalConfig values diff --git a/internal/api/handler_config_test.go b/internal/api/handler_config_test.go index 247cd9803..9341a2939 100644 --- a/internal/api/handler_config_test.go +++ b/internal/api/handler_config_test.go @@ -14,6 +14,13 @@ import ( func TestHandler_getConfig(t *testing.T) { ctx := context.Background() mockStore := new(MockConfigStore) + mockAuth := new(MockAuthService) + + adminSession := &Session{ + UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", + Email: "admin@example.com", + Role: "admin", + } globalCfg := &config.GlobalConfig{ EnabledProviders: []string{"aws"}, @@ -25,12 +32,16 @@ func TestHandler_getConfig(t *testing.T) { {Provider: "aws", Service: "rds", Enabled: true}, } + mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil) mockStore.On("GetGlobalConfig", ctx).Return(globalCfg, nil) mockStore.On("ListServiceConfigs", ctx).Return(serviceConfigs, nil) - handler := &Handler{config: mockStore} + handler := &Handler{config: mockStore, auth: mockAuth} - result, err := handler.getConfig(ctx) + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"Authorization": "Bearer admin-token"}, + } + result, err := handler.getConfig(ctx, req) require.NoError(t, err) assert.NotNil(t, result.Global) @@ -327,7 +338,8 @@ func TestHandler_getConfig_GlobalConfigError(t *testing.T) { handler := &Handler{config: mockStore} - result, err := handler.getConfig(ctx) + req := &events.LambdaFunctionURLRequest{} + result, err := handler.getConfig(ctx, req) assert.Error(t, err) assert.Nil(t, result) } @@ -345,11 +357,56 @@ func TestHandler_getConfig_ListServiceConfigsError(t *testing.T) { handler := &Handler{config: mockStore} - result, err := handler.getConfig(ctx) + req := &events.LambdaFunctionURLRequest{} + result, err := handler.getConfig(ctx, req) assert.Error(t, err) assert.Nil(t, result) } +// Regression tests for issue #407: SourceIdentity (cloud account ID, Azure +// tenant ID) must only be included in responses for admin sessions. + +func TestHandler_getConfig_SourceIdentity_AdminOnly(t *testing.T) { + ctx := context.Background() + mockStore := new(MockConfigStore) + mockAuth := new(MockAuthService) + + adminSession := &Session{Role: "admin", UserID: "admin-user"} + userSession := &Session{Role: "user", UserID: "regular-user"} + + globalCfg := &config.GlobalConfig{EnabledProviders: []string{"aws"}} + mockStore.On("GetGlobalConfig", ctx).Return(globalCfg, nil) + mockStore.On("ListServiceConfigs", ctx).Return([]config.ServiceConfig{}, nil) + mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil) + mockAuth.On("ValidateSession", ctx, "user-token").Return(userSession, nil) + + handler := &Handler{config: mockStore, auth: mockAuth} + + t.Run("admin sees SourceIdentity", func(t *testing.T) { + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"Authorization": "Bearer admin-token"}, + } + result, err := handler.getConfig(ctx, req) + require.NoError(t, err) + // SourceIdentity is set (may be a zero-value struct for the test + // env that lacks AWS credentials, but the field is non-nil for admin). + // We verify that a non-admin result is nil below, which is the key + // security invariant. + _ = result.SourceIdentity // may be nil or non-nil depending on cloud env + }) + + t.Run("regression #407: non-admin does not receive SourceIdentity", func(t *testing.T) { + mockAuth.On("HasPermissionAPI", ctx, "regular-user", mock.Anything, mock.Anything).Return(false, nil) + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"Authorization": "Bearer user-token"}, + } + result, err := handler.getConfig(ctx, req) + require.NoError(t, err) + assert.Nil(t, result.SourceIdentity, + "SourceIdentity must be nil for non-admin sessions (issue #407)") + }) +} + func TestHandler_getServiceConfig_Error(t *testing.T) { ctx := context.Background() mockStore := new(MockConfigStore) diff --git a/internal/api/router.go b/internal/api/router.go index e5a3e700e..30a4286eb 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -421,7 +421,7 @@ func (r *Router) upcomingPurchasesHandler(ctx context.Context, req *events.Lambd } func (r *Router) getConfigHandler(ctx context.Context, req *events.LambdaFunctionURLRequest, params map[string]string) (any, error) { - return r.h.getConfig(ctx) + return r.h.getConfig(ctx, req) } func (r *Router) updateConfigHandler(ctx context.Context, req *events.LambdaFunctionURLRequest, params map[string]string) (any, error) { From 41413665b69d85099e22fd9ff2245fa9caa55a33 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 5 Jun 2026 12:45:35 +0200 Subject: [PATCH 2/2] test(api/config): rebase + drop removed Session.Role, mock HasPermissionAPI for admin gate, assert SourceIdentity admin-only - Remove Session.Role field from all Session literals in handler_config_test.go (Role was removed in #907/#912; role-based short-circuit replaced by group membership via HasPermissionAPI) - Add mockAuth.grantAdmin() to TestHandler_getConfig so the admin permission check in requireAdmin is satisfied - Add explicit HasPermissionAPI stub for "admin-user" in TestHandler_getConfig_SourceIdentity_AdminOnly (required now that the admin path goes through HasPermissionAPI, not a Role field) - Replace no-op `_ = result.SourceIdentity` with require.NotNil assertion (admin sessions must receive SourceIdentity per issue #407) - Non-admin subtest already asserts SourceIdentity == nil via assert.Nil --- internal/api/handler_config_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/api/handler_config_test.go b/internal/api/handler_config_test.go index 9341a2939..132e01a41 100644 --- a/internal/api/handler_config_test.go +++ b/internal/api/handler_config_test.go @@ -19,7 +19,6 @@ func TestHandler_getConfig(t *testing.T) { adminSession := &Session{ UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", Email: "admin@example.com", - Role: "admin", } globalCfg := &config.GlobalConfig{ @@ -33,6 +32,7 @@ func TestHandler_getConfig(t *testing.T) { } mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil) + mockAuth.grantAdmin() mockStore.On("GetGlobalConfig", ctx).Return(globalCfg, nil) mockStore.On("ListServiceConfigs", ctx).Return(serviceConfigs, nil) @@ -371,14 +371,15 @@ func TestHandler_getConfig_SourceIdentity_AdminOnly(t *testing.T) { mockStore := new(MockConfigStore) mockAuth := new(MockAuthService) - adminSession := &Session{Role: "admin", UserID: "admin-user"} - userSession := &Session{Role: "user", UserID: "regular-user"} + adminSession := &Session{UserID: "admin-user"} + userSession := &Session{UserID: "regular-user"} globalCfg := &config.GlobalConfig{EnabledProviders: []string{"aws"}} mockStore.On("GetGlobalConfig", ctx).Return(globalCfg, nil) mockStore.On("ListServiceConfigs", ctx).Return([]config.ServiceConfig{}, nil) mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil) mockAuth.On("ValidateSession", ctx, "user-token").Return(userSession, nil) + mockAuth.On("HasPermissionAPI", mock.Anything, "admin-user", mock.Anything, mock.Anything).Return(true, nil) handler := &Handler{config: mockStore, auth: mockAuth} @@ -388,11 +389,10 @@ func TestHandler_getConfig_SourceIdentity_AdminOnly(t *testing.T) { } result, err := handler.getConfig(ctx, req) require.NoError(t, err) - // SourceIdentity is set (may be a zero-value struct for the test - // env that lacks AWS credentials, but the field is non-nil for admin). - // We verify that a non-admin result is nil below, which is the key - // security invariant. - _ = result.SourceIdentity // may be nil or non-nil depending on cloud env + // resolveSourceIdentity always returns a non-nil struct (best-effort, + // returns an empty struct on failure). The key invariant is that admin + // sessions receive the field and non-admin sessions do not. + require.NotNil(t, result.SourceIdentity) }) t.Run("regression #407: non-admin does not receive SourceIdentity", func(t *testing.T) {