Skip to content

Commit b173551

Browse files
author
David Ruzicka
committed
feat: add tool_description_length_policy to upstream MCP provider config
Adds a new optional field `tool_description_length_policy` on each `upstream_mcp` provider with values `drop` (default), `truncate`, and `allow`. Default `drop` preserves existing behaviour. `truncate` keeps the tool and shortens its description to the 2048-char limit. `allow` skips the length check entirely, exposing the upstream tool unchanged — intended for compatibility-preserving proxy deployments where dropping tools solely due to description length would break existing clients. The sanitizer is restructured so that the length policy and the HTML policy are evaluated independently: length is applied first, then the HTML check runs regardless of which length policy was selected. All other sanitisation checks (name validation, inputSchema structure, HTML content) remain active under every length policy value.
1 parent 102834b commit b173551

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)