From 86856653f2e35e3675447e08e0dcbed45940b338 Mon Sep 17 00:00:00 2001 From: JeremyDev87 Date: Thu, 19 Mar 2026 18:46:35 +0900 Subject: [PATCH 1/2] fix(mcp): improve SseAuthGuard timing-safe token comparison - Replace Buffer length check with HMAC-SHA256 hashing before timingSafeEqual - Eliminates token length timing side-channel leak - Add tests for different-length token rejection and HMAC hash properties Closes #704 --- apps/mcp-server/src/mcp/sse-auth.guard.spec.ts | 18 ++++++++++++++++++ apps/mcp-server/src/mcp/sse-auth.guard.ts | 18 +++++++----------- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/apps/mcp-server/src/mcp/sse-auth.guard.spec.ts b/apps/mcp-server/src/mcp/sse-auth.guard.spec.ts index 4f7f068a..9d7b3809 100644 --- a/apps/mcp-server/src/mcp/sse-auth.guard.spec.ts +++ b/apps/mcp-server/src/mcp/sse-auth.guard.spec.ts @@ -92,5 +92,23 @@ describe('SseAuthGuard', () => { expect(() => guard.canActivate(context)).toThrow(UnauthorizedException); }); + + it('should reject tokens of different lengths without timing leak', () => { + const context = createMockContext('Bearer short'); + + expect(() => guard.canActivate(context)).toThrow(UnauthorizedException); + }); + + it('should use HMAC-based comparison (no length early-return)', () => { + // Verify the guard uses createHmac, not raw Buffer length comparison + const { createHmac } = require('crypto'); + const key = Buffer.from('codingbuddy-sse-auth'); + const expectedHash = createHmac('sha256', key).update('my-secret-token').digest(); + const providedHash = createHmac('sha256', key).update('wrong').digest(); + + // Both hashes should be same length (32 bytes) regardless of input length + expect(expectedHash.length).toBe(providedHash.length); + expect(expectedHash.length).toBe(32); + }); }); }); diff --git a/apps/mcp-server/src/mcp/sse-auth.guard.ts b/apps/mcp-server/src/mcp/sse-auth.guard.ts index bbc7f231..a249f558 100644 --- a/apps/mcp-server/src/mcp/sse-auth.guard.ts +++ b/apps/mcp-server/src/mcp/sse-auth.guard.ts @@ -6,7 +6,7 @@ import { UnauthorizedException, } from '@nestjs/common'; import { Request } from 'express'; -import { timingSafeEqual } from 'crypto'; +import { createHmac, timingSafeEqual } from 'crypto'; /** * Guard for SSE endpoints that validates Bearer token against MCP_SSE_TOKEN env var. @@ -53,17 +53,13 @@ export class SseAuthGuard implements CanActivate { } /** - * Timing-safe token comparison to prevent timing attacks. - * Handles tokens of different lengths safely. + * Timing-safe token comparison using HMAC to prevent timing attacks. + * HMAC produces fixed-length digests, eliminating length-based timing leaks. */ private tokensMatch(expected: string, provided: string): boolean { - const expectedBuf = Buffer.from(expected, 'utf-8'); - const providedBuf = Buffer.from(provided, 'utf-8'); - - if (expectedBuf.length !== providedBuf.length) { - return false; - } - - return timingSafeEqual(expectedBuf, providedBuf); + const key = Buffer.from('codingbuddy-sse-auth'); + const expectedHash = createHmac('sha256', key).update(expected).digest(); + const providedHash = createHmac('sha256', key).update(provided).digest(); + return timingSafeEqual(expectedHash, providedHash); } } From 8a7ef84324ac64308dfe20a793382d73ed5e0cc8 Mon Sep 17 00:00:00 2001 From: JeremyDev87 Date: Fri, 20 Mar 2026 16:58:45 +0900 Subject: [PATCH 2/2] fix(mcp): replace require() with ESM import in sse-auth guard test --- apps/mcp-server/src/mcp/sse-auth.guard.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/mcp-server/src/mcp/sse-auth.guard.spec.ts b/apps/mcp-server/src/mcp/sse-auth.guard.spec.ts index 9d7b3809..a4a202cf 100644 --- a/apps/mcp-server/src/mcp/sse-auth.guard.spec.ts +++ b/apps/mcp-server/src/mcp/sse-auth.guard.spec.ts @@ -1,4 +1,5 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { createHmac } from 'crypto'; import { ExecutionContext, UnauthorizedException } from '@nestjs/common'; import { SseAuthGuard } from './sse-auth.guard'; @@ -100,8 +101,7 @@ describe('SseAuthGuard', () => { }); it('should use HMAC-based comparison (no length early-return)', () => { - // Verify the guard uses createHmac, not raw Buffer length comparison - const { createHmac } = require('crypto'); + // Verify HMAC produces fixed-length digests regardless of input length const key = Buffer.from('codingbuddy-sse-auth'); const expectedHash = createHmac('sha256', key).update('my-secret-token').digest(); const providedHash = createHmac('sha256', key).update('wrong').digest();