refactor(backend): improve user configuration validation logic

Author: Lasim

services/backend/src/routes/mcp/user-configurations/create.ts

@@ -23,7 +23,6 @@ export default async function createUserConfigRoute(server: FastifyInstance) {
   }>('/teams/:teamId/mcp/installations/:installationId/user-configs', {
     preValidation: [
       requireAuthenticationAny(),
-      requireOAuthScope('mcp:read'),
       requireTeamPermission('mcp.installations.edit')
     ],
     schema: {

services/backend/src/routes/mcp/user-configurations/schemas.ts

@@ -390,13 +390,30 @@ export interface UpdateUserEnvRouteRequest {
 // Helper function to format user config response
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
 export function formatUserConfigResponse(config: any): UserConfigData {
+  // Helper to safely handle JSON parsing - config might already be parsed by service layer
+  const safeJsonParse = (value: any, defaultValue: any) => {
+    if (value === null || value === undefined || value === '') {
+      return defaultValue
+    }
+    // If it's already parsed (object/array), return as-is
+    if (typeof value !== 'string') {
+      return value
+    }
+    // If it's a string, try to parse it
+    try {
+      return JSON.parse(value)
+    } catch {
+      return defaultValue
+    }
+  }
+
   return {
     id: config.id,
     installation_id: config.installation_id,
     user_id: config.user_id,
     device_id: config.device_id || undefined,
-    user_args: config.user_args ? JSON.parse(config.user_args) : undefined,
-    user_env: config.user_env ? JSON.parse(config.user_env) : undefined,
+    user_args: safeJsonParse(config.user_args, undefined),
+    user_env: safeJsonParse(config.user_env, undefined),
     created_at: config.created_at.toISOString(),
     updated_at: config.updated_at.toISOString(),
     last_used_at: config.last_used_at ? config.last_used_at.toISOString() : undefined

services/backend/src/routes/mcp/user-configurations/update.ts

@@ -41,7 +41,11 @@ export default async function updateUserConfigurationRoute(fastify: FastifyInsta
       } catch (error) {
         fastify.log.error({
           operation: 'update_user_configuration',
-          error,
+          error: error instanceof Error ? {
+            name: error.name,
+            message: error.message,
+            stack: error.stack
+          } : { errorType: typeof error, errorValue: error },
           teamId,
           installationId,
           configId,
@@ -66,7 +70,7 @@ export default async function updateUserConfigurationRoute(fastify: FastifyInsta
         }
 
         return reply.status(500).send({
-          error: 'Internal server error'
+          error: error instanceof Error ? error.message : 'Internal server error'
         });
       }
     }

services/backend/src/services/mcpUserConfigurationService.ts

@@ -1,5 +1,5 @@
 /* eslint-disable @typescript-eslint/no-explicit-any */
-import { eq, and, desc } from 'drizzle-orm';
+import { eq, and, desc, ne } from 'drizzle-orm';
 import { mcpUserConfigurations, mcpServerInstallations, mcpServers } from '../db/schema.sqlite';
 import type { AnyDatabase } from '../db';
 import type { FastifyBaseLogger } from 'fastify';
@@ -321,7 +321,7 @@ export class McpUserConfigurationService {
             eq(mcpUserConfigurations.user_id, userId),
             eq(mcpUserConfigurations.device_id, data.device_id),
             // Exclude current configuration
-            eq(mcpUserConfigurations.id, configId)
+            ne(mcpUserConfigurations.id, configId)
           )
         )
         .limit(1);
@@ -452,12 +452,20 @@ export class McpUserConfigurationService {
 
   private validateUserEnv(userEnv: Record<string, string>, schema: any[]): void {
     // Validate user environment variables against schema
-    const requiredVars = schema.filter((envVar: any) => envVar.required);
-    
-    for (const requiredVar of requiredVars) {
-      if (!userEnv[requiredVar.name] || userEnv[requiredVar.name].trim() === '') {
-        throw new Error(`Required environment variable '${requiredVar.name}' is missing or empty`);
+    // Only validate the fields that are actually being sent, not all required fields
+    for (const [envVarName, envVarValue] of Object.entries(userEnv)) {
+      const schemaEntry = schema.find((envVar: any) => envVar.name === envVarName)
+      
+      if (schemaEntry) {
+        // If this field is required and the sent value is empty, throw error
+        if (schemaEntry.required && (!envVarValue || envVarValue.trim() === '')) {
+          throw new Error(`Required environment variable '${envVarName}' is missing or empty`)
+        }
+        
+        // Additional type validation can be added here in the future
+        // e.g., if (schemaEntry.type === 'number' && isNaN(Number(envVarValue)))
       }
+      // Note: We don't validate fields that aren't in the schema - allowing flexibility
     }
   }