Skip to content

Commit a866147

Browse files
committed
fix(server): sanitize internal error details in tool error responses
When a tool handler throws an unexpected error, the full error message was sent to the MCP client, potentially leaking sensitive server internals (hostnames, connection strings, stack traces). Adds a ToolError class that tool authors can throw when they want a specific message shown to the client. All other unhandled errors are now sanitized to "Internal error". ProtocolErrors from SDK validation are still passed through since they contain safe, structured messages. Closes #1429
1 parent 7ba58da commit a866147

File tree

3 files changed

+135
-5
lines changed

3 files changed

+135
-5
lines changed

packages/server/src/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ export type {
2323
ResourceMetadata,
2424
ToolCallback
2525
} from './server/mcp.js';
26-
export { McpServer, ResourceTemplate } from './server/mcp.js';
26+
export { McpServer, ResourceTemplate, ToolError } from './server/mcp.js';
2727
export type { HostHeaderValidationResult } from './server/middleware/hostHeaderValidation.js';
2828
export { hostHeaderValidationResponse, localhostAllowedHostnames, validateHostHeader } from './server/middleware/hostHeaderValidation.js';
2929
export type { ServerOptions } from './server/server.js';

packages/server/src/server/mcp.ts

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,22 @@ import { Server } from './server.js';
5858
* });
5959
* ```
6060
*/
61+
62+
/**
63+
* Error class for tool handlers to throw when they want to send a
64+
* user-visible error message to the client. Unlike regular errors,
65+
* ToolError messages are passed through to the client as-is.
66+
*
67+
* Regular errors thrown from tool handlers are sanitized to "Internal
68+
* error" to prevent leaking sensitive server internals.
69+
*/
70+
export class ToolError extends Error {
71+
constructor(message: string) {
72+
super(message);
73+
this.name = 'ToolError';
74+
}
75+
}
76+
6177
export class McpServer {
6278
/**
6379
* The underlying {@linkcode Server} instance, useful for advanced operations like sending notifications.
@@ -209,7 +225,16 @@ export class McpServer {
209225
if (error instanceof ProtocolError && error.code === ProtocolErrorCode.UrlElicitationRequired) {
210226
throw error; // Return the error to the caller without wrapping in CallToolResult
211227
}
212-
return this.createToolError(error instanceof Error ? error.message : String(error));
228+
if (error instanceof ProtocolError) {
229+
// SDK-generated validation errors are safe to expose
230+
return this.createToolError(error.message);
231+
}
232+
if (error instanceof ToolError) {
233+
// Developer intentionally wants this message shown to client
234+
return this.createToolError(error.message);
235+
}
236+
// All other errors: sanitize to prevent leaking internals
237+
return this.createToolError('Internal error');
213238
}
214239
});
215240

test/integration/test/server/mcp.test.ts

Lines changed: 108 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {
88
UriTemplate,
99
UrlElicitationRequiredError
1010
} from '@modelcontextprotocol/core';
11-
import { completable, McpServer, ResourceTemplate } from '@modelcontextprotocol/server';
11+
import { completable, McpServer, ResourceTemplate, ToolError } from '@modelcontextprotocol/server';
1212
import { afterEach, beforeEach, describe, expect, test } from 'vitest';
1313
import * as z from 'zod/v4';
1414

@@ -1799,7 +1799,112 @@ describe('Zod v4', () => {
17991799
expect(result.content).toEqual([
18001800
{
18011801
type: 'text',
1802-
text: 'Tool execution failed'
1802+
text: 'Internal error'
1803+
}
1804+
]);
1805+
});
1806+
1807+
test('should pass through ToolError message to client', async () => {
1808+
const mcpServer = new McpServer({
1809+
name: 'test server',
1810+
version: '1.0'
1811+
});
1812+
1813+
const client = new Client({
1814+
name: 'test client',
1815+
version: '1.0'
1816+
});
1817+
1818+
mcpServer.registerTool('toolerror-test', {}, async () => {
1819+
throw new ToolError('Invalid input: country not supported');
1820+
});
1821+
1822+
const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
1823+
1824+
await Promise.all([client.connect(clientTransport), mcpServer.server.connect(serverTransport)]);
1825+
1826+
const result = await client.request({
1827+
method: 'tools/call',
1828+
params: {
1829+
name: 'toolerror-test'
1830+
}
1831+
});
1832+
1833+
expect(result.isError).toBe(true);
1834+
expect(result.content).toEqual([
1835+
{
1836+
type: 'text',
1837+
text: 'Invalid input: country not supported'
1838+
}
1839+
]);
1840+
});
1841+
1842+
test('should sanitize generic Error to Internal error', async () => {
1843+
const mcpServer = new McpServer({
1844+
name: 'test server',
1845+
version: '1.0'
1846+
});
1847+
1848+
const client = new Client({
1849+
name: 'test client',
1850+
version: '1.0'
1851+
});
1852+
1853+
mcpServer.registerTool('internal-error-test', {}, async () => {
1854+
throw new Error('Connection failed at 10.0.0.5:5432');
1855+
});
1856+
1857+
const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
1858+
1859+
await Promise.all([client.connect(clientTransport), mcpServer.server.connect(serverTransport)]);
1860+
1861+
const result = await client.request({
1862+
method: 'tools/call',
1863+
params: {
1864+
name: 'internal-error-test'
1865+
}
1866+
});
1867+
1868+
expect(result.isError).toBe(true);
1869+
expect(result.content).toEqual([
1870+
{
1871+
type: 'text',
1872+
text: 'Internal error'
1873+
}
1874+
]);
1875+
});
1876+
1877+
test('should sanitize non-Error throws to Internal error', async () => {
1878+
const mcpServer = new McpServer({
1879+
name: 'test server',
1880+
version: '1.0'
1881+
});
1882+
1883+
const client = new Client({
1884+
name: 'test client',
1885+
version: '1.0'
1886+
});
1887+
1888+
mcpServer.registerTool('string-throw-test', {}, async () => {
1889+
throw 'some raw string error';
1890+
});
1891+
1892+
const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
1893+
1894+
await Promise.all([client.connect(clientTransport), mcpServer.server.connect(serverTransport)]);
1895+
1896+
const result = await client.request({
1897+
method: 'tools/call',
1898+
params: {
1899+
name: 'string-throw-test'
1900+
}
1901+
});
1902+
1903+
expect(result.isError).toBe(true);
1904+
expect(result.content).toEqual([
1905+
{
1906+
type: 'text',
1907+
text: 'Internal error'
18031908
}
18041909
]);
18051910
});
@@ -6972,7 +7077,7 @@ describe('Zod v4', () => {
69727077

69737078
// Should receive an error since cancelled tasks don't have results
69747079
expect(result).toHaveProperty('content');
6975-
expect(result.content).toEqual([{ type: 'text' as const, text: expect.stringContaining('has no result stored') }]);
7080+
expect(result.content).toEqual([{ type: 'text' as const, text: 'Internal error' }]);
69767081

69777082
// Wait for async operations to complete
69787083
await waitForLatch();

0 commit comments

Comments
 (0)