From e13ace58e53418f168a8474735f15005aee956f1 Mon Sep 17 00:00:00 2001 From: kurihada Date: Sun, 1 Mar 2026 00:29:35 +0800 Subject: [PATCH] =?UTF-8?q?fix:=20=E7=A7=BB=E9=99=A4=20DNS=20rebinding=20?= =?UTF-8?q?=E9=98=B2=E5=BE=A1=EF=BC=88Bearer=20token=20=E8=AE=A4=E8=AF=81?= =?UTF-8?q?=E5=B7=B2=E8=B6=B3=E5=A4=9F=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/server/app.ts | 2 - src/server/middleware.ts | 35 +-------- test/middleware.test.ts | 153 --------------------------------------- 3 files changed, 1 insertion(+), 189 deletions(-) delete mode 100644 test/middleware.test.ts diff --git a/src/server/app.ts b/src/server/app.ts index b68b283..4d6eb23 100644 --- a/src/server/app.ts +++ b/src/server/app.ts @@ -8,7 +8,6 @@ import { config } from '../config/index.js'; import { BrowserManager, browserManager } from '../browser/manager.js'; import { logger } from '../utils/logger.js'; import { - dnsRebindingGuard, shutdownGuard, errorHandler, bearerAuth, @@ -88,7 +87,6 @@ export class AppServer { this.app.use(express.json()); // 2. Security & availability middleware - this.app.use(dnsRebindingGuard); this.app.use(shutdownGuard(() => this.shuttingDown)); // 3. MCP server diff --git a/src/server/middleware.ts b/src/server/middleware.ts index 3bb2e1d..16b17aa 100644 --- a/src/server/middleware.ts +++ b/src/server/middleware.ts @@ -12,40 +12,7 @@ import { sanitizeErrorMessage } from '../utils/errors.js'; // Allowed hosts for DNS rebinding protection // --------------------------------------------------------------------------- -const allowedHosts = new Set([ - '127.0.0.1', - 'localhost', - `127.0.0.1:${config.port}`, - `localhost:${config.port}`, -]); - -// --------------------------------------------------------------------------- -// 1. DNS Rebinding Guard -// --------------------------------------------------------------------------- - -/** - * Reject requests whose `Host` header does not match an expected localhost - * value. This prevents DNS rebinding attacks from reaching the service when - * it is bound to the loopback interface. - */ -export function dnsRebindingGuard( - req: Request, - res: Response, - next: NextFunction, -): void { - const host = req.headers.host; - - if (!host || !allowedHosts.has(host)) { - logger.warn( - { host, ip: req.ip, method: req.method, url: req.originalUrl }, - 'DNS rebinding guard: blocked request with disallowed Host header', - ); - res.status(403).json({ error: 'Forbidden' }); - return; - } - - next(); -} +// DNS rebinding guard removed — Bearer token auth is sufficient // --------------------------------------------------------------------------- // 2. Shutdown Guard (factory) diff --git a/test/middleware.test.ts b/test/middleware.test.ts deleted file mode 100644 index 9cf24e8..0000000 --- a/test/middleware.test.ts +++ /dev/null @@ -1,153 +0,0 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest'; - -// --------------------------------------------------------------------------- -// Mocks -- must be declared before importing the module under test. -// --------------------------------------------------------------------------- - -// The middleware module reads `config.port` at module scope to build the -// allowedHosts set, so we need the mock in place before the import. -vi.mock('../src/config/index.js', () => ({ - config: { - port: 3000, - }, -})); - -vi.mock('../src/utils/logger.js', () => ({ - logger: { - error: vi.fn(), - warn: vi.fn(), - info: vi.fn(), - debug: vi.fn(), - child: vi.fn(() => ({ - error: vi.fn(), - warn: vi.fn(), - info: vi.fn(), - debug: vi.fn(), - })), - }, -})); - -vi.mock('../src/utils/errors.js', () => ({ - sanitizeErrorMessage: vi.fn((msg: string) => msg), -})); - -import { dnsRebindingGuard } from '../src/server/middleware.js'; - -// --------------------------------------------------------------------------- -// Helpers -- lightweight Express req/res/next fakes -// --------------------------------------------------------------------------- - -interface FakeRequest { - headers: Record; - ip?: string; - method?: string; - originalUrl?: string; -} - -interface FakeResponse { - statusCode: number; - body: unknown; - status: (code: number) => FakeResponse; - json: (data: unknown) => FakeResponse; -} - -function createReq(host?: string): FakeRequest { - return { - headers: host !== undefined ? { host } : {}, - ip: '127.0.0.1', - method: 'GET', - originalUrl: '/test', - }; -} - -function createRes(): FakeResponse { - const res: FakeResponse = { - statusCode: 200, - body: undefined, - status(code: number) { - res.statusCode = code; - return res; - }, - json(data: unknown) { - res.body = data; - return res; - }, - }; - return res; -} - -// --------------------------------------------------------------------------- -// dnsRebindingGuard -// --------------------------------------------------------------------------- - -describe('dnsRebindingGuard', () => { - let next: ReturnType; - - beforeEach(() => { - next = vi.fn(); - }); - - it('allows requests with Host: 127.0.0.1', () => { - const req = createReq('127.0.0.1'); - const res = createRes(); - - dnsRebindingGuard(req as any, res as any, next); - - expect(next).toHaveBeenCalledTimes(1); - expect(res.statusCode).toBe(200); - }); - - it('allows requests with Host: localhost', () => { - const req = createReq('localhost'); - const res = createRes(); - - dnsRebindingGuard(req as any, res as any, next); - - expect(next).toHaveBeenCalledTimes(1); - }); - - it('allows requests with Host: localhost:', () => { - const req = createReq('localhost:3000'); - const res = createRes(); - - dnsRebindingGuard(req as any, res as any, next); - - expect(next).toHaveBeenCalledTimes(1); - }); - - it('allows requests with Host: 127.0.0.1:', () => { - const req = createReq('127.0.0.1:3000'); - const res = createRes(); - - dnsRebindingGuard(req as any, res as any, next); - - expect(next).toHaveBeenCalledTimes(1); - }); - - it('blocks requests with Host: evil.com', () => { - const req = createReq('evil.com'); - const res = createRes(); - - dnsRebindingGuard(req as any, res as any, next); - - expect(next).not.toHaveBeenCalled(); - expect(res.statusCode).toBe(403); - expect(res.body).toEqual({ error: 'Forbidden' }); - }); - - it('blocks requests with no Host header', () => { - const req: FakeRequest = { - headers: {}, - ip: '127.0.0.1', - method: 'GET', - originalUrl: '/test', - }; - const res = createRes(); - - dnsRebindingGuard(req as any, res as any, next); - - expect(next).not.toHaveBeenCalled(); - expect(res.statusCode).toBe(403); - expect(res.body).toEqual({ error: 'Forbidden' }); - }); -});