Skip to content

v2 remove console warns on middleware #1776

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
KKonstantinov wants to merge 5 commits into
base: main
Choose a base branch
from
+135 −116
Open

v2 remove console warns on middleware #1776

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d75a414
add skipostHeaderValidation option, default false, remove console war…
KKonstantinov Mar 26, 2026
e44e301
Merge branch 'main' into middleware-remove-console-warn
KKonstantinov Mar 26, 2026
d920205
test fixes
KKonstantinov Mar 26, 2026
9ee628f
Merge branch 'middleware-remove-console-warn' of github.com:modelcont…
KKonstantinov Mar 26, 2026
28fd4ad
Merge branch 'main' into middleware-remove-console-warn
KKonstantinov Mar 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 44 additions & 29 deletions packages/middleware/express/src/express.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,47 @@ import express from 'express';

import { hostHeaderValidation, localhostHostValidation } from './middleware/hostHeaderValidation.js';

/**
* Host header validation options for DNS rebinding protection.
*
* Either skip validation entirely, or optionally provide an explicit allowlist.
*/
export type HostHeaderValidationOptions =
| {
/**
* When set to `true`, disables all automatic host header validation
* (DNS rebinding protection).
*
* Use this when the server sits behind a reverse proxy or load balancer
* that rewrites the `Host` header, or when running in an isolated network
* (e.g., containers) where DNS rebinding is not a concern.
*/
skipHostHeaderValidation: true;
allowedHosts?: never;
}
| {
skipHostHeaderValidation?: false;
/**
* List of allowed hostnames for DNS rebinding protection.
* If provided, host header validation will be applied using this list.
* For IPv6, provide addresses with brackets (e.g., `'[::1]'`).
*
* This is useful when binding to `'0.0.0.0'` or `'::'` but still wanting
* to restrict which hostnames are allowed.
*/
allowedHosts?: string[];
};

/**
* Options for creating an MCP Express application.
*/
export interface CreateMcpExpressAppOptions {
export type CreateMcpExpressAppOptions = {
/**
* The hostname to bind to. Defaults to `'127.0.0.1'`.
* When set to `'127.0.0.1'`, `'localhost'`, or `'::1'`, DNS rebinding protection is automatically enabled.
*/
host?: string;

/**
* List of allowed hostnames for DNS rebinding protection.
* If provided, host header validation will be applied using this list.
* For IPv6, provide addresses with brackets (e.g., `'[::1]'`).
*
* This is useful when binding to `'0.0.0.0'` or `'::'` but still wanting
* to restrict which hostnames are allowed.
*/
allowedHosts?: string[];

/**
* Controls the maximum request body size for the JSON body parser.
* Passed directly to Express's `express.json({ limit })` option.
Expand All @@ -31,7 +52,7 @@ export interface CreateMcpExpressAppOptions {
* @example '1mb', '500kb', '10mb'
*/
jsonLimit?: string;
}
} & HostHeaderValidationOptions;

