🔐 fix: Secure iconURL Handling (#10753)

* 🔒 fix: `iconURL` in conversation parsing

- Updated the `buildEndpointOption` middleware to derive `iconURL` from model specs when not provided by the client, improving security by preventing malicious URLs.
- Modified the `parseCompactConvo` function to strip `iconURL` from conversation inputs, ensuring it is only set server-side.
- Added comprehensive tests to validate the stripping of `iconURL` across various endpoint types, enhancing overall input sanitization.

* ✨ feat: Add ESLint rule for unused variables

- Introduced a new ESLint rule to warn about unused variables, allowing for better code quality and maintainability.
- Configured the rule to ignore variables and arguments that start with an underscore, accommodating common coding practices.

api/server/middleware/buildEndpointOption.js

@@ -61,18 +61,24 @@ async function buildEndpointOption(req, res, next) {
 
     try {
       currentModelSpec.preset.spec = spec;
-      if (currentModelSpec.iconURL != null && currentModelSpec.iconURL !== '') {
-        currentModelSpec.preset.iconURL = currentModelSpec.iconURL;
-      }
       parsedBody = parseCompactConvo({
         endpoint,
         endpointType,
         conversation: currentModelSpec.preset,
       });
+      if (currentModelSpec.iconURL != null && currentModelSpec.iconURL !== '') {
+        parsedBody.iconURL = currentModelSpec.iconURL;
+      }
     } catch (error) {
       logger.error(`Error parsing model spec for endpoint ${endpoint}`, error);
       return handleError(res, { text: 'Error parsing model spec' });
     }
+  } else if (parsedBody.spec && appConfig.modelSpecs?.list) {
+    // Non-enforced mode: if spec is selected, derive iconURL from model spec
+    const modelSpec = appConfig.modelSpecs.list.find((s) => s.name === parsedBody.spec);
+    if (modelSpec?.iconURL) {
+      parsedBody.iconURL = modelSpec.iconURL;
+    }
   }
 
   try {

eslint.config.mjs

@@ -269,6 +269,16 @@ export default [
         project: './packages/data-provider/tsconfig.json',
       },
     },
+    rules: {
+      '@typescript-eslint/no-unused-vars': [
+        'warn',
+        {
+          argsIgnorePattern: '^_',
+          varsIgnorePattern: '^_',
+          caughtErrorsIgnorePattern: '^_',
+        },
+      ],
+    },
   },
   {
     files: ['./api/demo/**/*.ts'],

packages/data-provider/specs/parsers.spec.ts

@@ -1,6 +1,7 @@
-import { replaceSpecialVars } from '../src/parsers';
+import { replaceSpecialVars, parseCompactConvo } from '../src/parsers';
 import { specialVariables } from '../src/config';
-import type { TUser } from '../src/types';
+import { EModelEndpoint } from '../src/schemas';
+import type { TUser, TConversation } from '../src/types';
 
 // Mock dayjs module with consistent date/time values regardless of environment
 jest.mock('dayjs', () => {
@@ -123,3 +124,138 @@ describe('replaceSpecialVars', () => {
     expect(result).toContain('Test User'); // current_user
   });
 });
+
+describe('parseCompactConvo', () => {
+  describe('iconURL security sanitization', () => {
+    test('should strip iconURL from OpenAI endpoint conversation input', () => {
+      const maliciousIconURL = 'https://evil-tracker.example.com/pixel.png?user=victim';
+      const conversation: Partial<TConversation> = {
+        model: 'gpt-4',
+        iconURL: maliciousIconURL,
+        endpoint: EModelEndpoint.openAI,
+      };
+
+      const result = parseCompactConvo({
+        endpoint: EModelEndpoint.openAI,
+        conversation,
+      });
+
+      expect(result).not.toBeNull();
+      expect(result?.iconURL).toBeUndefined();
+      expect(result?.model).toBe('gpt-4');
+    });
+
+    test('should strip iconURL from agents endpoint conversation input', () => {
+      const maliciousIconURL = 'https://evil-tracker.example.com/pixel.png';
+      const conversation: Partial<TConversation> = {
+        agent_id: 'agent_123',
+        iconURL: maliciousIconURL,
+        endpoint: EModelEndpoint.agents,
+      };
+
+      const result = parseCompactConvo({
+        endpoint: EModelEndpoint.agents,
+        conversation,
+      });
+
+      expect(result).not.toBeNull();
+      expect(result?.iconURL).toBeUndefined();
+      expect(result?.agent_id).toBe('agent_123');
+    });
+
+    test('should strip iconURL from anthropic endpoint conversation input', () => {
+      const maliciousIconURL = 'https://tracker.malicious.com/beacon.gif';
+      const conversation: Partial<TConversation> = {
+        model: 'claude-3-opus',
+        iconURL: maliciousIconURL,
+        endpoint: EModelEndpoint.anthropic,
+      };
+
+      const result = parseCompactConvo({
+        endpoint: EModelEndpoint.anthropic,
+        conversation,
+      });
+
+      expect(result).not.toBeNull();
+      expect(result?.iconURL).toBeUndefined();
+      expect(result?.model).toBe('claude-3-opus');
+    });
+
+    test('should strip iconURL from google endpoint conversation input', () => {
+      const maliciousIconURL = 'https://tracking.example.com/spy.png';
+      const conversation: Partial<TConversation> = {
+        model: 'gemini-pro',
+        iconURL: maliciousIconURL,
+        endpoint: EModelEndpoint.google,
+      };
+
+      const result = parseCompactConvo({
+        endpoint: EModelEndpoint.google,
+        conversation,
+      });
+
+      expect(result).not.toBeNull();
+      expect(result?.iconURL).toBeUndefined();
+      expect(result?.model).toBe('gemini-pro');
+    });
+
+    test('should strip iconURL from assistants endpoint conversation input', () => {
+      const maliciousIconURL = 'https://evil.com/track.png';
+      const conversation: Partial<TConversation> = {
+        assistant_id: 'asst_123',
+        iconURL: maliciousIconURL,
+        endpoint: EModelEndpoint.assistants,
+      };
+
+      const result = parseCompactConvo({
+        endpoint: EModelEndpoint.assistants,
+        conversation,
+      });
+
+      expect(result).not.toBeNull();
+      expect(result?.iconURL).toBeUndefined();
+      expect(result?.assistant_id).toBe('asst_123');
+    });
+
+    test('should preserve other conversation properties while stripping iconURL', () => {
+      const conversation: Partial<TConversation> = {
+        model: 'gpt-4',
+        iconURL: 'https://malicious.com/track.png',
+        endpoint: EModelEndpoint.openAI,
+        temperature: 0.7,
+        top_p: 0.9,
+        promptPrefix: 'You are a helpful assistant.',
+        maxContextTokens: 4000,
+      };
+
+      const result = parseCompactConvo({
+        endpoint: EModelEndpoint.openAI,
+        conversation,
+      });
+
+      expect(result).not.toBeNull();
+      expect(result?.iconURL).toBeUndefined();
+      expect(result?.model).toBe('gpt-4');
+      expect(result?.temperature).toBe(0.7);
+      expect(result?.top_p).toBe(0.9);
+      expect(result?.promptPrefix).toBe('You are a helpful assistant.');
+      expect(result?.maxContextTokens).toBe(4000);
+    });
+
+    test('should handle conversation without iconURL (no error)', () => {
+      const conversation: Partial<TConversation> = {
+        model: 'gpt-4',
+        endpoint: EModelEndpoint.openAI,
+      };
+
+      const result = parseCompactConvo({
+        endpoint: EModelEndpoint.openAI,
+        conversation,
+      });
+
+      expect(result).not.toBeNull();
+      expect(result?.iconURL).toBeUndefined();
+      expect(result?.model).toBe('gpt-4');
+    });
+  });
+});

packages/data-provider/src/parsers.ts

@@ -343,7 +343,11 @@ export const parseCompactConvo = ({
     throw new Error(`Unknown endpointType: ${endpointType}`);
   }
 
-  const convo = schema.parse(conversation) as s.TConversation | null;
+  // Strip iconURL from input before parsing - it should only be derived server-side
+  // from model spec configuration, not accepted from client requests
+  const { iconURL: _clientIconURL, ...conversationWithoutIconURL } = conversation;
+
+  const convo = schema.parse(conversationWithoutIconURL) as s.TConversation | null;
   // const { models, secondaryModels } = possibleValues ?? {};
   const { models } = possibleValues ?? {};