From 4c002caee1d64b2b90967a67a18db8f097bbac80 Mon Sep 17 00:00:00 2001
From: Tam Nhu Tran <kaitran.ntt@gmail.com>
Date: Thu, 12 Feb 2026 08:46:38 +0700
Subject: [PATCH] fix(cliproxy): address code review feedback (attempt 4/5)

- Add default_tier and provider validation in POST/PUT routes
- Extract shared validateCompositeTiers() helper for DRY
- Validate providers against CLIPROXY_SUPPORTED_PROVIDERS
---
 src/web-server/routes/variant-routes.ts | 72 ++++++++++++++++---------
 1 file changed, 46 insertions(+), 26 deletions(-)

diff --git a/src/web-server/routes/variant-routes.ts b/src/web-server/routes/variant-routes.ts
index 7284cd40..fb86f4ce 100644
--- a/src/web-server/routes/variant-routes.ts
+++ b/src/web-server/routes/variant-routes.ts
@@ -7,6 +7,7 @@
 import { Router, Request, Response } from 'express';
 import { isReservedName, RESERVED_PROFILE_NAMES } from '../../config/reserved-names';
 import type { CLIProxyProvider } from '../../cliproxy/types';
+import { CLIPROXY_SUPPORTED_PROVIDERS } from '../../config/unified-config-types';
 import {
   createVariant,
   removeVariant,
@@ -19,6 +20,33 @@ import {
 
 const router = Router();
 
+const VALID_TIERS = ['opus', 'sonnet', 'haiku'] as const;
+
+/** Validate composite tiers shape and provider/default_tier values. Returns error string or null. */
+function validateCompositeTiers(
+  tiers: Record<string, { provider?: string; model?: string }>,
+  defaultTier?: string
+): string | null {
+  // Validate default_tier
+  if (defaultTier && !VALID_TIERS.includes(defaultTier as (typeof VALID_TIERS)[number])) {
+    return `Invalid default_tier '${defaultTier}': must be one of ${VALID_TIERS.join(', ')}`;
+  }
+  // Validate each tier
+  for (const tier of VALID_TIERS) {
+    if (
+      !tiers[tier] ||
+      typeof tiers[tier].provider !== 'string' ||
+      typeof tiers[tier].model !== 'string'
+    ) {
+      return `Invalid tier config for '${tier}': requires 'provider' and 'model' strings`;
+    }
+    if (!CLIPROXY_SUPPORTED_PROVIDERS.includes(tiers[tier].provider as CLIProxyProvider)) {
+      return `Invalid provider '${tiers[tier].provider}' for tier '${tier}': must be one of ${CLIPROXY_SUPPORTED_PROVIDERS.join(', ')}`;
+    }
+  }
+  return null;
+}
+
 /**
  * GET /api/cliproxy - List cliproxy variants
  * Uses variant-service for consistent behavior with CLI
@@ -75,19 +103,11 @@ router.post('/', (req: Request, res: Response): void => {
       return;
     }
 
-    // Validate tiers shape
-    const requiredTiers = ['opus', 'sonnet', 'haiku'] as const;
-    for (const tier of requiredTiers) {
-      if (
-        !tiers[tier] ||
-        typeof tiers[tier].provider !== 'string' ||
-        typeof tiers[tier].model !== 'string'
-      ) {
-        res.status(400).json({
-          error: `Invalid tier config for '${tier}': requires 'provider' and 'model' strings`,
-        });
-        return;
-      }
+    // Validate tiers shape, providers, and default_tier
+    const tierError = validateCompositeTiers(tiers, default_tier);
+    if (tierError) {
+      res.status(400).json({ error: tierError });
+      return;
     }
 
     const result = createCompositeVariant({ name, defaultTier: default_tier, tiers });
@@ -162,21 +182,21 @@ router.put('/:name', (req: Request, res: Response): void => {
         return;
       }
 
-      // Validate tiers shape if provided
+      // Validate tiers shape, providers, and default_tier if provided
       if (tiers) {
-        const requiredTiers = ['opus', 'sonnet', 'haiku'] as const;
-        for (const tier of requiredTiers) {
-          if (
-            !tiers[tier] ||
-            typeof tiers[tier].provider !== 'string' ||
-            typeof tiers[tier].model !== 'string'
-          ) {
-            res.status(400).json({
-              error: `Invalid tier config for '${tier}': requires 'provider' and 'model' strings`,
-            });
-            return;
-          }
+        const tierError = validateCompositeTiers(tiers, default_tier);
+        if (tierError) {
+          res.status(400).json({ error: tierError });
+          return;
         }
+      } else if (
+        default_tier &&
+        !VALID_TIERS.includes(default_tier as (typeof VALID_TIERS)[number])
+      ) {
+        res.status(400).json({
+          error: `Invalid default_tier '${default_tier}': must be one of ${VALID_TIERS.join(', ')}`,
+        });
+        return;
       }
 
       const result = updateCompositeVariant(name, { defaultTier: default_tier, tiers });