/**
* Creates an Express application pre-configured for MCP servers.
Expand Down Expand Up @@ -60,27 +81,21 @@ export interface CreateMcpExpressAppOptions {
* ```
*/
export function createMcpExpressApp(options: CreateMcpExpressAppOptions = {}): Express {
const { host = '127.0.0.1', allowedHosts, jsonLimit } = options;
const { host = '127.0.0.1', allowedHosts, jsonLimit, skipHostHeaderValidation } = options;

const app = express();
app.use(express.json(jsonLimit ? { limit: jsonLimit } : undefined));

// If allowedHosts is explicitly provided, use that for validation
if (allowedHosts) {
app.use(hostHeaderValidation(allowedHosts));
} else {
// Apply DNS rebinding protection automatically for localhost hosts
const localhostHosts = ['127.0.0.1', 'localhost', '::1'];
if (localhostHosts.includes(host)) {
app.use(localhostHostValidation());
} else if (host === '0.0.0.0' || host === '::') {
// Warn when binding to all interfaces without DNS rebinding protection
// eslint-disable-next-line no-console
console.warn(
`Warning: Server is binding to ${host} without DNS rebinding protection. ` +
'Consider using the allowedHosts option to restrict allowed hosts, ' +
'or use authentication to protect your server.'
);
if (!skipHostHeaderValidation) {
// If allowedHosts is explicitly provided, use that for validation
if (allowedHosts) {
app.use(hostHeaderValidation(allowedHosts));
} else {
// Apply DNS rebinding protection automatically for localhost hosts
const localhostHosts = ['127.0.0.1', 'localhost', '::1'];
if (localhostHosts.includes(host)) {
app.use(localhostHostValidation());
}
}
}

Expand Down
48 changes: 10 additions & 38 deletions packages/middleware/express/test/express.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,55 +128,27 @@ describe('@modelcontextprotocol/express', () => {
});

test('should use allowedHosts when provided', () => {
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
const app = createMcpExpressApp({ host: '0.0.0.0', allowedHosts: ['myapp.local'] });
warn.mockRestore();

expect(app).toBeDefined();
});

test('should warn when binding to 0.0.0.0 without allowedHosts', () => {
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});

createMcpExpressApp({ host: '0.0.0.0' });

expect(warn).toHaveBeenCalledWith(
expect.stringContaining('Warning: Server is binding to 0.0.0.0 without DNS rebinding protection')
);

warn.mockRestore();
});

test('should warn when binding to :: without allowedHosts', () => {
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});

createMcpExpressApp({ host: '::' });

expect(warn).toHaveBeenCalledWith(expect.stringContaining('Warning: Server is binding to :: without DNS rebinding protection'));
test('should not apply host validation for non-localhost hosts without allowedHosts', () => {
// For arbitrary hosts (not 0.0.0.0 or ::), no validation is applied
const app = createMcpExpressApp({ host: '192.168.1.1' });

warn.mockRestore();
expect(app).toBeDefined();
});

test('should not warn for 0.0.0.0 when allowedHosts is provided', () => {
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});

createMcpExpressApp({ host: '0.0.0.0', allowedHosts: ['myapp.local'] });

expect(warn).not.toHaveBeenCalled();

warn.mockRestore();
test('should skip host header validation when skipHostHeaderValidation is true', () => {
// HTTP-level verification is in integration tests (test/integration/test/server.test.ts)
const app = createMcpExpressApp({ host: '127.0.0.1', skipHostHeaderValidation: true });
expect(app).toBeDefined();
});

test('should not apply host validation for non-localhost hosts without allowedHosts', () => {
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});

// For arbitrary hosts (not 0.0.0.0 or ::), no validation is applied and no warning
const app = createMcpExpressApp({ host: '192.168.1.1' });

expect(warn).not.toHaveBeenCalled();
test('should skip host header validation for 0.0.0.0 when skipHostHeaderValidation is true', () => {
const app = createMcpExpressApp({ host: '0.0.0.0', skipHostHeaderValidation: true });
expect(app).toBeDefined();

warn.mockRestore();
});

test('should accept jsonLimit option', () => {
Expand Down
73 changes: 44 additions & 29 deletions packages/middleware/hono/src/hono.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,47 @@ import { Hono } from 'hono';

import { hostHeaderValidation, localhostHostValidation } from './middleware/hostHeaderValidation.js';

/**
* Host header validation options for DNS rebinding protection.
*
* Either skip validation entirely, or optionally provide an explicit allowlist.
*/
export type HostHeaderValidationOptions =
| {
/**
* When set to `true`, disables all automatic host header validation
* (DNS rebinding protection).
*
* Use this when the server sits behind a reverse proxy or load balancer
* that rewrites the `Host` header, or when running in an isolated network
* (e.g., containers) where DNS rebinding is not a concern.
*/
skipHostHeaderValidation: true;
allowedHosts?: never;
}
| {
skipHostHeaderValidation?: false;
/**
* List of allowed hostnames for DNS rebinding protection.
* If provided, host header validation will be applied using this list.
* For IPv6, provide addresses with brackets (e.g., '[::1]').
*
* This is useful when binding to '0.0.0.0' or '::' but still wanting
* to restrict which hostnames are allowed.
*/
allowedHosts?: string[];
};

/**
* Options for creating an MCP Hono application.
*/
export interface CreateMcpHonoAppOptions {
export type CreateMcpHonoAppOptions = {
/**
* The hostname to bind to. Defaults to `'127.0.0.1'`.
* When set to `'127.0.0.1'`, `'localhost'`, or `'::1'`, DNS rebinding protection is automatically enabled.
*/
host?: string;

/**
* List of allowed hostnames for DNS rebinding protection.
* If provided, host header validation will be applied using this list.
* For IPv6, provide addresses with brackets (e.g., '[::1]').
*
* This is useful when binding to '0.0.0.0' or '::' but still wanting
* to restrict which hostnames are allowed.
*/
allowedHosts?: string[];
}
} & HostHeaderValidationOptions;

/**
* Creates a Hono application pre-configured for MCP servers.
Expand All @@ -39,7 +60,7 @@ export interface CreateMcpHonoAppOptions {
* @returns A configured Hono application
*/
export function createMcpHonoApp(options: CreateMcpHonoAppOptions = {}): Hono {
const { host = '127.0.0.1', allowedHosts } = options;
const { host = '127.0.0.1', allowedHosts, skipHostHeaderValidation } = options;

const app = new Hono();

Expand Down Expand Up @@ -67,22 +88,16 @@ export function createMcpHonoApp(options: CreateMcpHonoAppOptions = {}): Hono {
return await next();
});

// If allowedHosts is explicitly provided, use that for validation.
if (allowedHosts) {
app.use('*', hostHeaderValidation(allowedHosts));
} else {
// Apply DNS rebinding protection automatically for localhost hosts.
const localhostHosts = ['127.0.0.1', 'localhost', '::1'];
if (localhostHosts.includes(host)) {
app.use('*', localhostHostValidation());
} else if (host === '0.0.0.0' || host === '::') {
// Warn when binding to all interfaces without DNS rebinding protection.
// eslint-disable-next-line no-console
console.warn(
`Warning: Server is binding to ${host} without DNS rebinding protection. ` +
'Consider using the allowedHosts option to restrict allowed hosts, ' +
'or use authentication to protect your server.'
);
if (!skipHostHeaderValidation) {
// If allowedHosts is explicitly provided, use that for validation.
if (allowedHosts) {
app.use('*', hostHeaderValidation(allowedHosts));
} else {
// Apply DNS rebinding protection automatically for localhost hosts.
const localhostHosts = ['127.0.0.1', 'localhost', '::1'];
if (localhostHosts.includes(host)) {
app.use('*', localhostHostValidation());
}
}
}

Expand Down
24 changes: 19 additions & 5 deletions packages/middleware/hono/test/hono.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import type { Context } from 'hono';
import { Hono } from 'hono';
import { vi } from 'vitest';

import { createMcpHonoApp } from '../src/hono.js';
import { hostHeaderValidation } from '../src/middleware/hostHeaderValidation.js';
Expand Down Expand Up @@ -40,9 +39,7 @@ describe('@modelcontextprotocol/hono', () => {
});

test('createMcpHonoApp uses allowedHosts when provided (even when binding to 0.0.0.0)', async () => {
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
const app = createMcpHonoApp({ host: '0.0.0.0', allowedHosts: ['myapp.local'] });
warn.mockRestore();

app.get('/health', c => c.text('ok'));

Expand All @@ -54,16 +51,33 @@ describe('@modelcontextprotocol/hono', () => {
});

test('createMcpHonoApp does not apply host validation for 0.0.0.0 without allowedHosts', async () => {
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
const app = createMcpHonoApp({ host: '0.0.0.0' });
warn.mockRestore();

app.get('/health', c => c.text('ok'));

const res = await app.request('http://localhost/health', { headers: { Host: 'evil.com:3000' } });
expect(res.status).toBe(200);
});

test('createMcpHonoApp skips all host validation when skipHostHeaderValidation is true', async () => {
const app = createMcpHonoApp({ host: '127.0.0.1', skipHostHeaderValidation: true });

app.get('/health', c => c.text('ok'));

// Would normally be blocked by localhost validation, but skipHostHeaderValidation disables it
const res = await app.request('http://localhost/health', { headers: { Host: 'evil.com:3000' } });
expect(res.status).toBe(200);
});

test('createMcpHonoApp skips validation for 0.0.0.0 when skipHostHeaderValidation is true', async () => {
const app = createMcpHonoApp({ host: '0.0.0.0', skipHostHeaderValidation: true });

app.get('/health', c => c.text('ok'));

const res = await app.request('http://localhost/health', { headers: { Host: 'anything.com:3000' } });
expect(res.status).toBe(200);
});

test('createMcpHonoApp parses JSON bodies into parsedBody (express.json()-like)', async () => {
const app = createMcpHonoApp();
app.post('/echo', (c: Context) => c.json(c.get('parsedBody')));
Expand Down
33 changes: 18 additions & 15 deletions test/integration/test/server.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2308,31 +2308,34 @@
expect(response.status).toBe(403);
});

test('should warn when binding to 0.0.0.0', () => {
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
createMcpExpressApp({ host: '0.0.0.0' });
expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('0.0.0.0'));
warnSpy.mockRestore();
test('should not apply host validation for 0.0.0.0 without allowedHosts', async () => {
const app = createMcpExpressApp({ host: '0.0.0.0' });
app.post('/test', (_req: Request, res: Response) => {
res.json({ success: true });
});

// No host validation applied, so any host should be accepted
const response = await supertest(app).post('/test').set('Host', 'anything.com:3000').send({});
expect(response.status).toBe(200);
});

Check warning on line 2320 in test/integration/test/server.test.ts

View check run for this annotation

Claude / Claude Code Review

Duplicate integration test replaces missing :: coverage 

Nit: The new test at line 2311 ("should not apply host validation for 0.0.0.0 without allowedHosts") is functionally identical to the pre-existing test at line 2262 ("should not apply host validation when host is 0.0.0.0") — both create an app with `host: '0.0.0.0'`, POST with an arbitrary Host header, and assert 200. This should instead cover the `::` (IPv6 all-interfaces) case, which lost its test coverage when the old console.warn test was removed.
Comment on lines +2311 to 2320
Copy link

@claude claude bot Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Nit: The new test at line 2311 ("should not apply host validation for 0.0.0.0 without allowedHosts") is functionally identical to the pre-existing test at line 2262 ("should not apply host validation when host is 0.0.0.0") — both create an app with host: '0.0.0.0', POST with an arbitrary Host header, and assert 200. This should instead cover the :: (IPv6 all-interfaces) case, which lost its test coverage when the old console.warn test was removed.

Extended reasoning...

The Duplicate

The test at line 2262 ("should not apply host validation when host is 0.0.0.0") and the new test at line 2311 ("should not apply host validation for 0.0.0.0 without allowedHosts") are functionally identical:

  • Both call createMcpExpressApp({ host: '0.0.0.0' })
  • Both add a POST /test handler that returns { success: true }
  • Both send a request with an arbitrary Host header (any-host.com:3000 vs anything.com:3000)
  • Both assert response.status is 200

The only differences are the test name, the specific host header string used, and the older test also asserts response.body.

How This Happened

Before this PR, there were two console.warn tests — one for 0.0.0.0 (line 2311 in the old code) and one for :: (line 2318 in the old code). The PR replaced both with behavioral tests, but the replacement for the 0.0.0.0 warn test duplicates the pre-existing behavioral test at line 2262 instead of testing something new.

Step-by-Step Proof

  1. Line 2262: createMcpExpressApp({ host: '0.0.0.0' }) → POST /test with Host: any-host.com:3000 → assert 200
  2. Line 2311: createMcpExpressApp({ host: '0.0.0.0' }) → POST /test with Host: anything.com:3000 → assert 200
  3. These exercise the exact same code path: host is '0.0.0.0', which is not in localhostHosts, so no validation middleware is applied, and any Host header is accepted.

Lost Coverage

The old test "should warn when binding to :: (IPv6 all interfaces)" was the only test exercising the :: host case. When it was removed, no replacement test for host: '::' was added. The :: case follows the same code path as 0.0.0.0 (not in localhostHosts, no validation applied), but it still deserves its own test to confirm IPv6 all-interfaces behavior.

Impact

This is a minor issue — both tests pass fine and no incorrect behavior results. It is test redundancy and a missed opportunity to cover the :: case.

Suggested Fix

Change the test at line 2311 to use host: '::' instead of host: '0.0.0.0', making it cover the IPv6 all-interfaces case that lost its test.


test('should warn when binding to :: (IPv6 all interfaces)', () => {
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
createMcpExpressApp({ host: '::' });
expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('::'));
warnSpy.mockRestore();
test('should skip host validation when skipHostHeaderValidation is true', async () => {
const app = createMcpExpressApp({ host: '127.0.0.1', skipHostHeaderValidation: true });
app.post('/test', (_req: Request, res: Response) => {
res.json({ success: true });
});

// Localhost validation would normally block this, but skipHostHeaderValidation disables it
const response = await supertest(app).post('/test').set('Host', 'evil.com:3000').send({});
expect(response.status).toBe(200);
});

test('should use custom allowedHosts when provided', async () => {
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
const app = createMcpExpressApp({ host: '0.0.0.0', allowedHosts: ['myapp.local', 'localhost'] });
app.post('/test', (_req: Request, res: Response) => {
res.json({ success: true });
});

// Should not warn when allowedHosts is provided
expect(warnSpy).not.toHaveBeenCalled();
warnSpy.mockRestore();

// Should allow myapp.local
const allowedResponse = await supertest(app).post('/test').set('Host', 'myapp.local:3000').send({});
expect(allowedResponse.status).toBe(200);
Expand Down
Loading