From 063489afc73f1e9f8e8696a884363ab5a2cf4a47 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 28 May 2026 17:01:21 +0200 Subject: [PATCH 1/2] sec(api): move APIKeySecretURL/DeploymentAWSAccountID off /api/info (#633) Both fields exposed the AWS account ID + Secrets Manager ARN path to unauthenticated callers via GET /api/info (AuthPublic). - Strip both fields from PublicInfoResponse and getPublicInfo - Add DeploymentInfoResponse + getDeploymentInfo handler - Register GET /api/info/deployment with AuthUser - Frontend: approval-details.ts calls getDeploymentInfo (post-login); app.ts first-time setup no longer passes the removed hint - Regression test: JSON-marshals /api/info response and asserts neither sensitive key appears on the wire --- frontend/src/__tests__/api.test.ts | 38 ++++++++++-- frontend/src/api/auth.ts | 17 ++++++ frontend/src/api/index.ts | 2 + frontend/src/api/types.ts | 7 +++ frontend/src/app.ts | 5 +- frontend/src/approval-details.ts | 12 ++-- internal/api/handler_dashboard.go | 21 +++++-- internal/api/handler_dashboard_test.go | 83 +++++++++++++++++++------- internal/api/openapi.yaml | 36 ++++++++++- internal/api/router.go | 8 ++- internal/api/types.go | 23 +++++-- 11 files changed, 210 insertions(+), 42 deletions(-) diff --git a/frontend/src/__tests__/api.test.ts b/frontend/src/__tests__/api.test.ts index 458435380..4fa667525 100644 --- a/frontend/src/__tests__/api.test.ts +++ b/frontend/src/__tests__/api.test.ts @@ -33,7 +33,8 @@ import { executePurchase, getPurchaseDetails, cancelPurchase, - getPublicInfo + getPublicInfo, + getDeploymentInfo } from '../api'; import type { CreatePlanRequest, Config, Recommendation } from '../api'; @@ -963,14 +964,18 @@ describe('Purchase API', () => { describe('Public Info API', () => { describe('getPublicInfo', () => { - test('fetches public info without auth', async () => { + test('fetches version and admin_exists without auth (#633: no sensitive fields)', async () => { fetchMock.mockResolvedValue({ ok: true, - json: () => Promise.resolve({ version: '1.0.0', admin_exists: true, api_key_secret_url: 'https://...' }) + json: () => Promise.resolve({ version: '1.0.0', admin_exists: true }) }); const info = await getPublicInfo(); - expect(info.api_key_secret_url).toBeTruthy(); + expect(info.version).toBe('1.0.0'); + expect(info.admin_exists).toBe(true); + // Sensitive fields must not be present on the public endpoint response. + expect((info as Record)['api_key_secret_url']).toBeUndefined(); + expect((info as Record)['deployment_aws_account_id']).toBeUndefined(); expect(fetchMock).toHaveBeenCalledWith('/api/info'); }); @@ -982,4 +987,29 @@ describe('Public Info API', () => { expect(info.admin_exists).toBe(false); }); }); + + describe('getDeploymentInfo', () => { + test('fetches api_key_secret_url and deployment_aws_account_id with auth', async () => { + fetchMock.mockResolvedValue({ + ok: true, + json: () => Promise.resolve({ + api_key_secret_url: 'https://us-east-1.console.aws.amazon.com/secretsmanager/secret?name=arn:...', + deployment_aws_account_id: '123456789012', + }) + }); + + const info = await getDeploymentInfo(); + expect(info.api_key_secret_url).toContain('secretsmanager'); + expect(info.deployment_aws_account_id).toBe('123456789012'); + expect(fetchMock).toHaveBeenCalledWith('/api/info/deployment', expect.objectContaining({})); + }); + + test('returns empty object on error', async () => { + fetchMock.mockResolvedValue({ ok: false }); + + const info = await getDeploymentInfo(); + expect(info.api_key_secret_url).toBeUndefined(); + expect(info.deployment_aws_account_id).toBeUndefined(); + }); + }); }); diff --git a/frontend/src/api/auth.ts b/frontend/src/api/auth.ts index a8b5c50e2..faef1847e 100644 --- a/frontend/src/api/auth.ts +++ b/frontend/src/api/auth.ts @@ -7,6 +7,7 @@ import type { LoginResponse, User, PublicInfo, + DeploymentInfo, MFASetupResponse, MFARecoveryCodesResponse, MFALoginErrorCode, @@ -370,3 +371,19 @@ export async function getPublicInfo(): Promise { } return { version: '', admin_exists: false }; } + +/** + * Get deployment info (AuthUser required). Returns sensitive identifiers + * (API key secret URL, deployment AWS account ID) that must not be exposed + * to unauthenticated callers (#633). + */ +export async function getDeploymentInfo(): Promise { + const API_BASE = getApiBase(); + const response = await fetch(`${API_BASE}/info/deployment`, { + headers: getAuthHeaders(), + }); + if (response.ok) { + return response.json() as Promise; + } + return {}; +} diff --git a/frontend/src/api/index.ts b/frontend/src/api/index.ts index 7feb085bf..111048744 100644 --- a/frontend/src/api/index.ts +++ b/frontend/src/api/index.ts @@ -21,6 +21,7 @@ export type { Config, ServiceConfig, PublicInfo, + DeploymentInfo, PurchaseResult, PurchaseDetails, PlannedPurchasesResponse, @@ -86,6 +87,7 @@ export { setupAdmin, changePassword, getPublicInfo, + getDeploymentInfo, // MFA lifecycle (issue #497) MFALoginError, setupMFA, diff --git a/frontend/src/api/types.ts b/frontend/src/api/types.ts index 900627ba2..1723209db 100644 --- a/frontend/src/api/types.ts +++ b/frontend/src/api/types.ts @@ -254,9 +254,16 @@ export interface ServiceConfig { exclude_types?: string[]; } +/** Response from GET /api/info (unauthenticated). Only safe-to-expose fields. */ export interface PublicInfo { version: string; admin_exists: boolean; +} + +/** Response from GET /api/info/deployment (AuthUser). Sensitive identifiers + * that must not be visible to unauthenticated callers (#633). */ +export interface DeploymentInfo { + /** AWS Console deep-link to the Secrets Manager secret holding the API key. */ api_key_secret_url?: string; /** AWS account ID of the CUDly Lambda (from STS GetCallerIdentity). Present * only on AWS-hosted deployments; omitted on Azure/GCP or when STS is diff --git a/frontend/src/app.ts b/frontend/src/app.ts index 124b7e0dc..71882160e 100644 --- a/frontend/src/app.ts +++ b/frontend/src/app.ts @@ -46,7 +46,10 @@ export async function init(): Promise { try { const publicInfo = await api.getPublicInfo(); if (!publicInfo.admin_exists) { - await showAdminSetupModal(publicInfo.api_key_secret_url); + // api_key_secret_url is no longer served on the unauthenticated + // /api/info endpoint (#633). The admin setup modal works without + // the hint — the deployer can find the secret in the AWS console. + await showAdminSetupModal(undefined); return; } } catch { diff --git a/frontend/src/approval-details.ts b/frontend/src/approval-details.ts index 10f839807..84b454367 100644 --- a/frontend/src/approval-details.ts +++ b/frontend/src/approval-details.ts @@ -313,20 +313,24 @@ export async function buildApprovalDetailsBody(executionId: string): Promise { console.warn('Failed to load accounts for approval modal — falling back to UUID-prefixed labels:', err); return [] as CloudAccount[]; }), - api.getPublicInfo().catch((err) => { - console.warn('Failed to load public info for approval modal — orphan label will show "Account deleted":', err); + // getDeploymentInfo requires an authenticated session (AuthUser). The + // approval modal is only reachable post-login, so this is safe. On + // failure (e.g. token expiry) the orphan label degrades to "Account deleted" + // which is the safe default (#633). + api.getDeploymentInfo().catch((err) => { + console.warn('Failed to load deployment info for approval modal — orphan label will show "Account deleted":', err); return undefined; }), ]); const accountsById = new Map(); for (const acct of accounts) accountsById.set(acct.id, acct); - const hostAWSAccountID = publicInfo?.deployment_aws_account_id; + const hostAWSAccountID = deploymentInfo?.deployment_aws_account_id; return renderApprovalDetailsBody(details, accountsById, hostAWSAccountID); } catch (err) { console.error('Failed to load purchase details for approval modal:', err); diff --git a/internal/api/handler_dashboard.go b/internal/api/handler_dashboard.go index 1de157228..e239b803a 100644 --- a/internal/api/handler_dashboard.go +++ b/internal/api/handler_dashboard.go @@ -331,8 +331,10 @@ func (h *Handler) planIntersectsAllowed(ctx context.Context, planID string, allo return false, nil } -// getPublicInfo returns public information about the CUDly instance (no auth required) +// getPublicInfo returns public information about the CUDly instance (no auth required). // No rate limiting — this is hit by Terraform deployment checks and the frontend on every page load. +// Sensitive identifiers (API key secret URL, deployment AWS account ID) are intentionally +// absent here; they live on the authenticated GET /api/info/deployment endpoint (#633). func (h *Handler) getPublicInfo(ctx context.Context, req *events.LambdaFunctionURLRequest) (*PublicInfoResponse, error) { // Check if admin exists adminExists := false @@ -343,7 +345,18 @@ func (h *Handler) getPublicInfo(ctx context.Context, req *events.LambdaFunctionU } } - // Build the API key secret URL for the console + return &PublicInfoResponse{ + Version: "1.0.0", + AdminExists: adminExists, + }, nil +} + +// getDeploymentInfo returns sensitive deployment identifiers for authenticated callers. +// Requires at least AuthUser (enforced by the router). The two fields it returns +// expose the AWS account ID and the Secrets Manager ARN path — neither should be +// reachable without a valid session (#633). +func (h *Handler) getDeploymentInfo(ctx context.Context, _ *events.LambdaFunctionURLRequest) (*DeploymentInfoResponse, error) { + // Build the AWS Console deep-link to the Secrets Manager secret. var apiKeySecretURL string if h.secretsARN != "" { // Extract region from ARN: arn:aws:secretsmanager:region:account:secret:name @@ -364,9 +377,7 @@ func (h *Handler) getPublicInfo(ctx context.Context, req *events.LambdaFunctionU // warning label, which is safe. deploymentAWSAccountID, _ := h.resolveAWSAccountID(ctx) - return &PublicInfoResponse{ - Version: "1.0.0", - AdminExists: adminExists, + return &DeploymentInfoResponse{ APIKeySecretURL: apiKeySecretURL, DeploymentAWSAccountID: deploymentAWSAccountID, }, nil diff --git a/internal/api/handler_dashboard_test.go b/internal/api/handler_dashboard_test.go index 43c673ee5..b800d76f4 100644 --- a/internal/api/handler_dashboard_test.go +++ b/internal/api/handler_dashboard_test.go @@ -2,6 +2,7 @@ package api import ( "context" + "encoding/json" "errors" "testing" "time" @@ -546,8 +547,6 @@ func TestHandler_getPublicInfo(t *testing.T) { assert.Equal(t, "1.0.0", result.Version) assert.True(t, result.AdminExists) - assert.Contains(t, result.APIKeySecretURL, "us-east-1") - assert.Contains(t, result.APIKeySecretURL, "secretsmanager") }) t.Run("with auth service and no admin", func(t *testing.T) { @@ -562,7 +561,6 @@ func TestHandler_getPublicInfo(t *testing.T) { require.NoError(t, err) assert.False(t, result.AdminExists) - assert.Empty(t, result.APIKeySecretURL) }) t.Run("auth service check error still returns response", func(t *testing.T) { @@ -589,53 +587,94 @@ func TestHandler_getPublicInfo(t *testing.T) { assert.False(t, result.AdminExists) }) - t.Run("ARN parsing for different regions", func(t *testing.T) { + t.Run("with rate limiting - allowed", func(t *testing.T) { mockAuth := new(MockAuthService) + mockRateLimiter := new(MockRateLimiter) mockAuth.On("CheckAdminExists", ctx).Return(true, nil) + mockRateLimiter.On("AllowWithIP", ctx, "192.168.1.1", "api_general").Return(true, nil) handler := &Handler{ - auth: mockAuth, - secretsARN: "arn:aws:secretsmanager:eu-west-1:987654321098:secret:my-secret-xyz789", + auth: mockAuth, + rateLimiter: mockRateLimiter, } result, err := handler.getPublicInfo(ctx, createMockLambdaRequest("192.168.1.1")) require.NoError(t, err) - - assert.Contains(t, result.APIKeySecretURL, "eu-west-1") + assert.True(t, result.AdminExists) }) - t.Run("invalid ARN format", func(t *testing.T) { + // Regression test for #633: sensitive identifiers must never appear in the + // unauthenticated /api/info response, even when a secretsARN is configured. + t.Run("no sensitive fields in unauthenticated response (#633)", func(t *testing.T) { mockAuth := new(MockAuthService) mockAuth.On("CheckAdminExists", ctx).Return(true, nil) handler := &Handler{ auth: mockAuth, - secretsARN: "invalid-arn", + secretsARN: "arn:aws:secretsmanager:us-east-1:123456789012:secret:api-key-abc123", } - result, err := handler.getPublicInfo(ctx, createMockLambdaRequest("192.168.1.1")) + result, err := handler.getPublicInfo(ctx, createMockLambdaRequest("10.0.0.1")) require.NoError(t, err) - // Invalid ARN should result in empty URL - assert.Empty(t, result.APIKeySecretURL) + // PublicInfoResponse no longer carries these fields — the struct itself is + // the compile-time guard. The JSON assertion catches any future re-addition + // via an embedded struct or interface{} workaround. + assert.Equal(t, "1.0.0", result.Version) + assert.True(t, result.AdminExists) + encoded, jsonErr := json.Marshal(result) + require.NoError(t, jsonErr) + body := string(encoded) + assert.NotContains(t, body, "api_key_secret_url", "api_key_secret_url must not appear in /api/info response") + assert.NotContains(t, body, "deployment_aws_account_id", "deployment_aws_account_id must not appear in /api/info response") }) +} - t.Run("with rate limiting - allowed", func(t *testing.T) { - mockAuth := new(MockAuthService) - mockRateLimiter := new(MockRateLimiter) - mockAuth.On("CheckAdminExists", ctx).Return(true, nil) - mockRateLimiter.On("AllowWithIP", ctx, "192.168.1.1", "api_general").Return(true, nil) +func TestHandler_getDeploymentInfo(t *testing.T) { + ctx := context.Background() + t.Run("returns ARN-derived URL and account ID", func(t *testing.T) { handler := &Handler{ - auth: mockAuth, - rateLimiter: mockRateLimiter, + secretsARN: "arn:aws:secretsmanager:us-east-1:123456789012:secret:api-key-abc123", } - result, err := handler.getPublicInfo(ctx, createMockLambdaRequest("192.168.1.1")) + result, err := handler.getDeploymentInfo(ctx, createMockLambdaRequest("10.0.0.1")) require.NoError(t, err) - assert.True(t, result.AdminExists) + + assert.Contains(t, result.APIKeySecretURL, "us-east-1") + assert.Contains(t, result.APIKeySecretURL, "secretsmanager") + }) + + t.Run("ARN parsing for different regions", func(t *testing.T) { + handler := &Handler{ + secretsARN: "arn:aws:secretsmanager:eu-west-1:987654321098:secret:my-secret-xyz789", + } + + result, err := handler.getDeploymentInfo(ctx, createMockLambdaRequest("10.0.0.1")) + require.NoError(t, err) + + assert.Contains(t, result.APIKeySecretURL, "eu-west-1") }) + t.Run("invalid ARN format returns empty URL", func(t *testing.T) { + handler := &Handler{ + secretsARN: "invalid-arn", + } + + result, err := handler.getDeploymentInfo(ctx, createMockLambdaRequest("10.0.0.1")) + require.NoError(t, err) + + assert.Empty(t, result.APIKeySecretURL) + }) + + t.Run("empty secretsARN returns empty URL", func(t *testing.T) { + handler := &Handler{} + + result, err := handler.getDeploymentInfo(ctx, createMockLambdaRequest("10.0.0.1")) + require.NoError(t, err) + + assert.Empty(t, result.APIKeySecretURL) + }) } func TestHandler_calculateCommitmentMetrics(t *testing.T) { diff --git a/internal/api/openapi.yaml b/internal/api/openapi.yaml index ec83a4488..8804221b8 100644 --- a/internal/api/openapi.yaml +++ b/internal/api/openapi.yaml @@ -1244,7 +1244,7 @@ paths: get: operationId: getPublicInfo tags: [Info] - summary: Public instance information (version, admin status) + summary: Public instance information (version, admin status — no auth required) security: [] responses: '200': @@ -1254,6 +1254,28 @@ paths: schema: $ref: '#/components/schemas/PublicInfoResponse' + /api/info/deployment: + get: + operationId: getDeploymentInfo + tags: [Info] + summary: Deployment-scoped identifiers (AuthUser required) + description: > + Returns the Secrets Manager console URL and the deployment AWS account ID. + Requires at least a valid user session. Not accessible to unauthenticated + callers (closes #633). + security: + - bearerAuth: [] + - apiKeyAuth: [] + responses: + '200': + description: Deployment info + content: + application/json: + schema: + $ref: '#/components/schemas/DeploymentInfoResponse' + '401': + $ref: '#/components/responses/Unauthorized' + # ---- Docs -------------------------------------------------------------- /api/docs: get: @@ -2399,13 +2421,25 @@ components: # -- Info --------------------------------------------------------------- PublicInfoResponse: type: object + description: > + Unauthenticated public info. Only safe-to-expose fields are included. + Sensitive identifiers (API key secret URL, AWS account ID) are served + by the authenticated /api/info/deployment endpoint (#633). properties: version: type: string admin_exists: type: boolean + + DeploymentInfoResponse: + type: object + description: > + Deployment-scoped identifiers. Requires AuthUser. Not accessible to + unauthenticated callers (#633). + properties: api_key_secret_url: type: string + description: AWS Console deep-link to the Secrets Manager secret holding the API key. deployment_aws_account_id: type: string description: > diff --git a/internal/api/router.go b/internal/api/router.go index 75e38059d..7dec2326b 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -311,8 +311,10 @@ func (r *Router) registerRoutes() { {ExactPath: "/health", Handler: r.healthCheckHandler, Auth: AuthPublic}, {ExactPath: "/api/health", Handler: r.healthCheckHandler, Auth: AuthPublic}, - // Public info endpoint + // Public info endpoint (unauthenticated — version + admin_exists only) {ExactPath: "/api/info", Method: "GET", Handler: r.getPublicInfoHandler, Auth: AuthPublic}, + // Deployment info endpoint (authenticated — API key secret URL + AWS account ID, #633) + {ExactPath: "/api/info/deployment", Method: "GET", Handler: r.getDeploymentInfoHandler, Auth: AuthUser}, // Build-version endpoint — public, returns only version / git SHA / // build time (no account IDs, ARNs, or secrets) so an operator can @@ -689,6 +691,10 @@ func (r *Router) getVersionHandler(ctx context.Context, req *events.LambdaFuncti return r.h.getVersion(ctx, req) } +func (r *Router) getDeploymentInfoHandler(ctx context.Context, req *events.LambdaFunctionURLRequest, params map[string]string) (any, error) { + return r.h.getDeploymentInfo(ctx, req) +} + func (r *Router) docsHandler(ctx context.Context, req *events.LambdaFunctionURLRequest, params map[string]string) (any, error) { return r.h.docsHandler(ctx, req, params) } diff --git a/internal/api/types.go b/internal/api/types.go index 3cce623d2..c8f8dbf17 100644 --- a/internal/api/types.go +++ b/internal/api/types.go @@ -456,11 +456,26 @@ type MFARegenerateResponse struct { // EmptyServiceConfigResponse represents an empty service config type EmptyServiceConfigResponse struct{} -// PublicInfoResponse holds public information about the CUDly instance +// PublicInfoResponse holds public information about the CUDly instance. +// Only fields safe for unauthenticated callers — sensitive identifiers +// (API key secret URL, deployment AWS account ID) live on the +// authenticated /api/info/deployment endpoint instead. type PublicInfoResponse struct { - Version string `json:"version"` - AdminExists bool `json:"admin_exists"` - APIKeySecretURL string `json:"api_key_secret_url,omitempty"` + Version string `json:"version"` + AdminExists bool `json:"admin_exists"` +} + +// DeploymentInfoResponse holds deployment-scoped identifiers that must +// not be exposed to unauthenticated callers. Served by +// GET /api/info/deployment (AuthUser). +type DeploymentInfoResponse struct { + // APIKeySecretURL is the AWS Console deep-link to the Secrets Manager + // secret holding the CUDly API key. + APIKeySecretURL string `json:"api_key_secret_url,omitempty"` + // DeploymentAWSAccountID is the AWS account ID of the Lambda host, + // resolved via STS GetCallerIdentity. Empty on non-AWS deployments + // or when STS is unreachable. Used by the frontend to distinguish + // legitimate ambient-credential executions from orphan rows (#608). DeploymentAWSAccountID string `json:"deployment_aws_account_id,omitempty"` } From 435b3c4e0dbc8d707461be5162c6e523dec1493e Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 1 Jun 2026 19:21:55 +0200 Subject: [PATCH 2/2] fix(frontend/api): cast PublicInfo via unknown for absent-field assertions PublicInfo no longer includes api_key_secret_url or deployment_aws_account_id (moved to DeploymentInfo by #633). The direct cast to Record therefore fails TS2352. Cast through unknown first to satisfy the type checker while preserving the runtime assertion that these fields are absent. --- frontend/src/__tests__/api.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/src/__tests__/api.test.ts b/frontend/src/__tests__/api.test.ts index 4fa667525..af4ddf2b8 100644 --- a/frontend/src/__tests__/api.test.ts +++ b/frontend/src/__tests__/api.test.ts @@ -974,8 +974,8 @@ describe('Public Info API', () => { expect(info.version).toBe('1.0.0'); expect(info.admin_exists).toBe(true); // Sensitive fields must not be present on the public endpoint response. - expect((info as Record)['api_key_secret_url']).toBeUndefined(); - expect((info as Record)['deployment_aws_account_id']).toBeUndefined(); + expect((info as unknown as Record)['api_key_secret_url']).toBeUndefined(); + expect((info as unknown as Record)['deployment_aws_account_id']).toBeUndefined(); expect(fetchMock).toHaveBeenCalledWith('/api/info'); });