diff --git a/CHANGELOG.md b/CHANGELOG.md index f76edd8..1e4bfb8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,45 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [2.17.3] - 2025-10-07 + +### 🔧 Validation + +**Fixed critical validation gap for AI model nodes with resourceLocator properties.** + +This release adds validation for `resourceLocator` type properties, fixing a critical issue where AI agents could create invalid configurations that passed validation but failed at runtime. + +#### Fixed + +- **resourceLocator Property Validation** + - **Issue:** No validation existed for `resourceLocator` type properties used in AI model nodes + - **Impact:** + - AI agents could create invalid configurations like `model: "gpt-4o-mini"` (string) instead of `model: {mode: "list", value: "gpt-4o-mini"}` (object) + - Invalid configs passed validation but failed at runtime in n8n + - Affected many langchain nodes: OpenAI Chat Model (v1.2+), Anthropic, Cohere, DeepSeek, Groq, Mistral, OpenRouter, xAI Grok, and embeddings nodes + - **Root Cause:** `validatePropertyTypes()` method in ConfigValidator only validated `string`, `number`, `boolean`, and `options` types - `resourceLocator` was completely missing + - **Fix:** Added comprehensive resourceLocator validation in `config-validator.ts:237-274` + - Validates value is an object (not string, number, null, or array) + - Validates required `mode` property exists and is a string + - Validates required `value` property exists + - Provides helpful error messages with exact fix suggestions + - Example error: `Property 'model' is a resourceLocator and must be an object with 'mode' and 'value' properties, got string` + - Example fix: `Change model to { mode: "list", value: "gpt-4o-mini" } or { mode: "id", value: "gpt-4o-mini" }` + +#### Added + +- Comprehensive resourceLocator validation with 14 test cases covering: + - String value rejection with helpful fix suggestions + - Null and array value rejection + - Missing `mode` or `value` property detection + - Invalid `mode` type detection (e.g., number instead of string) + - Invalid `mode` value validation (must be 'list', 'id', or 'url') + - Empty object detection (missing both mode and value) + - Extra properties handling (ignored gracefully) + - Valid resourceLocator acceptance for "list", "id", and "url" modes + - JSDoc documentation explaining resourceLocator structure and common mistakes + - All 29 tests passing (100% coverage for new validation logic) + ## [2.17.1] - 2025-10-07 ### 🔧 Telemetry diff --git a/data/nodes.db b/data/nodes.db index 8c0f831..0152e62 100644 Binary files a/data/nodes.db and b/data/nodes.db differ diff --git a/package.json b/package.json index bf7595d..254ec6a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "n8n-mcp", - "version": "2.17.2", + "version": "2.17.3", "description": "Integration between n8n workflow automation and Model Context Protocol (MCP)", "main": "dist/index.js", "bin": { diff --git a/package.runtime.json b/package.runtime.json index 0cc150c..0a7a85f 100644 --- a/package.runtime.json +++ b/package.runtime.json @@ -1,6 +1,6 @@ { "name": "n8n-mcp-runtime", - "version": "2.17.1", + "version": "2.17.3", "description": "n8n MCP Server Runtime Dependencies Only", "private": true, "dependencies": { diff --git a/src/services/config-validator.ts b/src/services/config-validator.ts index cea5ebb..2028aea 100644 --- a/src/services/config-validator.ts +++ b/src/services/config-validator.ts @@ -234,8 +234,56 @@ export class ConfigValidator { message: `Property '${key}' must be a boolean, got ${typeof value}`, fix: `Change ${key} to true or false` }); + } else if (prop.type === 'resourceLocator') { + // resourceLocator validation: Used by AI model nodes (OpenAI, Anthropic, etc.) + // Must be an object with required properties: + // - mode: string ('list' | 'id' | 'url') + // - value: any (the actual model/resource identifier) + // Common mistake: passing string directly instead of object structure + if (typeof value !== 'object' || value === null || Array.isArray(value)) { + const fixValue = typeof value === 'string' ? value : JSON.stringify(value); + errors.push({ + type: 'invalid_type', + property: key, + message: `Property '${key}' is a resourceLocator and must be an object with 'mode' and 'value' properties, got ${typeof value}`, + fix: `Change ${key} to { mode: "list", value: ${JSON.stringify(fixValue)} } or { mode: "id", value: ${JSON.stringify(fixValue)} }` + }); + } else { + // Check required properties + if (!value.mode) { + errors.push({ + type: 'missing_required', + property: `${key}.mode`, + message: `resourceLocator '${key}' is missing required property 'mode'`, + fix: `Add mode property: { mode: "list", value: ${JSON.stringify(value.value || '')} }` + }); + } else if (typeof value.mode !== 'string') { + errors.push({ + type: 'invalid_type', + property: `${key}.mode`, + message: `resourceLocator '${key}.mode' must be a string, got ${typeof value.mode}`, + fix: `Set mode to "list" or "id"` + }); + } else if (!['list', 'id', 'url'].includes(value.mode)) { + errors.push({ + type: 'invalid_value', + property: `${key}.mode`, + message: `resourceLocator '${key}.mode' must be 'list', 'id', or 'url', got '${value.mode}'`, + fix: `Change mode to "list", "id", or "url"` + }); + } + + if (value.value === undefined) { + errors.push({ + type: 'missing_required', + property: `${key}.value`, + message: `resourceLocator '${key}' is missing required property 'value'`, + fix: `Add value property to specify the ${prop.displayName || key}` + }); + } + } } - + // Options validation if (prop.type === 'options' && prop.options) { const validValues = prop.options.map((opt: any) => diff --git a/tests/unit/services/config-validator-basic.test.ts b/tests/unit/services/config-validator-basic.test.ts index dc97072..1da62f0 100644 --- a/tests/unit/services/config-validator-basic.test.ts +++ b/tests/unit/services/config-validator-basic.test.ts @@ -439,4 +439,335 @@ describe('ConfigValidator - Basic Validation', () => { expect(result.suggestions.length).toBeGreaterThanOrEqual(0); }); }); + + describe('resourceLocator validation', () => { + it('should reject string value when resourceLocator object is required', () => { + const nodeType = '@n8n/n8n-nodes-langchain.lmChatOpenAi'; + const config = { + model: 'gpt-4o-mini' // Wrong - should be object with mode and value + }; + const properties = [ + { + name: 'model', + displayName: 'Model', + type: 'resourceLocator', + required: true, + default: { mode: 'list', value: 'gpt-4o-mini' } + } + ]; + + const result = ConfigValidator.validate(nodeType, config, properties); + + expect(result.valid).toBe(false); + expect(result.errors).toHaveLength(1); + expect(result.errors[0]).toMatchObject({ + type: 'invalid_type', + property: 'model', + message: expect.stringContaining('must be an object with \'mode\' and \'value\' properties') + }); + expect(result.errors[0].fix).toContain('mode'); + expect(result.errors[0].fix).toContain('value'); + }); + + it('should accept valid resourceLocator with mode and value', () => { + const nodeType = '@n8n/n8n-nodes-langchain.lmChatOpenAi'; + const config = { + model: { + mode: 'list', + value: 'gpt-4o-mini' + } + }; + const properties = [ + { + name: 'model', + displayName: 'Model', + type: 'resourceLocator', + required: true, + default: { mode: 'list', value: 'gpt-4o-mini' } + } + ]; + + const result = ConfigValidator.validate(nodeType, config, properties); + + expect(result.valid).toBe(true); + expect(result.errors).toHaveLength(0); + }); + + it('should reject null value for resourceLocator', () => { + const nodeType = '@n8n/n8n-nodes-langchain.lmChatOpenAi'; + const config = { + model: null + }; + const properties = [ + { + name: 'model', + type: 'resourceLocator', + required: true + } + ]; + + const result = ConfigValidator.validate(nodeType, config, properties); + + expect(result.valid).toBe(false); + expect(result.errors.some(e => + e.property === 'model' && + e.type === 'invalid_type' + )).toBe(true); + }); + + it('should reject array value for resourceLocator', () => { + const nodeType = '@n8n/n8n-nodes-langchain.lmChatOpenAi'; + const config = { + model: ['gpt-4o-mini'] + }; + const properties = [ + { + name: 'model', + type: 'resourceLocator', + required: true + } + ]; + + const result = ConfigValidator.validate(nodeType, config, properties); + + expect(result.valid).toBe(false); + expect(result.errors.some(e => + e.property === 'model' && + e.type === 'invalid_type' && + e.message.includes('must be an object') + )).toBe(true); + }); + + it('should detect missing mode property in resourceLocator', () => { + const nodeType = '@n8n/n8n-nodes-langchain.lmChatOpenAi'; + const config = { + model: { + value: 'gpt-4o-mini' + // Missing mode property + } + }; + const properties = [ + { + name: 'model', + type: 'resourceLocator', + required: true + } + ]; + + const result = ConfigValidator.validate(nodeType, config, properties); + + expect(result.valid).toBe(false); + expect(result.errors.some(e => + e.property === 'model.mode' && + e.type === 'missing_required' && + e.message.includes('missing required property \'mode\'') + )).toBe(true); + }); + + it('should detect missing value property in resourceLocator', () => { + const nodeType = '@n8n/n8n-nodes-langchain.lmChatOpenAi'; + const config = { + model: { + mode: 'list' + // Missing value property + } + }; + const properties = [ + { + name: 'model', + displayName: 'Model', + type: 'resourceLocator', + required: true + } + ]; + + const result = ConfigValidator.validate(nodeType, config, properties); + + expect(result.valid).toBe(false); + expect(result.errors.some(e => + e.property === 'model.value' && + e.type === 'missing_required' && + e.message.includes('missing required property \'value\'') + )).toBe(true); + }); + + it('should detect invalid mode type in resourceLocator', () => { + const nodeType = '@n8n/n8n-nodes-langchain.lmChatOpenAi'; + const config = { + model: { + mode: 123, // Should be string + value: 'gpt-4o-mini' + } + }; + const properties = [ + { + name: 'model', + type: 'resourceLocator', + required: true + } + ]; + + const result = ConfigValidator.validate(nodeType, config, properties); + + expect(result.valid).toBe(false); + expect(result.errors.some(e => + e.property === 'model.mode' && + e.type === 'invalid_type' && + e.message.includes('must be a string') + )).toBe(true); + }); + + it('should accept resourceLocator with mode "id"', () => { + const nodeType = '@n8n/n8n-nodes-langchain.lmChatOpenAi'; + const config = { + model: { + mode: 'id', + value: 'gpt-4o-2024-11-20' + } + }; + const properties = [ + { + name: 'model', + type: 'resourceLocator', + required: true + } + ]; + + const result = ConfigValidator.validate(nodeType, config, properties); + + expect(result.valid).toBe(true); + expect(result.errors).toHaveLength(0); + }); + + it('should reject number value when resourceLocator is required', () => { + const nodeType = '@n8n/n8n-nodes-langchain.lmChatOpenAi'; + const config = { + model: 12345 // Wrong type + }; + const properties = [ + { + name: 'model', + type: 'resourceLocator', + required: true + } + ]; + + const result = ConfigValidator.validate(nodeType, config, properties); + + expect(result.valid).toBe(false); + expect(result.errors[0].type).toBe('invalid_type'); + expect(result.errors[0].message).toContain('must be an object'); + }); + + it('should provide helpful fix suggestion for string to resourceLocator conversion', () => { + const nodeType = '@n8n/n8n-nodes-langchain.lmChatOpenAi'; + const config = { + model: 'gpt-4o-mini' + }; + const properties = [ + { + name: 'model', + type: 'resourceLocator', + required: true + } + ]; + + const result = ConfigValidator.validate(nodeType, config, properties); + + expect(result.errors[0].fix).toContain('{ mode: "list", value: "gpt-4o-mini" }'); + expect(result.errors[0].fix).toContain('{ mode: "id", value: "gpt-4o-mini" }'); + }); + + it('should reject invalid mode values', () => { + const nodeType = '@n8n/n8n-nodes-langchain.lmChatOpenAi'; + const config = { + model: { + mode: 'invalid-mode', + value: 'gpt-4o-mini' + } + }; + const properties = [ + { + name: 'model', + type: 'resourceLocator', + required: true + } + ]; + + const result = ConfigValidator.validate(nodeType, config, properties); + + expect(result.valid).toBe(false); + expect(result.errors.some(e => + e.property === 'model.mode' && + e.type === 'invalid_value' && + e.message.includes("must be 'list', 'id', or 'url'") + )).toBe(true); + }); + + it('should accept resourceLocator with mode "url"', () => { + const nodeType = '@n8n/n8n-nodes-langchain.lmChatOpenAi'; + const config = { + model: { + mode: 'url', + value: 'https://api.example.com/models/custom' + } + }; + const properties = [ + { + name: 'model', + type: 'resourceLocator', + required: true + } + ]; + + const result = ConfigValidator.validate(nodeType, config, properties); + + expect(result.valid).toBe(true); + expect(result.errors).toHaveLength(0); + }); + + it('should detect empty resourceLocator object', () => { + const nodeType = '@n8n/n8n-nodes-langchain.lmChatOpenAi'; + const config = { + model: {} // Empty object, missing both mode and value + }; + const properties = [ + { + name: 'model', + type: 'resourceLocator', + required: true + } + ]; + + const result = ConfigValidator.validate(nodeType, config, properties); + + expect(result.valid).toBe(false); + expect(result.errors.length).toBeGreaterThanOrEqual(2); // Both mode and value missing + expect(result.errors.some(e => e.property === 'model.mode')).toBe(true); + expect(result.errors.some(e => e.property === 'model.value')).toBe(true); + }); + + it('should handle resourceLocator with extra properties gracefully', () => { + const nodeType = '@n8n/n8n-nodes-langchain.lmChatOpenAi'; + const config = { + model: { + mode: 'list', + value: 'gpt-4o-mini', + extraProperty: 'ignored' // Extra properties should be ignored + } + }; + const properties = [ + { + name: 'model', + type: 'resourceLocator', + required: true + } + ]; + + const result = ConfigValidator.validate(nodeType, config, properties); + + expect(result.valid).toBe(true); // Should pass with extra properties + expect(result.errors).toHaveLength(0); + }); + }); }); \ No newline at end of file