revert: restore rate-limiting test to original beforeAll approach

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 <noreply@anthropic.com>
This commit is contained in:
czlonkowski
2025-10-06 16:49:30 +02:00
parent d207cc3723
commit 0ec02fa0da

View File

@@ -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<ChildProcess> => {
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++) {