refactor(routes): resolve DRY violations, config race, and error leaks

- Extract generic shouldCacheQuotaResult replacing 4 identical functions
- Move parseTarget to route-helpers.ts (was duplicated in profile/variant routes)
- Add createRouteErrorHelpers factory (was duplicated in auth/settings routes)
- Fix config read-then-write race in proxy-routes.ts and misc-routes.ts
  using mutateUnifiedConfig for atomic read-modify-write
- Replace raw error messages in 500 responses with generic messages +
  server-side logging across all cliproxy-stats-routes handlers
- Convert extractErrorLogMetadata from sync fs to async fs.promises
This commit is contained in:
Tam Nhu Tran
2026-03-04 01:30:46 +07:00
parent a4c5bb7421
commit e255855775
8 changed files with 189 additions and 211 deletions
+2 -17
View File
@@ -50,29 +50,14 @@ import {
validateAntigravityRiskAcknowledgement, validateAntigravityRiskAcknowledgement,
isAntigravityResponsibilityBypassEnabled, isAntigravityResponsibilityBypassEnabled,
} from '../../cliproxy/antigravity-responsibility'; } from '../../cliproxy/antigravity-responsibility';
import { createRouteErrorHelpers } from './route-helpers';
const router = Router(); const router = Router();
// Valid providers list - derived from canonical CLIPROXY_PROFILES // Valid providers list - derived from canonical CLIPROXY_PROFILES
const validProviders: CLIProxyProvider[] = [...CLIPROXY_PROFILES]; const validProviders: CLIProxyProvider[] = [...CLIPROXY_PROFILES];
function logRouteError(context: string, error: unknown): void { const { respondInternalError } = createRouteErrorHelpers('cliproxy-auth-routes');
if (error instanceof Error) {
console.error(`[cliproxy-auth-routes] ${context}: ${error.message}`);
return;
}
console.error(`[cliproxy-auth-routes] ${context}: unknown error`);
}
function respondInternalError(
res: Response,
error: unknown,
fallbackMessage: string,
statusCode = 500
): void {
logRouteError(fallbackMessage, error);
res.status(statusCode).json({ error: fallbackMessage });
}
function parseKiroMethod(raw: unknown): { method: KiroAuthMethod; invalid: boolean } { function parseKiroMethod(raw: unknown): { method: KiroAuthMethod; invalid: boolean } {
if (raw === undefined || raw === null) { if (raw === undefined || raw === null) {
+67 -84
View File
@@ -100,62 +100,21 @@ function isQuotaRouteRateLimited(req: Request, provider: string): boolean {
} }
/** /**
* Cache only stable failures; avoid pinning transient network failures (timeouts, 429s). * Cache only stable failures; skip transient network errors (timeouts, 429s, 5xx).
* Generic across all quota result types.
*/ */
function shouldCacheCodexQuotaResult(result: CodexQuotaResult): boolean { function shouldCacheQuotaResult(result: {
success: boolean;
needsReauth?: boolean;
isForbidden?: boolean;
error?: string;
}): boolean {
if (result.success) return true; if (result.success) return true;
if (result.needsReauth || result.isForbidden) return true; if (result.needsReauth || result.isForbidden) return true;
const msg = (result.error || '').toLowerCase(); const msg = (result.error || '').toLowerCase();
if (!msg) return false; if (!msg) return false;
if (msg.includes('timeout')) return false; const transientPatterns = ['timeout', 'rate limited', 'api error: 5', 'fetch failed'];
if (msg.includes('rate limited')) return false; return !transientPatterns.some((p) => msg.includes(p));
if (msg.includes('api error: 5')) return false;
if (msg.includes('fetch failed')) return false;
return false;
}
function shouldCacheGeminiQuotaResult(result: GeminiCliQuotaResult): boolean {
if (result.success) return true;
if (result.needsReauth) return true;
const msg = (result.error || '').toLowerCase();
if (!msg) return false;
if (msg.includes('timeout')) return false;
if (msg.includes('rate limited')) return false;
if (msg.includes('api error: 5')) return false;
if (msg.includes('fetch failed')) return false;
return false;
}
function shouldCacheClaudeQuotaResult(result: ClaudeQuotaResult): boolean {
if (result.success) return true;
if (result.needsReauth) return true;
const msg = (result.error || '').toLowerCase();
if (!msg) return false;
if (msg.includes('timeout')) return false;
if (msg.includes('rate limited')) return false;
if (msg.includes('api error: 5')) return false;
if (msg.includes('fetch failed')) return false;
return false;
}
function shouldCacheGhcpQuotaResult(result: GhcpQuotaResult): boolean {
if (result.success) return true;
if (result.needsReauth) return true;
const msg = (result.error || '').toLowerCase();
if (!msg) return false;
if (msg.includes('timeout')) return false;
if (msg.includes('rate limited')) return false;
if (msg.includes('api error: 5')) return false;
if (msg.includes('fetch failed')) return false;
return false;
} }
/** Get configured backend from config */ /** Get configured backend from config */
@@ -169,20 +128,21 @@ function getConfiguredBackend() {
} }
/** /**
* Extract status code and model from error log file (lightweight parsing) * Extract status code and model from error log file (lightweight parsing).
* Reads first 4KB for model, last 2KB for status code * Reads first 4KB for model, last 2KB for status code. Async to avoid blocking event loop.
*/ */
async function extractErrorLogMetadata( async function extractErrorLogMetadata(
filePath: string filePath: string
): Promise<{ statusCode?: number; model?: string }> { ): Promise<{ statusCode?: number; model?: string }> {
let fh: fs.promises.FileHandle | null = null;
try { try {
const fd = fs.openSync(filePath, 'r'); fh = await fs.promises.open(filePath, 'r');
const stat = fs.fstatSync(fd); const stat = await fh.stat();
const fileSize = stat.size; const fileSize = stat.size;
// Read first 4KB for model (in request body) // Read first 4KB for model (in request body)
const startBuffer = Buffer.alloc(Math.min(4096, fileSize)); const startBuffer = Buffer.alloc(Math.min(4096, fileSize));
fs.readSync(fd, startBuffer, 0, startBuffer.length, 0); await fh.read(startBuffer, 0, startBuffer.length, 0);
const startContent = startBuffer.toString('utf-8'); const startContent = startBuffer.toString('utf-8');
// Extract model from request body JSON: "model":"gemini-3-flash-preview" // Extract model from request body JSON: "model":"gemini-3-flash-preview"
@@ -193,7 +153,7 @@ async function extractErrorLogMetadata(
let statusCode: number | undefined; let statusCode: number | undefined;
if (fileSize > 2048) { if (fileSize > 2048) {
const endBuffer = Buffer.alloc(2048); const endBuffer = Buffer.alloc(2048);
fs.readSync(fd, endBuffer, 0, 2048, fileSize - 2048); await fh.read(endBuffer, 0, 2048, fileSize - 2048);
const endContent = endBuffer.toString('utf-8'); const endContent = endBuffer.toString('utf-8');
const statusMatch = endContent.match(/Status:\s*(\d{3})/); const statusMatch = endContent.match(/Status:\s*(\d{3})/);
statusCode = statusMatch ? parseInt(statusMatch[1], 10) : undefined; statusCode = statusMatch ? parseInt(statusMatch[1], 10) : undefined;
@@ -203,10 +163,11 @@ async function extractErrorLogMetadata(
statusCode = statusMatch ? parseInt(statusMatch[1], 10) : undefined; statusCode = statusMatch ? parseInt(statusMatch[1], 10) : undefined;
} }
fs.closeSync(fd);
return { statusCode, model }; return { statusCode, model };
} catch { } catch {
return {}; return {};
} finally {
await fh?.close();
} }
} }
@@ -237,7 +198,8 @@ const handleStatsRequest = async (_req: Request, res: Response): Promise<void> =
res.json(stats); res.json(stats);
} catch (error) { } catch (error) {
res.status(500).json({ error: (error as Error).message }); console.error(`[cliproxy-stats] ${(error as Error).message}`);
res.status(500).json({ error: 'Internal server error' });
} }
}; };
@@ -261,7 +223,8 @@ router.get('/status', async (_req: Request, res: Response): Promise<void> => {
const running = await isCliproxyRunning(); const running = await isCliproxyRunning();
res.json({ running }); res.json({ running });
} catch (error) { } catch (error) {
res.status(500).json({ error: (error as Error).message }); console.error(`[cliproxy-stats] ${(error as Error).message}`);
res.status(500).json({ error: 'Internal server error' });
} }
}); });
@@ -297,7 +260,8 @@ router.get('/proxy-status', async (_req: Request, res: Response): Promise<void>
res.json(sessionStatus); res.json(sessionStatus);
} }
} catch (error) { } catch (error) {
res.status(500).json({ error: (error as Error).message }); console.error(`[cliproxy-stats] ${(error as Error).message}`);
res.status(500).json({ error: 'Internal server error' });
} }
}); });
@@ -311,7 +275,8 @@ router.post('/proxy-start', async (_req: Request, res: Response): Promise<void>
const result = await ensureCliproxyService(); const result = await ensureCliproxyService();
res.json(result); res.json(result);
} catch (error) { } catch (error) {
res.status(500).json({ error: (error as Error).message }); console.error(`[cliproxy-stats] ${(error as Error).message}`);
res.status(500).json({ error: 'Internal server error' });
} }
}); });
@@ -324,7 +289,8 @@ router.post('/proxy-stop', async (_req: Request, res: Response): Promise<void> =
const result = await stopProxy(); const result = await stopProxy();
res.json(result); res.json(result);
} catch (error) { } catch (error) {
res.status(500).json({ error: (error as Error).message }); console.error(`[cliproxy-stats] ${(error as Error).message}`);
res.status(500).json({ error: 'Internal server error' });
} }
}); });
@@ -338,7 +304,8 @@ router.get('/update-check', async (_req: Request, res: Response): Promise<void>
const result = await checkCliproxyUpdate(backend); const result = await checkCliproxyUpdate(backend);
res.json(result); res.json(result);
} catch (error) { } catch (error) {
res.status(500).json({ error: (error as Error).message }); console.error(`[cliproxy-stats] ${(error as Error).message}`);
res.status(500).json({ error: 'Internal server error' });
} }
}); });
@@ -370,7 +337,8 @@ router.get('/models', async (_req: Request, res: Response): Promise<void> => {
res.json(modelsResponse); res.json(modelsResponse);
} catch (error) { } catch (error) {
res.status(500).json({ error: (error as Error).message }); console.error(`[cliproxy-stats] ${(error as Error).message}`);
res.status(500).json({ error: 'Internal server error' });
} }
}); });
@@ -417,7 +385,8 @@ router.get('/error-logs', async (_req: Request, res: Response): Promise<void> =>
res.json({ files: filesWithMetadata }); res.json({ files: filesWithMetadata });
} catch (error) { } catch (error) {
res.status(500).json({ error: (error as Error).message }); console.error(`[cliproxy-stats] ${(error as Error).message}`);
res.status(500).json({ error: 'Internal server error' });
} }
}); });
@@ -456,7 +425,8 @@ router.get('/error-logs/:name', async (req: Request, res: Response): Promise<voi
res.type('text/plain').send(content); res.type('text/plain').send(content);
} catch (error) { } catch (error) {
res.status(500).json({ error: (error as Error).message }); console.error(`[cliproxy-stats] ${(error as Error).message}`);
res.status(500).json({ error: 'Internal server error' });
} }
}); });
@@ -477,7 +447,8 @@ router.get('/config.yaml', async (_req: Request, res: Response): Promise<void> =
const content = fs.readFileSync(configPath, 'utf8'); const content = fs.readFileSync(configPath, 'utf8');
res.type('text/yaml').send(content); res.type('text/yaml').send(content);
} catch (error) { } catch (error) {
res.status(500).json({ error: (error as Error).message }); console.error(`[cliproxy-stats] ${(error as Error).message}`);
res.status(500).json({ error: 'Internal server error' });
} }
}); });
@@ -510,7 +481,8 @@ router.put('/config.yaml', async (req: Request, res: Response): Promise<void> =>
res.json({ success: true, path: configPath }); res.json({ success: true, path: configPath });
} catch (error) { } catch (error) {
res.status(500).json({ error: (error as Error).message }); console.error(`[cliproxy-stats] ${(error as Error).message}`);
res.status(500).json({ error: 'Internal server error' });
} }
}); });
@@ -544,7 +516,8 @@ router.get('/auth-files', async (_req: Request, res: Response): Promise<void> =>
res.json({ files, directory: authDir }); res.json({ files, directory: authDir });
} catch (error) { } catch (error) {
res.status(500).json({ error: (error as Error).message }); console.error(`[cliproxy-stats] ${(error as Error).message}`);
res.status(500).json({ error: 'Internal server error' });
} }
}); });
@@ -580,7 +553,8 @@ router.get('/auth-files/download', async (req: Request, res: Response): Promise<
res.setHeader('Content-Disposition', `attachment; filename="${name}"`); res.setHeader('Content-Disposition', `attachment; filename="${name}"`);
res.type('application/octet-stream').send(content); res.type('application/octet-stream').send(content);
} catch (error) { } catch (error) {
res.status(500).json({ error: (error as Error).message }); console.error(`[cliproxy-stats] ${(error as Error).message}`);
res.status(500).json({ error: 'Internal server error' });
} }
}); });
@@ -667,7 +641,8 @@ router.put('/models/:provider', async (req: Request, res: Response): Promise<voi
res.json({ success: true, provider, model: canonicalModel }); res.json({ success: true, provider, model: canonicalModel });
} catch (error) { } catch (error) {
res.status(500).json({ error: (error as Error).message }); console.error(`[cliproxy-stats] ${(error as Error).message}`);
res.status(500).json({ error: 'Internal server error' });
} }
}); });
@@ -712,13 +687,14 @@ router.get('/quota/codex/:accountId', async (req: Request, res: Response): Promi
const result = await fetchCodexQuota(accountId); const result = await fetchCodexQuota(accountId);
// Cache successful and stable failure states; skip transient network failures. // Cache successful and stable failure states; skip transient network failures.
if (shouldCacheCodexQuotaResult(result)) { if (shouldCacheQuotaResult(result)) {
setCachedQuota('codex', accountId, result); setCachedQuota('codex', accountId, result);
} }
res.json(result); res.json(result);
} catch (error) { } catch (error) {
res.status(500).json({ error: (error as Error).message }); console.error(`[cliproxy-stats] ${(error as Error).message}`);
res.status(500).json({ error: 'Internal server error' });
} }
}); });
@@ -759,13 +735,14 @@ router.get('/quota/claude/:accountId', async (req: Request, res: Response): Prom
const result = await fetchClaudeQuota(accountId); const result = await fetchClaudeQuota(accountId);
// Cache successful and stable failure states; skip transient network failures. // Cache successful and stable failure states; skip transient network failures.
if (shouldCacheClaudeQuotaResult(result)) { if (shouldCacheQuotaResult(result)) {
setCachedQuota('claude', accountId, result); setCachedQuota('claude', accountId, result);
} }
res.json(result); res.json(result);
} catch (error) { } catch (error) {
res.status(500).json({ error: (error as Error).message }); console.error(`[cliproxy-stats] ${(error as Error).message}`);
res.status(500).json({ error: 'Internal server error' });
} }
}); });
@@ -806,13 +783,14 @@ router.get('/quota/gemini/:accountId', async (req: Request, res: Response): Prom
const result = await fetchGeminiCliQuota(accountId); const result = await fetchGeminiCliQuota(accountId);
// Cache successful and stable failure states; skip transient network failures. // Cache successful and stable failure states; skip transient network failures.
if (shouldCacheGeminiQuotaResult(result)) { if (shouldCacheQuotaResult(result)) {
setCachedQuota('gemini', accountId, result); setCachedQuota('gemini', accountId, result);
} }
res.json(result); res.json(result);
} catch (error) { } catch (error) {
res.status(500).json({ error: (error as Error).message }); console.error(`[cliproxy-stats] ${(error as Error).message}`);
res.status(500).json({ error: 'Internal server error' });
} }
}); });
@@ -853,13 +831,14 @@ router.get('/quota/ghcp/:accountId', async (req: Request, res: Response): Promis
const result = await fetchGhcpQuota(accountId); const result = await fetchGhcpQuota(accountId);
// Cache successful and stable failure states; skip transient network failures. // Cache successful and stable failure states; skip transient network failures.
if (shouldCacheGhcpQuotaResult(result)) { if (shouldCacheQuotaResult(result)) {
setCachedQuota('ghcp', accountId, result); setCachedQuota('ghcp', accountId, result);
} }
res.json(result); res.json(result);
} catch (error) { } catch (error) {
res.status(500).json({ error: (error as Error).message }); console.error(`[cliproxy-stats] ${(error as Error).message}`);
res.status(500).json({ error: 'Internal server error' });
} }
}); });
@@ -917,7 +896,8 @@ router.get('/quota/:provider/:accountId', async (req: Request, res: Response): P
res.json(result); res.json(result);
} catch (error) { } catch (error) {
res.status(500).json({ error: (error as Error).message }); console.error(`[cliproxy-stats] ${(error as Error).message}`);
res.status(500).json({ error: 'Internal server error' });
} }
}); });
@@ -940,7 +920,8 @@ router.get('/versions', async (_req: Request, res: Response): Promise<void> => {
faultyRange: CLIPROXY_FAULTY_RANGE, faultyRange: CLIPROXY_FAULTY_RANGE,
}); });
} catch (error) { } catch (error) {
res.status(500).json({ error: (error as Error).message }); console.error(`[cliproxy-stats] ${(error as Error).message}`);
res.status(500).json({ error: 'Internal server error' });
} }
}); });
@@ -1004,7 +985,8 @@ router.post('/install', async (req: Request, res: Response): Promise<void> => {
message: `Successfully installed CLIProxy Plus v${version}`, message: `Successfully installed CLIProxy Plus v${version}`,
}); });
} catch (error) { } catch (error) {
res.status(500).json({ error: (error as Error).message }); console.error(`[cliproxy-stats] ${(error as Error).message}`);
res.status(500).json({ error: 'Internal server error' });
} }
}); });
@@ -1029,7 +1011,8 @@ router.post('/restart', async (_req: Request, res: Response): Promise<void> => {
res.json({ success: false, error: startResult.error || 'Failed to start proxy' }); res.json({ success: false, error: startResult.error || 'Failed to start proxy' });
} }
} catch (error) { } catch (error) {
res.status(500).json({ error: (error as Error).message }); console.error(`[cliproxy-stats] ${(error as Error).message}`);
res.status(500).json({ error: 'Internal server error' });
} }
}); });
+18 -18
View File
@@ -10,6 +10,7 @@ import { expandPath } from '../../utils/helpers';
import { import {
loadOrCreateUnifiedConfig, loadOrCreateUnifiedConfig,
saveUnifiedConfig, saveUnifiedConfig,
mutateUnifiedConfig,
getGlobalEnvConfig, getGlobalEnvConfig,
getThinkingConfig, getThinkingConfig,
getConfigYamlPath, getConfigYamlPath,
@@ -218,28 +219,27 @@ router.get('/global-env', (_req: Request, res: Response): void => {
* Updates the global_env section in config.yaml * Updates the global_env section in config.yaml
*/ */
router.put('/global-env', (req: Request, res: Response): void => { router.put('/global-env', (req: Request, res: Response): void => {
try { const { enabled, env } = req.body;
const { enabled, env } = req.body;
const config = loadOrCreateUnifiedConfig();
// Validate env is an object with string values // Validate env values before acquiring write lock
if (env !== undefined && typeof env === 'object' && env !== null) { if (env !== undefined && typeof env === 'object' && env !== null) {
for (const [key, value] of Object.entries(env)) { for (const [key, value] of Object.entries(env)) {
if (typeof value !== 'string') { if (typeof value !== 'string') {
res.status(400).json({ error: `Invalid value for ${key}: must be a string` }); res.status(400).json({ error: `Invalid value for ${key}: must be a string` });
return; return;
}
} }
} }
}
// Update global_env section try {
config.global_env = { // Atomic read-modify-write — avoids race between load and save
enabled: enabled ?? config.global_env?.enabled ?? true, const updated = mutateUnifiedConfig((config) => {
env: env ?? config.global_env?.env ?? {}, config.global_env = {
}; enabled: enabled ?? config.global_env?.enabled ?? true,
env: env ?? config.global_env?.env ?? {},
saveUnifiedConfig(config); };
res.json({ success: true, config: config.global_env }); });
res.json({ success: true, config: updated.global_env });
} catch (error) { } catch (error) {
res.status(500).json({ error: (error as Error).message }); res.status(500).json({ error: (error as Error).message });
} }
+1 -19
View File
@@ -13,9 +13,8 @@ import {
updateApiProfileTarget, updateApiProfileTarget,
} from '../../api/services/profile-writer'; } from '../../api/services/profile-writer';
import { apiProfileExists, listApiProfiles } from '../../api/services/profile-reader'; import { apiProfileExists, listApiProfiles } from '../../api/services/profile-reader';
import type { TargetType } from '../../targets/target-adapter';
import { normalizeDroidProvider } from '../../targets/droid-provider'; import { normalizeDroidProvider } from '../../targets/droid-provider';
import { updateSettingsFile } from './route-helpers'; import { updateSettingsFile, parseTarget } from './route-helpers';
const router = Router(); const router = Router();
@@ -23,23 +22,6 @@ function isDenylistError(message: string | undefined): boolean {
return typeof message === 'string' && message.toLowerCase().includes('denylist'); return typeof message === 'string' && message.toLowerCase().includes('denylist');
} }
export function parseTarget(rawTarget: unknown): TargetType | null {
if (rawTarget === undefined || rawTarget === null || rawTarget === '') {
return null;
}
if (typeof rawTarget !== 'string') {
return null;
}
const normalized = rawTarget.trim().toLowerCase();
if (normalized === 'claude' || normalized === 'droid') {
return normalized;
}
return null;
}
// ==================== Profile CRUD ==================== // ==================== Profile CRUD ====================
/** /**
+41 -38
View File
@@ -8,7 +8,7 @@
*/ */
import { Router, Request, Response } from 'express'; import { Router, Request, Response } from 'express';
import { loadOrCreateUnifiedConfig, saveUnifiedConfig } from '../../config/unified-config-loader'; import { loadOrCreateUnifiedConfig, mutateUnifiedConfig } from '../../config/unified-config-loader';
import { testConnection } from '../../cliproxy/remote-proxy-client'; import { testConnection } from '../../cliproxy/remote-proxy-client';
import { isProxyRunning } from '../../cliproxy/services/proxy-lifecycle-service'; import { isProxyRunning } from '../../cliproxy/services/proxy-lifecycle-service';
import { DEFAULT_BACKEND } from '../../cliproxy/platform-detector'; import { DEFAULT_BACKEND } from '../../cliproxy/platform-detector';
@@ -36,32 +36,32 @@ router.get('/', async (_req: Request, res: Response) => {
/** /**
* PUT /api/cliproxy-server - Update proxy configuration * PUT /api/cliproxy-server - Update proxy configuration
*/ */
router.put('/', async (req: Request, res: Response) => { router.put('/', (req: Request, res: Response) => {
try { try {
const config = await loadOrCreateUnifiedConfig();
const updates = req.body as Partial<CliproxyServerConfig>; const updates = req.body as Partial<CliproxyServerConfig>;
// Deep merge with defaults and current config // Atomic read-modify-write — avoids race between load and save
config.cliproxy_server = { const updated = mutateUnifiedConfig((config) => {
remote: { config.cliproxy_server = {
...DEFAULT_CLIPROXY_SERVER_CONFIG.remote, remote: {
...config.cliproxy_server?.remote, ...DEFAULT_CLIPROXY_SERVER_CONFIG.remote,
...updates.remote, ...config.cliproxy_server?.remote,
}, ...updates.remote,
fallback: { },
...DEFAULT_CLIPROXY_SERVER_CONFIG.fallback, fallback: {
...config.cliproxy_server?.fallback, ...DEFAULT_CLIPROXY_SERVER_CONFIG.fallback,
...updates.fallback, ...config.cliproxy_server?.fallback,
}, ...updates.fallback,
local: { },
...DEFAULT_CLIPROXY_SERVER_CONFIG.local, local: {
...config.cliproxy_server?.local, ...DEFAULT_CLIPROXY_SERVER_CONFIG.local,
...updates.local, ...config.cliproxy_server?.local,
}, ...updates.local,
}; },
};
});
await saveUnifiedConfig(config); res.json(updated.cliproxy_server);
res.json(config.cliproxy_server);
} catch (error) { } catch (error) {
console.error('[cliproxy-server-routes] Failed to save proxy config:', error); console.error('[cliproxy-server-routes] Failed to save proxy config:', error);
res.status(500).json({ error: 'Failed to save proxy config' }); res.status(500).json({ error: 'Failed to save proxy config' });
@@ -91,7 +91,7 @@ router.get('/backend', async (_req: Request, res: Response) => {
* @throws {400} Invalid backend value * @throws {400} Invalid backend value
* @throws {409} Proxy is running (unless force=true) * @throws {409} Proxy is running (unless force=true)
*/ */
router.put('/backend', async (req: Request, res: Response) => { router.put('/backend', (req: Request, res: Response) => {
try { try {
const { backend, force } = req.body; const { backend, force } = req.body;
if (backend !== 'original' && backend !== 'plus') { if (backend !== 'original' && backend !== 'plus') {
@@ -99,9 +99,9 @@ router.put('/backend', async (req: Request, res: Response) => {
return; return;
} }
// Check if proxy is running - warn about restart requirement // Pre-flight read: check running state before acquiring write lock
const config = await loadOrCreateUnifiedConfig(); const currentConfig = loadOrCreateUnifiedConfig();
const currentBackend = config.cliproxy?.backend ?? DEFAULT_BACKEND; const currentBackend = currentConfig.cliproxy?.backend ?? DEFAULT_BACKEND;
if (currentBackend !== backend && isProxyRunning() && !force) { if (currentBackend !== backend && isProxyRunning() && !force) {
res.status(409).json({ res.status(409).json({
error: 'Proxy is running. Stop proxy first or use force=true to change backend.', error: 'Proxy is running. Stop proxy first or use force=true to change backend.',
@@ -110,18 +110,21 @@ router.put('/backend', async (req: Request, res: Response) => {
}); });
return; return;
} }
if (!config.cliproxy) {
config.cliproxy = {
backend,
oauth_accounts: {},
providers: [...CLIPROXY_PROVIDER_IDS],
variants: {},
};
} else {
config.cliproxy.backend = backend;
}
await saveUnifiedConfig(config); // Atomic write — avoids race between load and save
mutateUnifiedConfig((config) => {
if (!config.cliproxy) {
config.cliproxy = {
backend,
oauth_accounts: {},
providers: [...CLIPROXY_PROVIDER_IDS],
variants: {},
};
} else {
config.cliproxy.backend = backend;
}
});
res.json({ backend }); res.json({ backend });
} catch (error) { } catch (error) {
console.error('[cliproxy-server-routes] Failed to save backend config:', error); console.error('[cliproxy-server-routes] Failed to save backend config:', error);
+57
View File
@@ -4,6 +4,7 @@
import * as fs from 'fs'; import * as fs from 'fs';
import * as path from 'path'; import * as path from 'path';
import type { Response } from 'express';
import { getCcsDir, getConfigPath, loadConfigSafe, loadSettings } from '../../utils/config-manager'; import { getCcsDir, getConfigPath, loadConfigSafe, loadSettings } from '../../utils/config-manager';
import { expandPath } from '../../utils/helpers'; import { expandPath } from '../../utils/helpers';
import { getClaudeSettingsPath } from '../../utils/claude-config-path'; import { getClaudeSettingsPath } from '../../utils/claude-config-path';
@@ -16,6 +17,7 @@ import {
} from '../../cliproxy/model-id-normalizer'; } from '../../cliproxy/model-id-normalizer';
import type { CLIProxyProvider } from '../../cliproxy/types'; import type { CLIProxyProvider } from '../../cliproxy/types';
import type { Config, Settings } from '../../types/config'; import type { Config, Settings } from '../../types/config';
import type { TargetType } from '../../targets/target-adapter';
import { ValidationError } from '../../errors/error-types'; import { ValidationError } from '../../errors/error-types';
/** Model mapping for API profiles */ /** Model mapping for API profiles */
@@ -389,3 +391,58 @@ export function validateFilePath(filePath: string): {
return { valid: false, readonly: false, error: 'Access to this path is not allowed' }; return { valid: false, readonly: false, error: 'Access to this path is not allowed' };
} }
/**
* Parse and validate a target param (claude/droid). Returns null if invalid/absent.
* Shared by profile-routes and variant-routes.
*/
export function parseTarget(rawTarget: unknown): TargetType | null {
if (rawTarget === undefined || rawTarget === null || rawTarget === '') {
return null;
}
if (typeof rawTarget !== 'string') {
return null;
}
const normalized = rawTarget.trim().toLowerCase();
if (normalized === 'claude' || normalized === 'droid') {
return normalized;
}
return null;
}
/**
* Create route-specific error helpers with a log prefix.
* Eliminates duplicate logRouteError/respondInternalError in each route file.
*/
export function createRouteErrorHelpers(prefix: string): {
logRouteError: (context: string, error: unknown) => void;
respondInternalError: (
res: Response,
error: unknown,
fallbackMessage: string,
statusCode?: number
) => void;
} {
function logRouteError(context: string, error: unknown): void {
if (error instanceof Error) {
console.error(`[${prefix}] ${context}: ${error.message}`);
return;
}
console.error(`[${prefix}] ${context}: unknown error`);
}
function respondInternalError(
res: Response,
error: unknown,
fallbackMessage: string,
statusCode = 500
): void {
logRouteError(fallbackMessage, error);
res.status(statusCode).json({ error: fallbackMessage });
}
return { logRouteError, respondInternalError };
}
+2 -17
View File
@@ -32,6 +32,7 @@ import {
extractProviderFromPathname, extractProviderFromPathname,
getDeniedModelIdReasonForProvider, getDeniedModelIdReasonForProvider,
} from '../../cliproxy/model-id-normalizer'; } from '../../cliproxy/model-id-normalizer';
import { createRouteErrorHelpers } from './route-helpers';
const router = Router(); const router = Router();
const MODEL_ENV_KEYS = [ const MODEL_ENV_KEYS = [
@@ -42,23 +43,7 @@ const MODEL_ENV_KEYS = [
] as const; ] as const;
const PRESET_MODEL_KEYS = ['default', 'opus', 'sonnet', 'haiku'] as const; const PRESET_MODEL_KEYS = ['default', 'opus', 'sonnet', 'haiku'] as const;
function logRouteError(context: string, error: unknown): void { const { logRouteError, respondInternalError } = createRouteErrorHelpers('settings-routes');
if (error instanceof Error) {
console.error(`[settings-routes] ${context}: ${error.message}`);
return;
}
console.error(`[settings-routes] ${context}: unknown error`);
}
function respondInternalError(
res: Response,
error: unknown,
fallbackMessage: string,
statusCode = 500
): void {
logRouteError(fallbackMessage, error);
res.status(statusCode).json({ error: fallbackMessage });
}
function isLoopbackAddress(value: string | undefined): boolean { function isLoopbackAddress(value: string | undefined): boolean {
if (!value) return false; if (!value) return false;
+1 -18
View File
@@ -7,7 +7,7 @@
import { Router, Request, Response } from 'express'; import { Router, Request, Response } from 'express';
import { isReservedName, RESERVED_PROFILE_NAMES } from '../../config/reserved-names'; import { isReservedName, RESERVED_PROFILE_NAMES } from '../../config/reserved-names';
import type { CLIProxyProvider } from '../../cliproxy/types'; import type { CLIProxyProvider } from '../../cliproxy/types';
import type { TargetType } from '../../targets/target-adapter'; import { parseTarget } from './route-helpers';
import { import {
createVariant, createVariant,
removeVariant, removeVariant,
@@ -24,23 +24,6 @@ import {
const router = Router(); const router = Router();
export function parseTarget(rawTarget: unknown): TargetType | null {
if (rawTarget === undefined || rawTarget === null || rawTarget === '') {
return null;
}
if (typeof rawTarget !== 'string') {
return null;
}
const normalized = rawTarget.trim().toLowerCase();
if (normalized === 'claude' || normalized === 'droid') {
return normalized;
}
return null;
}
/** /**
* GET /api/cliproxy - List cliproxy variants * GET /api/cliproxy - List cliproxy variants
* Uses variant-service for consistent behavior with CLI * Uses variant-service for consistent behavior with CLI