From d6fc5dd64c3232e9c95a54b0e433f3e057fd7fa4 Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Thu, 5 Mar 2026 18:49:16 +0700 Subject: [PATCH] fix: harden profile lifecycle validation and dashboard UX flows --- src/api/services/profile-lifecycle-service.ts | 45 +++++++++++++---- src/commands/api-command.ts | 4 +- src/web-server/routes/profile-routes.ts | 38 +++++++++++++-- .../api/profile-lifecycle-service.test.ts | 48 ++++++++++++++++++- .../profile-routes-lifecycle.test.ts | 37 +++++++++++++- ui/src/hooks/use-profiles.ts | 15 ------ ui/src/lib/api-client.ts | 9 +++- ui/src/pages/api.tsx | 24 ++++++++-- 8 files changed, 184 insertions(+), 36 deletions(-) diff --git a/src/api/services/profile-lifecycle-service.ts b/src/api/services/profile-lifecycle-service.ts index f7e9e3fe..5e91f455 100644 --- a/src/api/services/profile-lifecycle-service.ts +++ b/src/api/services/profile-lifecycle-service.ts @@ -32,6 +32,13 @@ import type { const SETTINGS_FILE_SUFFIX = '.settings.json'; const REDACTED_TOKEN_SENTINEL = '__CCS_REDACTED__'; +function parseTargetValue(value: unknown): TargetType | null { + if (value === 'claude' || value === 'droid') { + return value; + } + return null; +} + function validateProfileNameForPath(name: string, label: string): string | null { const validationError = validateApiName(name); if (validationError) { @@ -195,6 +202,15 @@ export function registerApiProfileOrphans(options?: { const result: RegisterApiProfileOrphansResult = { registered: [], skipped: [] }; for (const orphan of selected) { + const nameError = validateApiName(orphan.name); + if (nameError) { + result.skipped.push({ + name: orphan.name, + reason: `Invalid profile name: ${nameError}`, + }); + continue; + } + if (!options?.force && !orphan.validation.valid) { result.skipped.push({ name: orphan.name, @@ -336,12 +352,29 @@ export function importApiProfileBundle( const nameError = validateApiName(name); if (nameError) return { success: false, error: nameError }; + const bundleTarget = parseTargetValue(input.profile.target); + if (input.profile.target !== undefined && bundleTarget === null) { + return { + success: false, + error: 'Invalid bundle profile target. Expected: claude or droid.', + }; + } + const settings = JSON.parse(JSON.stringify(input.settings)) as Record; const env = settings.env as Record | undefined; const warnings: string[] = []; - if (env?.ANTHROPIC_AUTH_TOKEN === REDACTED_TOKEN_SENTINEL) { - env.ANTHROPIC_AUTH_TOKEN = ''; - warnings.push('Imported bundle had redacted token. Set ANTHROPIC_AUTH_TOKEN before use.'); + if (env) { + const redactedKeys = Object.entries(env) + .filter(([key, value]) => isSensitiveKey(key) && value === REDACTED_TOKEN_SENTINEL) + .map(([key]) => key); + if (redactedKeys.length > 0) { + for (const key of redactedKeys) { + env[key] = ''; + } + warnings.push( + `Imported bundle had redacted values for ${redactedKeys.join(', ')}. Set secrets before use.` + ); + } } const validation = validateApiProfileSettingsPayload(settings); @@ -357,11 +390,7 @@ export function importApiProfileBundle( writeJsonObjectAtomically(settingsPath, settings); ensureProfileHooks(name); try { - registerApiProfileInConfig( - name, - options?.target || input.profile.target || 'claude', - options?.force - ); + registerApiProfileInConfig(name, options?.target || bundleTarget || 'claude', options?.force); } catch (registrationError) { rollbackSettingsFile(settingsPath, previousSettingsContent, settingsExisted); throw registrationError; diff --git a/src/commands/api-command.ts b/src/commands/api-command.ts index f1d4fc6f..60c5ec79 100644 --- a/src/commands/api-command.ts +++ b/src/commands/api-command.ts @@ -926,7 +926,9 @@ async function showHelp(): Promise { console.log(` ${color('--out ', 'command')} Export bundle output path`); console.log(` ${color('--include-secrets', 'command')} Include token in export bundle`); console.log(` ${color('--name ', 'command')} Override profile name during import`); - console.log(` ${color('--force', 'command')} Overwrite existing (create)`); + console.log( + ` ${color('--force', 'command')} Overwrite existing or bypass validation (create/discover/copy/import)` + ); console.log(` ${color('--yes, -y', 'command')} Skip confirmation prompts`); console.log(''); console.log(subheader('Provider Presets')); diff --git a/src/web-server/routes/profile-routes.ts b/src/web-server/routes/profile-routes.ts index ac7c2179..a71247e8 100644 --- a/src/web-server/routes/profile-routes.ts +++ b/src/web-server/routes/profile-routes.ts @@ -181,9 +181,28 @@ router.post('/orphans/register', (req: Request, res: Response): void => { } const payload = shape.payload; - const names = Array.isArray(payload.names) - ? payload.names.filter((value): value is string => typeof value === 'string') - : undefined; + let names: string[] | undefined; + if (payload.names !== undefined) { + if (!Array.isArray(payload.names)) { + res.status(400).json({ error: 'names must be an array of profile names' }); + return; + } + + const invalidNameEntry = payload.names.find( + (value) => typeof value !== 'string' || value.trim().length === 0 + ); + if (invalidNameEntry !== undefined) { + res.status(400).json({ error: 'names must contain non-empty strings only' }); + return; + } + + names = payload.names.map((value) => value.trim()); + const invalidName = names.find((name) => validateApiName(name) !== null); + if (invalidName) { + res.status(400).json({ error: `Invalid profile name in names: ${invalidName}` }); + return; + } + } const target = parseTarget(payload.target); const force = payload.force === true; @@ -284,7 +303,18 @@ router.post('/import', (req: Request, res: Response): void => { return; } - const result = importApiProfileBundle(shape.payload.bundle, { + const bundle = shape.payload.bundle; + if (!bundle || typeof bundle !== 'object' || Array.isArray(bundle)) { + res.status(400).json({ error: 'bundle must be a JSON object' }); + return; + } + const bundleTarget = (bundle as { profile?: { target?: unknown } }).profile?.target; + if (bundleTarget !== undefined && parseTarget(bundleTarget) === null) { + res.status(400).json({ error: 'Invalid bundle profile target. Expected: claude or droid' }); + return; + } + + const result = importApiProfileBundle(bundle, { name: typeof shape.payload.name === 'string' ? shape.payload.name : undefined, target: target || undefined, force: shape.payload.force === true, diff --git a/tests/unit/api/profile-lifecycle-service.test.ts b/tests/unit/api/profile-lifecycle-service.test.ts index d6820f0b..6f50dad6 100644 --- a/tests/unit/api/profile-lifecycle-service.test.ts +++ b/tests/unit/api/profile-lifecycle-service.test.ts @@ -6,6 +6,7 @@ import { copyApiProfile, discoverApiProfileOrphans, exportApiProfile, + importApiProfileBundle, registerApiProfileOrphans, } from '../../../src/api/services/profile-lifecycle-service'; @@ -113,5 +114,50 @@ describe('profile lifecycle service', () => { expect(result.success).toBe(false); expect(result.error).toContain('Invalid source profile name'); }); -}); + it('rejects import bundle with invalid profile target', () => { + const result = importApiProfileBundle({ + schemaVersion: 1, + exportedAt: new Date().toISOString(), + profile: { name: 'glm', target: 'invalid-target' }, + settings: { + env: { + ANTHROPIC_BASE_URL: 'https://api.example.com', + ANTHROPIC_AUTH_TOKEN: 'token', + }, + }, + }); + + expect(result.success).toBe(false); + expect(result.error).toContain('Invalid bundle profile target'); + }); + + it('clears and warns for all redacted sensitive env keys on import', () => { + const ccsDir = path.join(tempHome, '.ccs'); + fs.mkdirSync(ccsDir, { recursive: true }); + fs.writeFileSync(path.join(ccsDir, 'config.json'), JSON.stringify({ profiles: {} }, null, 2) + '\n'); + + const result = importApiProfileBundle({ + schemaVersion: 1, + exportedAt: new Date().toISOString(), + profile: { name: 'redacted-import', target: 'claude' }, + settings: { + env: { + ANTHROPIC_BASE_URL: 'https://api.example.com', + ANTHROPIC_AUTH_TOKEN: '__CCS_REDACTED__', + OPENROUTER_API_KEY: '__CCS_REDACTED__', + }, + }, + }); + + expect(result.success).toBe(true); + expect(result.warnings?.length).toBeGreaterThan(0); + + const settingsPath = path.join(ccsDir, 'redacted-import.settings.json'); + const parsed = JSON.parse(fs.readFileSync(settingsPath, 'utf8')) as { + env: Record; + }; + expect(parsed.env.ANTHROPIC_AUTH_TOKEN).toBe(''); + expect(parsed.env.OPENROUTER_API_KEY).toBe(''); + }); +}); diff --git a/tests/unit/web-server/profile-routes-lifecycle.test.ts b/tests/unit/web-server/profile-routes-lifecycle.test.ts index 6f7ddcb9..a79ae8e1 100644 --- a/tests/unit/web-server/profile-routes-lifecycle.test.ts +++ b/tests/unit/web-server/profile-routes-lifecycle.test.ts @@ -95,6 +95,42 @@ describe('profile-routes lifecycle endpoints', () => { expect(body.skipped).toEqual([]); }); + it('rejects malformed names payload for orphan registration', async () => { + const response = await fetch(`${baseUrl}/api/profiles/orphans/register`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ names: 'lonely' }), + }); + + expect(response.status).toBe(400); + const body = (await response.json()) as { error: string }; + expect(body.error).toContain('names must be an array'); + }); + + it('rejects import bundle with invalid profile target', async () => { + const response = await fetch(`${baseUrl}/api/profiles/import`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + bundle: { + schemaVersion: 1, + exportedAt: new Date().toISOString(), + profile: { name: 'demo', target: 'invalid' }, + settings: { + env: { + ANTHROPIC_BASE_URL: 'https://api.example.com', + ANTHROPIC_AUTH_TOKEN: 'token', + }, + }, + }, + }), + }); + + expect(response.status).toBe(400); + const body = (await response.json()) as { error: string }; + expect(body.error).toContain('Invalid bundle profile target'); + }); + it('validates source profile name on export endpoint', async () => { const response = await fetch(`${baseUrl}/api/profiles/1invalid/export`, { method: 'POST', @@ -107,4 +143,3 @@ describe('profile-routes lifecycle endpoints', () => { expect(body.error).toContain('API name must start with letter'); }); }); - diff --git a/ui/src/hooks/use-profiles.ts b/ui/src/hooks/use-profiles.ts index 2922c588..5e46dcd3 100644 --- a/ui/src/hooks/use-profiles.ts +++ b/ui/src/hooks/use-profiles.ts @@ -70,9 +70,6 @@ export function useDeleteProfile() { export function useDiscoverProfileOrphans() { return useMutation({ mutationFn: () => api.profiles.discoverOrphans(), - onError: (error: Error) => { - toast.error(error.message); - }, }); } @@ -85,9 +82,6 @@ export function useRegisterProfileOrphans() { queryClient.invalidateQueries({ queryKey: ['profiles'] }); toast.success('Orphan profiles registration complete'); }, - onError: (error: Error) => { - toast.error(error.message); - }, }); } @@ -101,9 +95,6 @@ export function useCopyProfile() { queryClient.invalidateQueries({ queryKey: ['profiles'] }); toast.success('Profile copied successfully'); }, - onError: (error: Error) => { - toast.error(error.message); - }, }); } @@ -111,9 +102,6 @@ export function useExportProfile() { return useMutation({ mutationFn: ({ name, includeSecrets }: { name: string; includeSecrets?: boolean }) => api.profiles.export(name, includeSecrets ?? false), - onError: (error: Error) => { - toast.error(error.message); - }, }); } @@ -126,8 +114,5 @@ export function useImportProfile() { queryClient.invalidateQueries({ queryKey: ['profiles'] }); toast.success('Profile imported successfully'); }, - onError: (error: Error) => { - toast.error(error.message); - }, }); } diff --git a/ui/src/lib/api-client.ts b/ui/src/lib/api-client.ts index 7f178efe..b18dd2ad 100644 --- a/ui/src/lib/api-client.ts +++ b/ui/src/lib/api-client.ts @@ -164,6 +164,13 @@ export interface CopyProfileRequest { force?: boolean; } +export interface CopyProfileResponse { + success: boolean; + name?: string; + settingsPath?: string; + warnings?: string[]; +} + export interface ApiProfileExportBundle { schemaVersion: 1; exportedAt: string; @@ -711,7 +718,7 @@ export const api = { body: JSON.stringify(data), }), copy: (name: string, data: CopyProfileRequest) => - request(`/profiles/${name}/copy`, { + request(`/profiles/${name}/copy`, { method: 'POST', body: JSON.stringify(data), }), diff --git a/ui/src/pages/api.tsx b/ui/src/pages/api.tsx index 1952da56..b1f1c734 100644 --- a/ui/src/pages/api.tsx +++ b/ui/src/pages/api.tsx @@ -82,6 +82,8 @@ export function ApiPage() { onSuccess: () => { if (selectedProfile === name) { setSelectedProfile(null); + setEditorHasChanges(false); + setPendingSwitch(null); } setDeleteConfirm(null); }, @@ -135,18 +137,26 @@ export function ApiPage() { const handleCopySelectedProfile = async () => { if (!selectedProfileData) return; - const destination = window.prompt( + const destinationInput = window.prompt( `Copy profile "${selectedProfileData.name}" to new profile name:`, `${selectedProfileData.name}-copy` ); - if (!destination) return; + if (!destinationInput) return; + const destination = destinationInput.trim(); + if (!destination) { + toast.error('Destination profile name cannot be empty'); + return; + } try { - await copyProfileMutation.mutateAsync({ + const result = await copyProfileMutation.mutateAsync({ name: selectedProfileData.name, - data: { destination: destination.trim() }, + data: { destination }, }); - switchToProfile(destination.trim()); + switchToProfile(destination); + if (result.warnings && result.warnings.length > 0) { + toast.info(result.warnings.join('\n')); + } } catch (error) { toast.error((error as Error).message); } @@ -210,6 +220,8 @@ export function ApiPage() { variant="outline" onClick={() => void handleDiscoverOrphans()} disabled={discoverOrphansMutation.isPending || registerOrphansMutation.isPending} + aria-label="Discover orphan profiles" + title="Discover orphan profiles" >