Skip to content

Commit f991d7a

Browse files
authored
fix(server): guard against non-string content text in tool results (#50)
JSON.stringify(undefined) returns the VALUE undefined, so any domain handler that stringifies a missing SDK response (e.g. actor.get() returning undefined from @wyre-technology/node-huntress) emitted {type: 'text', text: undefined}, which fails MCP client Zod validation (invalid_union on content[0]) on every call. Surfaced by the EpiOn customer tool audit via the Conduit gateway: huntress_accounts_actor was broken on every call. Sanitize at the single dispatch choke point in server.ts so every domain (accounts, agents, organizations, incidents, billing, signals, users) is covered: coerce any non-string text to the string 'null' and log a warning when the guard fires.
1 parent 8928e23 commit f991d7a

2 files changed

Lines changed: 68 additions & 2 deletions

File tree

src/__tests__/server.test.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
2+
import { Client } from '@modelcontextprotocol/sdk/client/index.js';
3+
import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js';
4+
import { createServer } from '../server.js';
5+
6+
// Regression for the EpiOn audit finding: the node-huntress SDK can return
7+
// undefined from client.actor.get(), and JSON.stringify(undefined, null, 2)
8+
// yields the VALUE undefined — so the server emitted
9+
// {type: 'text', text: undefined}, which fails MCP client Zod validation
10+
// (invalid_union on content[0]) on every call.
11+
vi.mock('@wyre-technology/node-huntress', () => ({
12+
HuntressClient: class {
13+
accounts = { get: vi.fn().mockResolvedValue(undefined) };
14+
actor = { get: vi.fn().mockResolvedValue(undefined) };
15+
},
16+
}));
17+
18+
describe('server content guard', () => {
19+
beforeEach(() => {
20+
process.env.HUNTRESS_API_KEY = 'test-key';
21+
process.env.HUNTRESS_API_SECRET = 'test-secret';
22+
});
23+
24+
afterEach(() => {
25+
delete process.env.HUNTRESS_API_KEY;
26+
delete process.env.HUNTRESS_API_SECRET;
27+
});
28+
29+
async function connectedClient(): Promise<Client> {
30+
const server = createServer();
31+
const client = new Client({ name: 'test-client', version: '1.0.0' });
32+
const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
33+
await Promise.all([server.connect(serverTransport), client.connect(clientTransport)]);
34+
return client;
35+
}
36+
37+
it('never emits a text content block whose text is not a string, even when the SDK returns undefined', async () => {
38+
const client = await connectedClient();
39+
40+
const result = await client.callTool({ name: 'huntress_accounts_actor', arguments: {} });
41+
42+
const content = result.content as Array<{ type: string; text: unknown }>;
43+
expect(Array.isArray(content)).toBe(true);
44+
expect(content.length).toBeGreaterThan(0);
45+
for (const block of content) {
46+
expect(block.type).toBe('text');
47+
expect(typeof block.text).toBe('string');
48+
}
49+
});
50+
});

src/server.ts

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,24 @@ import { getNavigationTools, DOMAINS } from './domains/navigation.js';
44
import { getDomainHandler } from './domains/index.js';
55
import { getCredentials } from './utils/client.js';
66
import { logger } from './utils/logger.js';
7-
import type { DomainName } from './utils/types.js';
7+
import type { CallToolResult, DomainName } from './utils/types.js';
88
import { registerPromptHandlers } from './prompts.js';
99

10+
// Belt-and-braces guard: never emit a content block whose text is not a
11+
// string. JSON.stringify(undefined) returns the VALUE undefined, so any
12+
// handler that stringifies a missing SDK response would otherwise emit
13+
// {type: 'text', text: undefined} — which fails MCP client validation
14+
// (Zod invalid_union on content[0]) on every call.
15+
function sanitizeResult(toolName: string, result: CallToolResult): CallToolResult {
16+
for (const block of result.content) {
17+
if (typeof block.text !== 'string') {
18+
logger.warn('Coerced non-string content text to "null"', { tool: toolName, textType: typeof block.text });
19+
block.text = 'null';
20+
}
21+
}
22+
return result;
23+
}
24+
1025
export function createServer(): Server {
1126
const server = new Server(
1227
{ name: 'huntress-mcp', version: '1.0.0' },
@@ -78,7 +93,8 @@ export function createServer(): Server {
7893
const toolNames = handler.getTools().map(t => t.name);
7994
if (toolNames.includes(name)) {
8095
try {
81-
return await handler.handleCall(name, (args || {}) as Record<string, unknown>, extra);
96+
const result = await handler.handleCall(name, (args || {}) as Record<string, unknown>, extra);
97+
return sanitizeResult(name, result);
8298
} catch (error) {
8399
logger.error('Tool call failed', { tool: name, error: (error as Error).message });
84100
return {

0 commit comments

Comments
 (0)