Skip to content

Commit 82bac90

Browse files
author
Růžička, David
committed
Merge branch 'dr-extend-upstream-mcp-description-filter' into 'main'
feat: add tool_description_length_policy to upstream MCP provider config See merge request ai-adoption/mcp/mcp4openapi!10
2 parents 102834b + b173551 commit 82bac90

5 files changed

Lines changed: 173 additions & 22 deletions

File tree

src/generated-schemas.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,7 @@ export const upstreamMcpServerConfigSchema = z.object({
360360
tools: upstreamMcpToolPolicySchema.optional(),
361361
timeout_ms: z.number().optional(),
362362
html_description_policy: z.enum(['allow', 'strip', 'drop']).optional(),
363+
tool_description_length_policy: z.enum(['drop', 'truncate', 'allow']).optional(),
363364
validation_endpoint: z.string().optional(),
364365
validation_method: z.union([z.literal("HEAD"), z.literal("GET")]).optional(),
365366
validation_timeout_ms: z.number().optional()

src/mcp/mcp-server.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1906,7 +1906,7 @@ export class MCPServer {
19061906
);
19071907
}
19081908
const rawTools = result.tools;
1909-
const sanitized = sanitizeToolList(rawTools, this.logger, provider.html_description_policy ?? 'drop');
1909+
const sanitized = sanitizeToolList(rawTools, this.logger, provider.html_description_policy ?? 'drop', provider.tool_description_length_policy ?? 'drop');
19101910
const policyFiltered = applyProviderToolPolicy(sanitized.tools, provider.tools);
19111911
// Cache sanitized+policy-filtered tool names for tools/call gate enforcement.
19121912
// Tools dropped here (bad description/inputSchema) must not be callable via tools/call.

src/types/profile.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,14 @@ export interface UpstreamMcpServerConfig {
104104
*/
105105
html_description_policy?: 'allow' | 'strip' | 'drop';
106106

107+
/**
108+
* Controls how upstream tools with descriptions exceeding the internal length limit are handled.
109+
* - drop (default): tools with overlong descriptions are dropped
110+
* - truncate: description is truncated to the limit; tool is kept
111+
* - allow: description length check is skipped; tool passes through unchanged
112+
*/
113+
tool_description_length_policy?: 'drop' | 'truncate' | 'allow';
114+
107115
/** Optional endpoint to validate upstream credentials at session init (fail-fast). */
108116
validation_endpoint?: string;
109117
/** HTTP method for validation probe. Default: 'HEAD'. */

src/upstream/upstream-tool-sanitizer.test.ts

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import { describe, it, expect, vi, beforeEach } from 'vitest';
66
import type { Logger } from '../core/logger.js';
77
import { sanitizeToolList, applyProviderToolPolicy, isToolAllowedByProviderPolicy, isValidUpstreamToolName } from './upstream-tool-sanitizer.js';
8+
import type { ToolDescriptionLengthPolicy } from './upstream-tool-sanitizer.js';
89
import type { Tool } from '@modelcontextprotocol/sdk/types.js';
910

1011
function makeTool(name: string, description?: string): Tool {
@@ -503,6 +504,125 @@ describe('sanitizeToolList — html_description_policy', () => {
503504
});
504505
});
505506

507+
describe('sanitizeToolList — tool_description_length_policy', () => {
508+
const LONG_DESC = 'a'.repeat(2049);
509+
const EXACT_DESC = 'a'.repeat(2048);
510+
let logger: Logger;
511+
512+
beforeEach(() => {
513+
logger = { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() };
514+
});
515+
516+
describe('drop (default)', () => {
517+
it('drops tool with description > 2048 chars', () => {
518+
const tool = makeTool('t', LONG_DESC);
519+
const result = sanitizeToolList([tool], logger, 'drop', 'drop');
520+
expect(result.tools).toHaveLength(0);
521+
expect(result.dropped[0].reason).toBe('tool description too long');
522+
});
523+
524+
it('passes tool with description exactly 2048 chars', () => {
525+
const tool = makeTool('t', EXACT_DESC);
526+
const result = sanitizeToolList([tool], logger, 'drop', 'drop');
527+
expect(result.tools).toHaveLength(1);
528+
expect(result.dropped).toHaveLength(0);
529+
});
530+
});
531+
532+
describe('truncate', () => {
533+
it('keeps tool and truncates description to 2048 chars', () => {
534+
const tool = makeTool('t', LONG_DESC);
535+
const result = sanitizeToolList([tool], logger, 'drop', 'truncate');
536+
expect(result.tools).toHaveLength(1);
537+
expect(result.tools[0].description).toHaveLength(2048);
538+
expect(result.tools[0].description).toBe(LONG_DESC.slice(0, 2048));
539+
expect(result.dropped).toHaveLength(0);
540+
expect(logger.warn).not.toHaveBeenCalled();
541+
});
542+
543+
it('does not mutate original tool object', () => {
544+
const tool = makeTool('t', LONG_DESC);
545+
sanitizeToolList([tool], logger, 'drop', 'truncate');
546+
expect(tool.description).toBe(LONG_DESC);
547+
});
548+
549+
it('passes tool with description exactly 2048 chars unchanged', () => {
550+
const tool = makeTool('t', EXACT_DESC);
551+
const result = sanitizeToolList([tool], logger, 'drop', 'truncate');
552+
expect(result.tools).toHaveLength(1);
553+
expect(result.tools[0].description).toHaveLength(2048);
554+
});
555+
556+
it('still drops tool with invalid name regardless of truncate policy', () => {
557+
const tool = makeTool('bad name!', LONG_DESC);
558+
const result = sanitizeToolList([tool], logger, 'drop', 'truncate');
559+
expect(result.tools).toHaveLength(0);
560+
expect(result.dropped[0].reason).toBe('invalid characters in tool name');
561+
});
562+
563+
it('still drops tool with malformed inputSchema regardless of truncate policy', () => {
564+
const tool = { name: 'valid', description: LONG_DESC, inputSchema: null } as unknown as Tool;
565+
const result = sanitizeToolList([tool], logger, 'drop', 'truncate');
566+
expect(result.tools).toHaveLength(0);
567+
expect(result.dropped[0].reason).toBe('malformed tool definition: inputSchema is not an object');
568+
});
569+
});
570+
571+
describe('allow', () => {
572+
it('passes tool with overlong description through unchanged', () => {
573+
const tool = makeTool('t', LONG_DESC);
574+
const result = sanitizeToolList([tool], logger, 'drop', 'allow');
575+
expect(result.tools).toHaveLength(1);
576+
expect(result.tools[0].description).toBe(LONG_DESC);
577+
expect(result.dropped).toHaveLength(0);
578+
expect(logger.warn).not.toHaveBeenCalled();
579+
});
580+
581+
it('still drops tool with invalid name regardless of allow policy', () => {
582+
const tool = makeTool('bad name!', LONG_DESC);
583+
const result = sanitizeToolList([tool], logger, 'drop', 'allow');
584+
expect(result.tools).toHaveLength(0);
585+
expect(result.dropped[0].reason).toBe('invalid characters in tool name');
586+
});
587+
588+
it('still drops tool with malformed inputSchema regardless of allow policy', () => {
589+
const tool = { name: 'valid', description: LONG_DESC, inputSchema: null } as unknown as Tool;
590+
const result = sanitizeToolList([tool], logger, 'drop', 'allow');
591+
expect(result.tools).toHaveLength(0);
592+
expect(result.dropped[0].reason).toBe('malformed tool definition: inputSchema is not an object');
593+
});
594+
});
595+
596+
describe('interactions with html_description_policy', () => {
597+
it('truncate length + strip html: truncates first then strips HTML from truncated string', () => {
598+
// desc = '<b>' + 'a' * 2049 + '</b>' = 2056 chars
599+
// truncate to 2048: '<b>' + 'a' * 2045 (first 2048 chars)
600+
// then strip HTML: 'a' * 2045
601+
const desc = '<b>' + 'a'.repeat(2049) + '</b>';
602+
const tool = makeTool('t', desc);
603+
const result = sanitizeToolList([tool], logger, 'strip', 'truncate');
604+
expect(result.tools).toHaveLength(1);
605+
expect(result.tools[0].description).toBe('a'.repeat(2045));
606+
expect(result.dropped).toHaveLength(0);
607+
});
608+
609+
it('allow length + drop html: still drops tool with HTML chars in description', () => {
610+
const tool = makeTool('t', '<b>' + 'a'.repeat(2049) + '</b>');
611+
const result = sanitizeToolList([tool], logger, 'drop', 'allow');
612+
expect(result.tools).toHaveLength(0);
613+
expect(result.dropped[0].reason).toBe('forbidden characters in description');
614+
});
615+
616+
it('allow length + allow html: passes overlong HTML description through unchanged', () => {
617+
const desc = '<b>' + 'a'.repeat(2049) + '</b>';
618+
const tool = makeTool('t', desc);
619+
const result = sanitizeToolList([tool], logger, 'allow', 'allow');
620+
expect(result.tools).toHaveLength(1);
621+
expect(result.tools[0].description).toBe(desc);
622+
});
623+
});
624+
});
625+
506626
describe('applyProviderToolPolicy', () => {
507627
const tools = [makeTool('alpha'), makeTool('beta'), makeTool('gamma')];
508628

src/upstream/upstream-tool-sanitizer.ts

Lines changed: 43 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import type { Logger } from '../core/logger.js';
1919
import type { UpstreamMcpToolPolicy } from '../types/profile.js';
2020

2121
export type HtmlDescriptionPolicy = 'allow' | 'strip' | 'drop';
22+
export type ToolDescriptionLengthPolicy = 'drop' | 'truncate' | 'allow';
2223

2324
export interface SanitizationResult {
2425
tools: Tool[];
@@ -97,15 +98,23 @@ const truncateName = (name: string): string =>
9798
* Each tool is checked in order:
9899
* 1. Name length <= 255
99100
* 2. Name matches [a-zA-Z0-9_-]
100-
* 3. Description length <= 2048 (if present)
101+
* 3. Description length policy (tool_description_length_policy):
102+
* - drop (default): tools with description > 2048 chars are dropped
103+
* - truncate: description is truncated to 2048 chars; tool is kept
104+
* - allow: description length check is skipped; tool passes through unchanged
101105
* 4. HTML policy (html_description_policy):
102106
* - drop (default): tools with <, >, or backtick in description/inputSchema are dropped
103107
* - strip: HTML tags stripped from description and inputSchema string values; tool kept
104108
* - allow: HTML checks skipped entirely; tool passes through as-is
105109
*
106110
* Offending tools are dropped and logged. Safe tools pass through unchanged.
107111
*/
108-
export function sanitizeToolList(tools: Tool[], logger?: Logger, htmlPolicy: HtmlDescriptionPolicy = 'drop'): SanitizationResult {
112+
export function sanitizeToolList(
113+
tools: Tool[],
114+
logger?: Logger,
115+
htmlPolicy: HtmlDescriptionPolicy = 'drop',
116+
lengthPolicy: ToolDescriptionLengthPolicy = 'drop',
117+
): SanitizationResult {
109118
const safe: Tool[] = [];
110119
const dropped: { name: string; reason: string }[] = [];
111120

@@ -131,28 +140,41 @@ export function sanitizeToolList(tools: Tool[], logger?: Logger, htmlPolicy: Htm
131140
reason = 'invalid characters in tool name';
132141
} else if (tool.description !== undefined && typeof tool.description !== 'string') {
133142
reason = 'malformed tool definition: description is not a string';
134-
} else if (tool.description && tool.description.length > MAX_DESCRIPTION_LENGTH) {
135-
reason = 'tool description too long';
136-
} else if (tool.inputSchema !== undefined && (typeof tool.inputSchema !== 'object' || tool.inputSchema === null || Array.isArray(tool.inputSchema))) {
137-
reason = 'malformed tool definition: inputSchema is not an object';
138-
} else if (htmlPolicy === 'drop') {
139-
if (tool.description && DESCRIPTION_FORBIDDEN_CHARS.test(tool.description)) {
140-
reason = 'forbidden characters in description';
141-
excerpt = firstForbiddenExcerpt(tool.description);
142-
} else if (tool.inputSchema && schemaContainsForbiddenChars(tool.inputSchema)) {
143-
reason = 'forbidden characters in input schema';
144-
const schemaStr = JSON.stringify(tool.inputSchema);
145-
excerpt = firstForbiddenExcerpt(schemaStr);
146-
}
147-
} else if (htmlPolicy === 'strip') {
148-
if (tool.description) {
149-
tool = { ...tool, description: stripHtmlTags(tool.description) };
143+
} else {
144+
// Description length policy: applies only to description length, not to other checks.
145+
// Evaluated independently so that 'allow'/'truncate' can still fall through to HTML checks.
146+
if (tool.description && tool.description.length > MAX_DESCRIPTION_LENGTH) {
147+
if (lengthPolicy === 'drop') {
148+
reason = 'tool description too long';
149+
} else if (lengthPolicy === 'truncate') {
150+
tool = { ...tool, description: tool.description.slice(0, MAX_DESCRIPTION_LENGTH) };
151+
}
152+
// lengthPolicy === 'allow': skip length check, continue to other checks unchanged
150153
}
151-
if (tool.inputSchema) {
152-
tool = { ...tool, inputSchema: stripHtmlFromSchema(tool.inputSchema) as Tool['inputSchema'] };
154+
155+
if (reason === undefined) {
156+
if (tool.inputSchema !== undefined && (typeof tool.inputSchema !== 'object' || tool.inputSchema === null || Array.isArray(tool.inputSchema))) {
157+
reason = 'malformed tool definition: inputSchema is not an object';
158+
} else if (htmlPolicy === 'drop') {
159+
if (tool.description && DESCRIPTION_FORBIDDEN_CHARS.test(tool.description)) {
160+
reason = 'forbidden characters in description';
161+
excerpt = firstForbiddenExcerpt(tool.description);
162+
} else if (tool.inputSchema && schemaContainsForbiddenChars(tool.inputSchema)) {
163+
reason = 'forbidden characters in input schema';
164+
const schemaStr = JSON.stringify(tool.inputSchema);
165+
excerpt = firstForbiddenExcerpt(schemaStr);
166+
}
167+
} else if (htmlPolicy === 'strip') {
168+
if (tool.description) {
169+
tool = { ...tool, description: stripHtmlTags(tool.description) };
170+
}
171+
if (tool.inputSchema) {
172+
tool = { ...tool, inputSchema: stripHtmlFromSchema(tool.inputSchema) as Tool['inputSchema'] };
173+
}
174+
}
175+
// htmlPolicy === 'allow': skip all HTML checks, pass tool through unchanged
153176
}
154177
}
155-
// htmlPolicy === 'allow': skip all HTML checks, pass tool through unchanged
156178

157179
if (reason !== undefined) {
158180
// Coerce non-string names to string for safe logging

0 commit comments

Comments
 (0)