fix: validate required config fields before save (#225)

* fix(ui): validate BASE_URL and AUTH_TOKEN before save

- Add validation in use-provider-editor to block save when required
  env vars (ANTHROPIC_BASE_URL, ANTHROPIC_AUTH_TOKEN) are missing
- Show warning banner in raw editor when required fields are absent
- Always apply default preset on account add (not just first account)

This implements defense-in-depth: validation UI prevents confusion,
save-time validation prevents broken configs, auto-preset ensures
new providers start with working configuration.

Fixes #224

* fix: add validation to remaining settings routes and UI hooks

Backend:
- settings-routes.ts: Validate ANTHROPIC_BASE_URL and AUTH_TOKEN on PUT
- profile-routes.ts: Validate baseUrl/apiKey not empty on update
- variant-routes.ts: Require model field for variant creation

Frontend:
- use-profile-editor.ts: Add required field validation + error handling
- use-copilot-config-form.ts: Add required field validation + error handling

Both hooks now expose missingRequiredFields for UI warnings.

Ref #224

* feat(ux): auto-fill missing BASE_URL/AUTH_TOKEN from defaults at runtime

Instead of blocking saves when required env vars are missing, the system now:
- Runtime fills missing values from getClaudeEnvVars() defaults
- Backend returns warning in response (not 400 error)
- UI shows informational toast instead of blocking error
- Yellow warning banner still shown for user awareness

This provides better UX for CLIProxy users - saves always work, and
missing fields automatically use sensible defaults (local proxy URL
and global API key).

* fix(api): validate settings object before write to prevent undefined file content

Add validation to PUT /api/settings/:profile to return 400 if settings
object is undefined/null. Previously JSON.stringify(undefined) would
write literal "undefined" string to settings file.

* fix: add port validation and pre-save warning UI

- Add validatePort() helper (1-65535 range) to config-generator.ts
- Add missing field warning banner to copilot/config-form UI
- Add missing field warning banner to profiles/editor UI
- Wire missingRequiredFields prop through components
This commit is contained in:
Kai (Tam Nhu) Tran
2025-12-29 14:09:27 -08:00
committed by GitHub
parent 68835aee8e
commit 388428bba9
15 changed files with 266 additions and 25 deletions
+40
View File
@@ -23,6 +23,42 @@ interface ProviderSettings {
env: NodeJS.ProcessEnv;
}
/**
* Validate port is a valid positive integer (1-65535).
* Returns default port if invalid.
*/
function validatePort(port: number): number {
if (!Number.isFinite(port) || port < 1 || port > 65535 || !Number.isInteger(port)) {
return CLIPROXY_DEFAULT_PORT;
}
return port;
}
/**
* Ensure required CLIProxy env vars are present.
* Falls back to bundled defaults if missing from user settings.
* This prevents 404 errors when users forget to set BASE_URL/AUTH_TOKEN.
*/
function ensureRequiredEnvVars(
envVars: NodeJS.ProcessEnv,
provider: CLIProxyProvider,
port: number
): NodeJS.ProcessEnv {
const validPort = validatePort(port);
const result = { ...envVars };
const defaults = getClaudeEnvVars(provider, validPort);
// Fill in missing required vars from defaults
if (!result.ANTHROPIC_BASE_URL?.trim()) {
result.ANTHROPIC_BASE_URL = defaults.ANTHROPIC_BASE_URL;
}
if (!result.ANTHROPIC_AUTH_TOKEN?.trim()) {
result.ANTHROPIC_AUTH_TOKEN = defaults.ANTHROPIC_AUTH_TOKEN;
}
return result;
}
/** Default CLIProxy port */
export const CLIPROXY_DEFAULT_PORT = 8317;
@@ -563,6 +599,8 @@ export function getEffectiveEnvVars(
if (settings.env && typeof settings.env === 'object') {
// Custom variant settings found - merge with global env
envVars = { ...globalEnv, ...settings.env };
// Ensure required vars are present (fall back to defaults if missing)
envVars = ensureRequiredEnvVars(envVars, provider, port);
// Apply remote rewrite if configured
if (remoteRewriteConfig) {
envVars = rewriteLocalhostUrls(envVars, provider, remoteRewriteConfig);
@@ -590,6 +628,8 @@ export function getEffectiveEnvVars(
if (settings.env && typeof settings.env === 'object') {
// User override found - merge with global env
envVars = { ...globalEnv, ...settings.env };
// Ensure required vars are present (fall back to defaults if missing)
envVars = ensureRequiredEnvVars(envVars, provider, port);
// Apply remote rewrite if configured
if (remoteRewriteConfig) {
envVars = rewriteLocalhostUrls(envVars, provider, remoteRewriteConfig);
+10
View File
@@ -88,6 +88,16 @@ router.put('/:name', (req: Request, res: Response): void => {
return;
}
// Validate required fields if provided (prevent setting to empty)
if (baseUrl !== undefined && !baseUrl.trim()) {
res.status(400).json({ error: 'baseUrl cannot be empty' });
return;
}
if (apiKey !== undefined && !apiKey.trim()) {
res.status(400).json({ error: 'apiKey cannot be empty' });
return;
}
try {
updateSettingsFile(name, { baseUrl, apiKey, model, opusModel, sonnetModel, haikuModel });
res.json({ name, updated: true });
+24
View File
@@ -113,6 +113,15 @@ router.get('/:profile/raw', (req: Request, res: Response): void => {
}
});
/** Required env vars for CLIProxy providers to function */
const REQUIRED_ENV_KEYS = ['ANTHROPIC_BASE_URL', 'ANTHROPIC_AUTH_TOKEN'] as const;
/** Check if settings have required fields (returns missing list for warnings) */
function checkRequiredEnvVars(settings: Settings): string[] {
const env = settings?.env || {};
return REQUIRED_ENV_KEYS.filter((key) => !env[key]?.trim());
}
/**
* PUT /api/settings/:profile - Update settings with conflict detection and backup
*/
@@ -120,7 +129,17 @@ router.put('/:profile', (req: Request, res: Response): void => {
try {
const { profile } = req.params;
const { settings, expectedMtime } = req.body;
// Validate settings object exists
if (!settings || typeof settings !== 'object') {
res.status(400).json({ error: 'settings object is required in request body' });
return;
}
const ccsDir = getCcsDir();
// Check for missing required fields (warning, not blocking - runtime fills defaults)
const missingFields = checkRequiredEnvVars(settings);
const settingsPath = resolveSettingsPath(profile);
const fileExists = fs.existsSync(settingsPath);
@@ -165,6 +184,11 @@ router.put('/:profile', (req: Request, res: Response): void => {
mtime: newStat.mtime.getTime(),
backupPath,
created: !fileExists,
// Include warning if fields missing (runtime will use defaults)
...(missingFields.length > 0 && {
warning: `Missing fields will use defaults: ${missingFields.join(', ')}`,
missingFields,
}),
});
} catch (error) {
res.status(500).json({ error: (error as Error).message });
+7 -1
View File
@@ -63,8 +63,14 @@ router.post('/', (req: Request, res: Response): void => {
return;
}
// Require model for variant creation (prevents empty model causing issues)
if (!model || !model.trim()) {
res.status(400).json({ error: 'Missing required field: model' });
return;
}
// Use variant-service for proper port allocation
const result = createVariant(name, provider as CLIProxyProvider, model || '', account);
const result = createVariant(name, provider as CLIProxyProvider, model, account);
if (!result.success) {
res.status(409).json({ error: result.error });
@@ -1,7 +1,7 @@
/**
* Add Account Dialog Component
* Triggers OAuth flow server-side to add another account to a provider
* Applies default preset when adding first account
* Always applies default preset to ensure required env vars are set
* For Kiro: Also shows "Import from IDE" option as fallback
*/
@@ -26,7 +26,7 @@ interface AddAccountDialogProps {
onClose: () => void;
provider: string;
displayName: string;
/** Whether this is the first account being added (triggers preset application) */
/** Whether this is the first account being added (shows different toast message) */
isFirstAccount?: boolean;
}
@@ -49,14 +49,17 @@ export function AddAccountDialog({
{ provider, nickname: nickname.trim() || undefined },
{
onSuccess: async () => {
// Apply default preset if this is the first account
if (isFirstAccount) {
const result = await applyDefaultPreset(provider);
if (result.success && result.presetName) {
// Always apply default preset to ensure BASE_URL and AUTH_TOKEN are set
const result = await applyDefaultPreset(provider);
if (result.success && result.presetName) {
if (isFirstAccount) {
toast.success(`Applied "${result.presetName}" preset`);
} else if (!result.success) {
toast.warning('Account added, but failed to apply default preset');
}
// Silent success for non-first accounts - preset ensures required vars exist
} else if (!result.success) {
toast.warning(
'Account added, but failed to apply default preset. You may need to configure settings manually.'
);
}
setNickname('');
onClose();
@@ -68,12 +71,10 @@ export function AddAccountDialog({
const handleKiroImport = () => {
kiroImportMutation.mutate(undefined, {
onSuccess: async () => {
// Apply default preset if this is the first account
if (isFirstAccount) {
const result = await applyDefaultPreset('kiro');
if (result.success && result.presetName) {
toast.success(`Applied "${result.presetName}" preset`);
}
// Always apply default preset for Kiro as well
const result = await applyDefaultPreset('kiro');
if (result.success && result.presetName && isFirstAccount) {
toast.success(`Applied "${result.presetName}" preset`);
}
setNickname('');
onClose();
@@ -88,6 +88,7 @@ export function ProviderEditor({
saveMutation,
conflictDialog,
handleConflictResolve,
missingRequiredFields,
} = useProviderEditor(provider);
const accounts = authStatus.accounts || [];
@@ -216,6 +217,7 @@ export function ProviderEditor({
rawJsonEdits={rawJsonEdits}
onRawJsonChange={handleRawJsonChange}
profileEnv={data?.settings?.env}
missingRequiredFields={missingRequiredFields}
/>
</div>
</div>
@@ -4,7 +4,7 @@
*/
import { lazy, Suspense } from 'react';
import { Loader2, X } from 'lucide-react';
import { Loader2, X, AlertTriangle } from 'lucide-react';
import { GlobalEnvIndicator } from '@/components/shared/global-env-indicator';
import type { RawEditorSectionProps } from './types';
@@ -19,7 +19,10 @@ export function RawEditorSection({
rawJsonEdits,
onRawJsonChange,
profileEnv,
missingRequiredFields = [],
}: RawEditorSectionProps) {
const hasMissingFields = missingRequiredFields.length > 0;
return (
<Suspense
fallback={
@@ -36,6 +39,23 @@ export function RawEditorSection({
Invalid JSON syntax
</div>
)}
{isRawJsonValid && hasMissingFields && (
<div className="mb-2 px-3 py-2 bg-warning/10 text-warning-foreground text-sm rounded-md flex items-start gap-2 mx-6 mt-4 shrink-0 border border-warning/20">
<AlertTriangle className="w-4 h-4 mt-0.5 text-amber-500 shrink-0" />
<div>
<span className="font-medium text-amber-600 dark:text-amber-400">
Missing required fields:
</span>{' '}
<code className="text-xs bg-muted px-1 py-0.5 rounded">
{missingRequiredFields.join(', ')}
</code>
<p className="text-xs text-muted-foreground mt-1">
Apply a preset from Model Config tab or add these fields manually. Without them,
Claude Code will fail with 404 errors.
</p>
</div>
</div>
)}
<div className="flex-1 overflow-hidden px-6 pb-4 pt-4">
<div className="h-full border rounded-md overflow-hidden bg-background">
<CodeEditor
@@ -67,6 +67,8 @@ export interface RawEditorSectionProps {
rawJsonEdits: string | null;
onRawJsonChange: (value: string) => void;
profileEnv?: Record<string, string>;
/** List of required env vars that are missing (empty if all present) */
missingRequiredFields?: string[];
}
export interface ModelConfigSectionProps {
@@ -113,4 +115,6 @@ export interface UseProviderEditorReturn {
conflictDialog: boolean;
setConflictDialog: (open: boolean) => void;
handleConflictResolve: (overwrite: boolean) => Promise<void>;
/** List of required env vars that are missing (empty if all present) */
missingRequiredFields: string[];
}
@@ -8,6 +8,15 @@ import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query';
import { toast } from 'sonner';
import type { SettingsResponse, UseProviderEditorReturn } from './types';
/** Required env vars for CLIProxy providers (informational only - runtime fills defaults) */
const REQUIRED_ENV_KEYS = ['ANTHROPIC_BASE_URL', 'ANTHROPIC_AUTH_TOKEN'] as const;
/** Check settings for missing fields (for UI warnings) */
function checkMissingFields(settings: { env?: Record<string, string> }): string[] {
const env = settings?.env || {};
return REQUIRED_ENV_KEYS.filter((key) => !env[key]?.trim());
}
export function useProviderEditor(provider: string): UseProviderEditorReturn {
const [rawJsonEdits, setRawJsonEdits] = useState<string | null>(null);
const [conflictDialog, setConflictDialog] = useState(false);
@@ -95,10 +104,14 @@ export function useProviderEditor(provider: string): UseProviderEditorReturn {
return rawJsonEdits !== JSON.stringify(settings, null, 2);
}, [rawJsonEdits, settings]);
// Save mutation
// Validation state for missing required fields (informational warning)
const missingFields = useMemo(() => checkMissingFields(currentSettings), [currentSettings]);
// Save mutation (no blocking validation - runtime uses defaults)
const saveMutation = useMutation({
mutationFn: async () => {
const settingsToSave = JSON.parse(rawJsonContent);
const res = await fetch(`/api/settings/${provider}`, {
method: 'PUT',
headers: { 'Content-Type': 'application/json' },
@@ -112,10 +125,17 @@ export function useProviderEditor(provider: string): UseProviderEditorReturn {
if (!res.ok) throw new Error('Failed to save');
return res.json();
},
onSuccess: () => {
onSuccess: (responseData) => {
queryClient.invalidateQueries({ queryKey: ['settings', provider] });
setRawJsonEdits(null);
toast.success('Settings saved');
// Show warning if fields missing (runtime uses defaults)
if (responseData?.warning) {
toast.success('Settings saved', {
description: responseData.warning,
});
} else {
toast.success('Settings saved');
}
},
onError: (error: Error) => {
if (error.message === 'CONFLICT') {
@@ -159,5 +179,7 @@ export function useProviderEditor(provider: string): UseProviderEditorReturn {
conflictDialog,
setConflictDialog,
handleConflictResolve,
// Validation (informational)
missingRequiredFields: missingFields,
};
}
@@ -50,6 +50,7 @@ export function CopilotConfigForm() {
handleSave,
handleConflictResolve,
refetchRawSettings,
missingRequiredFields,
} = useCopilotConfigForm();
if (configLoading || rawSettingsLoading) {
@@ -146,6 +147,7 @@ export function CopilotConfigForm() {
rawJsonEdits={rawJsonEdits}
rawSettingsEnv={rawSettings?.settings?.env as Record<string, string> | undefined}
onChange={handleRawJsonChange}
missingRequiredFields={missingRequiredFields}
/>
</div>
</div>
@@ -4,7 +4,7 @@
*/
import { Suspense, lazy } from 'react';
import { Loader2, X } from 'lucide-react';
import { Loader2, X, AlertTriangle } from 'lucide-react';
import { GlobalEnvIndicator } from '@/components/shared/global-env-indicator';
// Lazy load CodeEditor
@@ -18,6 +18,7 @@ interface RawEditorSectionProps {
rawJsonEdits: string | null;
rawSettingsEnv: Record<string, string> | undefined;
onChange: (value: string) => void;
missingRequiredFields?: string[];
}
export function RawEditorSection({
@@ -26,7 +27,10 @@ export function RawEditorSection({
rawJsonEdits,
rawSettingsEnv,
onChange,
missingRequiredFields = [],
}: RawEditorSectionProps) {
const hasMissingFields = missingRequiredFields.length > 0;
return (
<Suspense
fallback={
@@ -43,6 +47,22 @@ export function RawEditorSection({
Invalid JSON syntax
</div>
)}
{isRawJsonValid && hasMissingFields && (
<div className="mb-2 px-3 py-2 bg-warning/10 text-warning-foreground text-sm rounded-md flex items-start gap-2 mx-6 mt-4 shrink-0 border border-warning/20">
<AlertTriangle className="w-4 h-4 mt-0.5 text-amber-500 shrink-0" />
<div>
<span className="font-medium text-amber-600 dark:text-amber-400">
Missing required fields:
</span>{' '}
<code className="text-xs bg-muted px-1 py-0.5 rounded">
{missingRequiredFields.join(', ')}
</code>
<p className="text-xs text-muted-foreground mt-1">
These fields will use default values at runtime.
</p>
</div>
</div>
)}
<div className="flex-1 overflow-hidden px-6 pb-4 pt-4">
<div className="h-full border rounded-md overflow-hidden bg-background">
<CodeEditor
@@ -8,6 +8,15 @@ import { useCopilot } from '@/hooks/use-copilot';
import { toast } from 'sonner';
import type { ModelPreset } from './types';
/** Required env vars for Copilot settings (informational only - runtime fills defaults) */
const REQUIRED_ENV_KEYS = ['ANTHROPIC_BASE_URL', 'ANTHROPIC_AUTH_TOKEN'] as const;
/** Check settings for missing fields (for UI warnings) */
function checkMissingFields(settings: { env?: Record<string, string> } | undefined): string[] {
const env = settings?.env || {};
return REQUIRED_ENV_KEYS.filter((key) => !env[key]?.trim());
}
export function useCopilotConfigForm() {
const {
config,
@@ -101,6 +110,23 @@ export function useCopilotConfigForm() {
return hasLocalChanges || hasJsonChanges;
}, [localOverrides, rawJsonEdits, rawSettings]);
// Validation state for missing required fields (informational warning)
const currentSettingsForValidation = useMemo(() => {
if (rawJsonEdits !== null) {
try {
return JSON.parse(rawJsonEdits);
} catch {
return rawSettings?.settings;
}
}
return rawSettings?.settings;
}, [rawJsonEdits, rawSettings?.settings]);
const missingFields = useMemo(
() => checkMissingFields(currentSettingsForValidation),
[currentSettingsForValidation]
);
const handleSave = async () => {
try {
// Save config changes
@@ -119,19 +145,31 @@ export function useCopilotConfigForm() {
});
}
// Save raw JSON changes
// Save raw JSON changes (no blocking validation - runtime uses defaults)
if (rawJsonEdits !== null && isRawJsonValid) {
const settingsToSave = JSON.parse(rawJsonContent);
const missing = checkMissingFields(settingsToSave);
await saveRawSettingsAsync({
settings: settingsToSave,
expectedMtime: rawSettings?.mtime,
});
// Show warning if fields missing
if (missing.length > 0) {
toast.success('Copilot configuration saved', {
description: `Missing fields will use defaults: ${missing.join(', ')}`,
});
} else {
toast.success('Copilot configuration saved');
}
} else {
toast.success('Copilot configuration saved');
}
// Clear local state
setLocalOverrides({});
setRawJsonEdits(null);
toast.success('Copilot configuration saved');
} catch (error) {
if ((error as Error).message === 'CONFLICT') {
setConflictDialog(true);
@@ -189,5 +227,8 @@ export function useCopilotConfigForm() {
handleSave,
handleConflictResolve,
refetchRawSettings,
/** List of required env vars that are missing (empty if all present) - informational */
missingRequiredFields: missingFields,
};
}
@@ -100,6 +100,13 @@ export function ProfileEditor({ profileName, onDelete, onHasChangesUpdate }: Pro
return Object.keys(localEdits).length > 0;
}, [rawJsonEdits, localEdits, settings]);
// Check for missing required fields (informational warning)
const missingRequiredFields = useMemo(() => {
const REQUIRED_ENV_KEYS = ['ANTHROPIC_BASE_URL', 'ANTHROPIC_AUTH_TOKEN'] as const;
const env = currentSettings?.env || {};
return REQUIRED_ENV_KEYS.filter((key) => !env[key]?.trim());
}, [currentSettings]);
// Notify parent of hasChanges state
useEffect(() => {
onHasChangesUpdate?.(computedHasChanges);
@@ -208,6 +215,7 @@ export function ProfileEditor({ profileName, onDelete, onHasChangesUpdate }: Pro
rawJsonEdits={rawJsonEdits}
settings={settings}
onChange={handleRawJsonChange}
missingRequiredFields={missingRequiredFields}
/>
</div>
</div>
@@ -4,7 +4,7 @@
*/
import { Suspense, lazy } from 'react';
import { Loader2, X } from 'lucide-react';
import { Loader2, X, AlertTriangle } from 'lucide-react';
import { GlobalEnvIndicator } from '@/components/shared/global-env-indicator';
import type { Settings } from './types';
@@ -19,6 +19,7 @@ interface RawEditorSectionProps {
rawJsonEdits: string | null;
settings: Settings | undefined;
onChange: (value: string) => void;
missingRequiredFields?: string[];
}
export function RawEditorSection({
@@ -27,7 +28,10 @@ export function RawEditorSection({
rawJsonEdits,
settings,
onChange,
missingRequiredFields = [],
}: RawEditorSectionProps) {
const hasMissingFields = missingRequiredFields.length > 0;
return (
<Suspense
fallback={
@@ -44,6 +48,22 @@ export function RawEditorSection({
Invalid JSON syntax
</div>
)}
{isRawJsonValid && hasMissingFields && (
<div className="mb-2 px-3 py-2 bg-warning/10 text-warning-foreground text-sm rounded-md flex items-start gap-2 mx-6 mt-4 shrink-0 border border-warning/20">
<AlertTriangle className="w-4 h-4 mt-0.5 text-amber-500 shrink-0" />
<div>
<span className="font-medium text-amber-600 dark:text-amber-400">
Missing required fields:
</span>{' '}
<code className="text-xs bg-muted px-1 py-0.5 rounded">
{missingRequiredFields.join(', ')}
</code>
<p className="text-xs text-muted-foreground mt-1">
These fields will use default values at runtime.
</p>
</div>
</div>
)}
<div className="flex-1 overflow-hidden px-6 pb-4 pt-4">
<div className="h-full border rounded-md overflow-hidden bg-background">
<CodeEditor
@@ -8,6 +8,15 @@ import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query';
import { toast } from 'sonner';
import type { Settings, SettingsResponse } from './types';
/** Required env vars for profiles to function (informational only - runtime fills defaults) */
const REQUIRED_ENV_KEYS = ['ANTHROPIC_BASE_URL', 'ANTHROPIC_AUTH_TOKEN'] as const;
/** Check settings for missing fields (for UI warnings) */
function checkMissingFields(settings: Settings | undefined): string[] {
const env = settings?.env || {};
return REQUIRED_ENV_KEYS.filter((key) => !env[key]?.trim());
}
interface UseProfileEditorOptions {
profileName: string;
localEdits: Record<string, string>;
@@ -78,6 +87,9 @@ export function useProfileEditor({
return Object.keys(localEdits).length > 0;
}, [rawJsonEdits, localEdits, query.data?.settings]);
// Validation state for missing required fields (informational warning)
const missingFields = useMemo(() => checkMissingFields(currentSettings), [currentSettings]);
// Save mutation
const saveMutation = useMutation({
mutationFn: async () => {
@@ -114,11 +126,18 @@ export function useProfileEditor({
return res.json();
},
onSuccess: () => {
onSuccess: (data) => {
queryClient.invalidateQueries({ queryKey: ['settings', profileName] });
queryClient.invalidateQueries({ queryKey: ['profiles'] });
onSuccess();
toast.success('Settings saved');
// Show warning if fields missing (runtime uses defaults)
if (data?.warning) {
toast.success('Settings saved', {
description: data.warning,
});
} else {
toast.success('Settings saved');
}
},
onError: (error: Error) => {
if (error.message === 'CONFLICT') {
@@ -135,5 +154,7 @@ export function useProfileEditor({
isRawJsonValid,
hasChanges,
saveMutation,
/** List of required env vars that are missing (empty if all present) - informational */
missingRequiredFields: missingFields,
};
}