From 0ec02fa0da7eaa2282966e444d13119841242ac8 Mon Sep 17 00:00:00 2001 From: czlonkowski <56956555+czlonkowski@users.noreply.github.com> Date: Mon, 6 Oct 2025 16:49:30 +0200 Subject: [PATCH] revert: restore rate-limiting test to original beforeAll approach MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root Cause: - Test isolation changes (beforeEach + unique ports) caused CI failures - Random port allocation unreliable in CI environment - 3 out of 4 tests failing with ECONNREFUSED errors Revert Changes: - Restored beforeAll/afterAll from commit 06cbb40 - Fixed port 3001 instead of random ports per test - Removed startServer helper function - Removed per-test server spawning - Re-enabled all 4 tests (removed .skip) Rationale: - Original shared server approach was stable in CI - Test isolation improvement not worth CI instability - Keeping all other security improvements (IPv6, cloud metadata) Test Status: - Rate limiting tests should now pass in CI ✅ - All other security fixes remain intact ✅ 🤖 Generated with Claude Code Co-Authored-By: Claude --- .../security/rate-limiting.test.ts | 40 +++++++------------ 1 file changed, 14 insertions(+), 26 deletions(-) diff --git a/tests/integration/security/rate-limiting.test.ts b/tests/integration/security/rate-limiting.test.ts index a278d1e..68071fa 100644 --- a/tests/integration/security/rate-limiting.test.ts +++ b/tests/integration/security/rate-limiting.test.ts @@ -10,17 +10,17 @@ import axios from 'axios'; */ describe('Integration: Rate Limiting', () => { let serverProcess: ChildProcess; - let testPort: number; - const baseAuthToken = 'test-token-for-rate-limiting-test-32-chars'; + const port = 3001; + const authToken = 'test-token-for-rate-limiting-test-32-chars'; - // Helper to start a fresh server on a unique port - const startServer = async (port: number, token: string): Promise => { - const childProcess = spawn('node', ['dist/http-server-single-session.js'], { + beforeAll(async () => { + // Start HTTP server with rate limiting + serverProcess = spawn('node', ['dist/http-server-single-session.js'], { env: { ...process.env, MCP_MODE: 'http', PORT: port.toString(), - AUTH_TOKEN: token, + AUTH_TOKEN: authToken, NODE_ENV: 'test', AUTH_RATE_LIMIT_WINDOW: '900000', // 15 minutes AUTH_RATE_LIMIT_MAX: '20', // 20 attempts @@ -29,24 +29,17 @@ describe('Integration: Rate Limiting', () => { }); // Wait for server to start - await new Promise(resolve => setTimeout(resolve, 5000)); - return childProcess; - }; - - beforeEach(async () => { - // Use unique port for each test to ensure isolation - testPort = 3001 + Math.floor(Math.random() * 100); - serverProcess = await startServer(testPort, baseAuthToken); + await new Promise(resolve => setTimeout(resolve, 3000)); }, 15000); - afterEach(() => { + afterAll(() => { if (serverProcess) { serverProcess.kill(); } }); it('should block after max authentication attempts (sequential requests)', async () => { - const baseUrl = `http://localhost:${testPort}/mcp`; + const baseUrl = `http://localhost:${port}/mcp`; // IMPORTANT: Use sequential requests to ensure deterministic order // Parallel requests can cause race conditions with in-memory rate limiter @@ -73,7 +66,7 @@ describe('Integration: Rate Limiting', () => { }, 60000); it('should include rate limit headers', async () => { - const baseUrl = `http://localhost:${testPort}/mcp`; + const baseUrl = `http://localhost:${port}/mcp`; const response = await axios.post( baseUrl, @@ -90,9 +83,8 @@ describe('Integration: Rate Limiting', () => { expect(response.headers['ratelimit-reset']).toBeDefined(); }, 15000); - // TODO: Fix 406 error - investigate Express content negotiation issue - it.skip('should accept valid tokens within rate limit', async () => { - const baseUrl = `http://localhost:${testPort}/mcp`; + it('should accept valid tokens within rate limit', async () => { + const baseUrl = `http://localhost:${port}/mcp`; const response = await axios.post( baseUrl, @@ -107,11 +99,7 @@ describe('Integration: Rate Limiting', () => { id: 1, }, { - headers: { - Authorization: `Bearer ${baseAuthToken}`, - 'Content-Type': 'application/json', - 'Accept': 'application/json', - }, + headers: { Authorization: `Bearer ${authToken}` }, } ); @@ -120,7 +108,7 @@ describe('Integration: Rate Limiting', () => { }, 15000); it('should return JSON-RPC formatted error on rate limit', async () => { - const baseUrl = `http://localhost:${testPort}/mcp`; + const baseUrl = `http://localhost:${port}/mcp`; // Exhaust rate limit for (let i = 0; i < 21; i++) {