From 6fa94d3eb8f5779363226d10dccf8b01a735744c Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Mon, 1 Dec 2025 17:41:39 -0500 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=90=20fix:=20Secure=20`iconURL`=20Hand?= =?UTF-8?q?ling=20(#10753)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 🔒 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 | 12 +- eslint.config.mjs | 10 ++ packages/data-provider/specs/parsers.spec.ts | 140 ++++++++++++++++++- packages/data-provider/src/parsers.ts | 6 +- 4 files changed, 162 insertions(+), 6 deletions(-) diff --git a/api/server/middleware/buildEndpointOption.js b/api/server/middleware/buildEndpointOption.js index 6f554d95d0..7b19f7a3d4 100644 --- a/api/server/middleware/buildEndpointOption.js +++ b/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 { diff --git a/eslint.config.mjs b/eslint.config.mjs index f84a57058e..f9378d0c57 100644 --- a/eslint.config.mjs +++ b/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'], diff --git a/packages/data-provider/specs/parsers.spec.ts b/packages/data-provider/specs/parsers.spec.ts index 8bcbfac7ec..f506d16c6c 100644 --- a/packages/data-provider/specs/parsers.spec.ts +++ b/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 = { + 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 = { + 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 = { + 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 = { + 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 = { + 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 = { + 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 = { + 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'); + }); + }); +}); diff --git a/packages/data-provider/src/parsers.ts b/packages/data-provider/src/parsers.ts index 61616a57a8..be8d6dcde0 100644 --- a/packages/data-provider/src/parsers.ts +++ b/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 ?? {};