From a8a05ace40c8c8a3514eb3a2ba8748e5779992c7 Mon Sep 17 00:00:00 2001 From: Maurice Date: Sat, 29 Aug 2026 12:03:13 +0200 Subject: [PATCH] fix(plugins): close the gaps between what a tool schema advertises and what it enforces `.meta()` advertises every node of the declaration; `nodeToZod` only walks the ones it can type. Everything below follows from that gap, so the keyword allowlist moves into `sanitiseNode`, which is the one walker that sees every node. A `pattern` was compiled straight into a RegExp and run on the main thread against caller input, with no timeout and nothing to abort it: `^(a+)+$` against 33 characters stops the whole server, not just MCP. That is an escape from the supervised child into the host event loop, and it does not take malice, since a hand-written email pattern is the classic accident. Quantified groups that repeat or alternate inside are refused, and the source is capped. `mcp:tools` counted as a side effect, so `readOnlyHint` was structurally unreachable: holding that grant is what lets a plugin advertise a tool at all. Every plugin tool was therefore advertised as potentially destructive and every client's auto-approval was off. It is a host entry point like `hook:*`, and is classified as one. Property names, `default` and `examples` were the strings that never met the sanitiser. A key may carry newlines, so `\n## SYSTEM\n...` in one defeated the newline collapse everything else goes through. Names are identifiers now; sample values keep their own keys but have their strings bounded. Also refused at parse time, which is what the rest of the file already does: an object `enum` member carrying `$ref` past the deep scan, a non-boolean `additionalProperties` advertised and checked by nothing, `enum` beside `properties` skipping the children, a root `enum`/`const`/`nullable` producing a non-object schema the SDK cannot read, and a draft-4 `exclusiveMinimum: true` dropped from the check while staying in the advertisement. `required` used `in`, so a field named `toString` read as present on every call. Nodes are built with a null prototype, and the three names Zod cannot round-trip are refused rather than silently lost. A result block that had to be shortened got no marker and no log line, which reads to the model like the whole answer. It is counted now, like a dropped one, and `isError` survives every envelope path. The security test file carried two raw NUL bytes, so git, GitHub and ripgrep all treated it as binary: no diff, no blame, no review. Escaped, same runtime value. --- .../contributions/plugin-mcp-tools.service.ts | 52 +++- server/src/nest/plugins/mcp-tool-schema.ts | 260 ++++++++++++++++-- .../unit/plugins/mcp-tool-annotations.test.ts | 14 + .../unit/plugins/mcp-tool-schema.test.ts | Bin 12946 -> 21741 bytes 4 files changed, 294 insertions(+), 32 deletions(-) diff --git a/server/src/nest/plugins/contributions/plugin-mcp-tools.service.ts b/server/src/nest/plugins/contributions/plugin-mcp-tools.service.ts index 99e575a89..72a143f11 100644 --- a/server/src/nest/plugins/contributions/plugin-mcp-tools.service.ts +++ b/server/src/nest/plugins/contributions/plugin-mcp-tools.service.ts @@ -192,18 +192,25 @@ function fitToBudget(blocks: Array<{ type: 'text'; text: string }>): Array<{ typ const out: Array<{ type: 'text'; text: string }> = []; let left = RESULT_MAX; let dropped = 0; + let cut = false; for (const block of blocks.slice(0, RESULT_BLOCKS_MAX)) { if (left <= 0) { dropped += 1; continue; } + // A block that had to be shortened counts too. Without this, one 100 KB + // block came back silently clipped and read to the model like the whole + // answer — the same failure the dropped-block marker exists to prevent. + if (block.text.length > left) cut = true; const text = block.text.length > left ? block.text.slice(0, left) : block.text; left -= text.length; out.push({ type: 'text', text }); } dropped += Math.max(0, blocks.length - RESULT_BLOCKS_MAX); - if (dropped > 0) { - out.push({ type: 'text', text: `[truncated: ${dropped} more content block(s) omitted]` }); + if (dropped > 0 || cut) { + console.warn(`[plugins] MCP tool result truncated: ${dropped} block(s) omitted${cut ? ', one shortened' : ''}`); + const omitted = dropped > 0 ? `${dropped} more content block(s) omitted` : 'the last block was cut short'; + out.push({ type: 'text', text: `[truncated: ${omitted}]` }); } return out; } @@ -213,6 +220,12 @@ function fitToBudget(blocks: Array<{ type: 'text'; text: string }>): Array<{ typ * Exported for the tests, which is where the shapes are enumerated. */ export function toMcpTextResult(raw: unknown): McpTextResult { + // Carried through every path below: a plugin that flagged an error and then + // shaped its content oddly still reported an error, and dropping the flag + // would present a failure to the model as a successful call. + const flagged = isTextResult(raw) && raw.isError === true; + const errorFlag = flagged ? { isError: true as const } : {}; + if (isTextResult(raw)) { const blocks = raw.content .filter((c): c is { type: 'text'; text: string } => !!c && (c as { type?: unknown }).type === 'text') @@ -220,15 +233,28 @@ export function toMcpTextResult(raw: unknown): McpTextResult { // A content array we cannot read is worse than none: fall through and // serialise the whole thing rather than emit an empty result. if (blocks.length) { - return { content: fitToBudget(blocks), ...(raw.isError === true ? { isError: true as const } : {}) }; + return { content: fitToBudget(blocks), ...errorFlag }; } } - if (typeof raw === 'string') return { content: fitToBudget([{ type: 'text', text: raw }]) }; + if (typeof raw === 'string') return { content: fitToBudget([{ type: 'text', text: raw }]), ...errorFlag }; + if (raw === undefined) { + // JSON.stringify(undefined) is undefined, which would otherwise reach the + // model as the empty string — indistinguishable from a tool that answered. + return { content: [{ type: 'text', text: '(the plugin returned no value)' }], ...errorFlag }; + } try { // Serialised with a budget rather than stringified whole and then sliced: a // plugin returning a 200 MB object should not cost the host that string // before the cap is applied. - return { content: fitToBudget([{ type: 'text', text: serialiseBounded(raw) }]) }; + const { text, cut } = serialiseBounded(raw); + const content = fitToBudget([{ type: 'text', text }]); + if (cut) { + // fitToBudget cannot see this one: the value was already trimmed on its + // way out of JSON.stringify, so what arrives here is inside the budget. + console.warn('[plugins] MCP tool result truncated while being serialised'); + content.push({ type: 'text', text: '[truncated: the result was cut to fit the size budget]' }); + } + return { content, ...errorFlag }; } catch { // JSON.stringify still throws on a cycle or a BigInt. return errorResult('The plugin returned a value that could not be serialised.'); @@ -242,16 +268,24 @@ export function toMcpTextResult(raw: unknown): McpTextResult { * being built instead of after. Throws exactly like JSON.stringify on a cycle * or a BigInt, so the caller's catch still handles those. */ -function serialiseBounded(value: unknown): string { +function serialiseBounded(value: unknown): { text: string; cut: boolean } { let budget = RESULT_MAX; + let cut = false; const text = JSON.stringify(value, (_key, v: unknown) => { - if (budget <= 0) return undefined; + if (budget <= 0) { + cut = true; + return undefined; + } if (typeof v === 'string') { budget -= v.length; - return budget <= 0 ? v.slice(0, Math.max(0, v.length + budget)) : v; + if (budget <= 0) { + cut = true; + return v.slice(0, Math.max(0, v.length + budget)); + } + return v; } budget -= 8; // rough cost of a number, boolean, or structural token return v; }, 2); - return text ?? ''; + return { text: text ?? '', cut }; } diff --git a/server/src/nest/plugins/mcp-tool-schema.ts b/server/src/nest/plugins/mcp-tool-schema.ts index fd3829467..0767bbe86 100644 --- a/server/src/nest/plugins/mcp-tool-schema.ts +++ b/server/src/nest/plugins/mcp-tool-schema.ts @@ -46,6 +46,31 @@ export const SCHEMA_PROPERTIES_MAX = 64; export const SCHEMA_ENUM_MAX = 32; export const SCHEMA_DEPTH_MAX = 5; +/** A `pattern` long enough to be interesting is long enough to be a mistake. */ +export const SCHEMA_PATTERN_MAX = 200; + +/** + * A property name is advertised model text, so it is held to the shape of an + * identifier rather than to a length. + * + * The caps and the newline collapse elsewhere in this file all sit on VALUES. + * A key is a JSON string like any other and may carry newlines, control + * characters and prose, which is the one place a plugin could still write + * `\n## SYSTEM\n…` into every user's context. `trek_session_token` with a + * plausible description is credential harvesting shaped like a schema. + */ +const PROP_NAME_RE = /^[A-Za-z_][A-Za-z0-9_.-]{0,63}$/; + +/** + * Names that survive the shape test but not the round trip. + * + * Zod drops `__proto__` while parsing, so a schema declaring it would advertise + * a field the handler can never receive — and one marked required would make + * the tool permanently uncallable. Refused rather than advertised, like every + * other declaration this file cannot honour. + */ +const RESERVED_PROP_NAMES = new Set(['__proto__', 'constructor', 'prototype']); + /** Raised for a declaration the host will not advertise. */ export class McpToolSchemaError extends Error {} @@ -84,17 +109,138 @@ interface Counters { properties: number; } -function sanitiseNode(node: unknown, depth: number, counters: Counters): unknown { - if (Array.isArray(node)) return node.map((item) => sanitiseNode(item, depth, counters)); - if (!isPlainObject(node)) return node; +/** + * One `enum` or `const` member, checked and bounded. + * + * Object members are refused rather than walked. Walking them would mean a + * `$ref` inside one reaching the advertisement through a subtree `nodeToZod` + * never visits, and the strings inside one never passing the sanitiser — which + * is the whole point of this file. `literalUnion` refuses them too, but only + * for the nodes it is asked to build, so the check has to live here as well. + */ +function checkedMember(member: unknown): unknown { + if (member !== null && typeof member === 'object') { + throw new McpToolSchemaError('enum and const members must be primitives'); + } + // Only the strings carry instructions; a number or boolean cannot. + return typeof member === 'string' ? sanitiseAssistantText(member, SCHEMA_ENUM_VALUE_MAX) : member; +} + +/** + * A `pattern` the host is willing to run against untrusted input. + * + * `RegExp` matching happens on the main thread, with no timeout and nothing to + * abort it, so `^(a+)+$` against 33 characters stops the whole server — not + * just MCP. That is an escape from the supervised child into the host event + * loop, and it does not take malice: a hand-written email pattern is the + * classic accident. + * + * A quantified group whose body itself quantifies or alternates is the shape + * that backtracks exponentially, so it is refused. `^[a-z0-9_]+$`, + * `^\d{4}-\d{2}-\d{2}$` and `^(draft|final)$` all survive; `(a+)+`, `(a*)*` and + * `(a|a)*` do not. Refused at parse time, like every other thing this file + * cannot make safe, rather than advertised and hoped about. + */ +function checkedPattern(value: unknown): string { + if (typeof value !== 'string') throw new McpToolSchemaError('pattern must be a string'); + if (value.length > SCHEMA_PATTERN_MAX) { + throw new McpToolSchemaError(`pattern is longer than ${SCHEMA_PATTERN_MAX} characters`); + } + try { + new RegExp(value); + } catch { + throw new McpToolSchemaError('pattern is not a valid regular expression'); + } + if (hasQuantifiedGroup(value)) { + throw new McpToolSchemaError( + 'pattern quantifies a group that itself repeats or alternates, which can backtrack exponentially', + ); + } + return value; +} + +/** True when some `(…)` that repeats or alternates inside is itself quantified. */ +function hasQuantifiedGroup(pattern: string): boolean { + const opens: number[] = []; + for (let i = 0; i < pattern.length; i += 1) { + const ch = pattern[i]; + if (ch === '\\') { + i += 1; + continue; + } + if (ch === '[') { + // Skip the class wholesale: its `|` and `+` are literals in there. + while (i < pattern.length && pattern[i] !== ']') i += pattern[i] === '\\' ? 2 : 1; + continue; + } + if (ch === '(') { + opens.push(i); + continue; + } + if (ch === ')' && opens.length) { + const start = opens.pop() as number; + const next = pattern[i + 1]; + const quantified = next === '*' || next === '+' || next === '{'; + if (quantified && /[*+{|]/.test(pattern.slice(start + 1, i))) return true; + } + } + return false; +} + +/** + * `default` and `examples`: an author's sample values, not schema. + * + * They reach the model with the same authority as a description, so their + * strings are bounded and stripped like one — but their KEYS are the author's + * own data and must not be read as keywords, so this walks separately from + * `sanitiseNode`. + */ +function sanitiseData(value: unknown, depth: number): unknown { if (depth > SCHEMA_DEPTH_MAX) { throw new McpToolSchemaError(`schema nests deeper than ${SCHEMA_DEPTH_MAX} levels`); } - const out: Record = {}; + if (typeof value === 'string') return sanitiseAssistantText(value, SCHEMA_DESCRIPTION_MAX); + if (Array.isArray(value)) return value.map((v) => sanitiseData(v, depth + 1)); + if (!isPlainObject(value)) return value; + const out: Record = Object.create(null) as Record; + for (const [k, v] of Object.entries(value)) { + out[sanitiseAssistantText(k, SCHEMA_ENUM_VALUE_MAX)] = sanitiseData(v, depth + 1); + } + return out; +} + +function sanitiseNode(node: unknown, depth: number, counters: Counters): unknown { + if (depth > SCHEMA_DEPTH_MAX) { + throw new McpToolSchemaError(`schema nests deeper than ${SCHEMA_DEPTH_MAX} levels`); + } + // Arrays count towards the depth like anything else: `[[[[…]]]]` recurses just + // as far, and only the byte cap stood between that and a blown stack. + if (Array.isArray(node)) return node.map((item) => sanitiseNode(item, depth + 1, counters)); + if (!isPlainObject(node)) return node; + const out: Record = Object.create(null) as Record; for (const [key, value] of Object.entries(node)) { if (FORBIDDEN_KEYS.includes(key)) { throw new McpToolSchemaError(`schema uses "${key}", which is not supported`); } + // The allowlist belongs here rather than only in nodeToZod, which walks the + // nodes it can type and skips whole subtrees: an additionalProperties + // subschema, anything below an enum or const, `properties` hung off a + // string. This walker sees every node, and `.meta()` advertises every node, + // so this is the only place the two can be made to agree. + if (!SUPPORTED_KEYWORDS.has(key) && !(depth === 0 && STRIPPED_ROOT_KEYS.includes(key))) { + throw new McpToolSchemaError(`schema uses unsupported keyword "${key}"`); + } + // Data, not schema nodes: an author's example object must not be walked as + // one, or its own keys would be read as keywords. Its STRINGS are advertised + // like any other, though, so they are bounded the same way. + if (key === 'default' || key === 'examples') { + out[key] = sanitiseData(value, depth); + continue; + } + if (key === 'pattern') { + out[key] = checkedPattern(value); + continue; + } if (key === 'description') { out[key] = sanitiseAssistantText(value, SCHEMA_DESCRIPTION_MAX); continue; @@ -103,21 +249,30 @@ function sanitiseNode(node: unknown, depth: number, counters: Counters): unknown out[key] = sanitiseAssistantText(value, SCHEMA_TITLE_MAX); continue; } - if (key === 'const' && typeof value === 'string') { - out[key] = sanitiseAssistantText(value, SCHEMA_ENUM_VALUE_MAX); + if (key === 'const') { + out[key] = checkedMember(value); continue; } - if (key === 'enum' && Array.isArray(value)) { + if (key === 'enum') { + if (!Array.isArray(value)) throw new McpToolSchemaError('enum must be an array'); if (value.length > SCHEMA_ENUM_MAX) { throw new McpToolSchemaError(`an enum lists more than ${SCHEMA_ENUM_MAX} values`); } - // Only the string members: a number or boolean carries no instructions. - out[key] = value.map((m) => (typeof m === 'string' ? sanitiseAssistantText(m, SCHEMA_ENUM_VALUE_MAX) : m)); + out[key] = value.map(checkedMember); continue; } if (key === 'properties' && isPlainObject(value)) { - const props: Record = {}; + // Null-prototype, so a property literally named `__proto__` lands as a key + // instead of silently reassigning the prototype and vanishing from both + // the advertisement and the validator. + const props: Record = Object.create(null) as Record; for (const [propName, propSchema] of Object.entries(value)) { + if (!PROP_NAME_RE.test(propName)) { + throw new McpToolSchemaError(`property name "${propName.slice(0, 32)}" is not a plain identifier`); + } + if (RESERVED_PROP_NAMES.has(propName)) { + throw new McpToolSchemaError(`property name "${propName}" is reserved`); + } counters.properties += 1; if (counters.properties > SCHEMA_PROPERTIES_MAX) { throw new McpToolSchemaError(`schema declares more than ${SCHEMA_PROPERTIES_MAX} properties`); @@ -149,6 +304,14 @@ export function normaliseToolSchema(raw: unknown): Record | und if (raw.type !== undefined && raw.type !== 'object') { throw new McpToolSchemaError(`schema root type must be "object", got ${JSON.stringify(raw.type)}`); } + // Each of these makes buildToolInputSchema return something that is not a Zod + // object: an enum or const pins the arguments to one literal, and `nullable` + // wraps the whole thing. The SDK's normalizeObjectSchema then cannot read it, + // so tools/list advertises an empty schema while the runtime still enforces + // the declaration — a tool nobody can call successfully. + for (const key of ['enum', 'const', 'nullable']) { + if (key in raw) throw new McpToolSchemaError(`schema root must not use "${key}"`); + } const bytes = Buffer.byteLength(JSON.stringify(raw) ?? '', 'utf8'); if (bytes > SCHEMA_BYTES_MAX) { @@ -180,14 +343,30 @@ const SUPPORTED_KEYWORDS = new Set([ const SUPPORTED_TYPES = new Set(['object', 'string', 'number', 'integer', 'boolean', 'array', 'null']); -/** Formats worth enforcing. An unlisted one would be advertised and never checked. */ -const FORMAT_CHECKS: Record boolean> = { - email: (v) => /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(v), - uuid: (v) => /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i.test(v), - uri: (v) => /^[a-z][a-z0-9+.-]*:/i.test(v), - 'date-time': (v) => !Number.isNaN(Date.parse(v)), - date: (v) => /^\d{4}-\d{2}-\d{2}$/.test(v), -}; +/** Keywords the branches below read with a `typeof` guard, and the type they need. */ +const NUMERIC_KEYWORDS: ReadonlyArray = [ + ['minimum', 'number'], ['maximum', 'number'], + ['exclusiveMinimum', 'number'], ['exclusiveMaximum', 'number'], + ['multipleOf', 'number'], ['minLength', 'number'], ['maxLength', 'number'], + ['minItems', 'number'], ['maxItems', 'number'], + ['pattern', 'string'], ['format', 'string'], + ['uniqueItems', 'boolean'], ['nullable', 'boolean'], +]; + +/** + * Formats worth enforcing. An unlisted one would be advertised and never checked. + * + * A Map rather than an object literal: on a literal, `format: "constructor"` + * finds `Object.prototype.constructor`, passes the allowlist gate, and either + * advertises a format nothing checks or throws inside the caller's validation. + */ +const FORMAT_CHECKS = new Map boolean>([ + ['email', (v) => /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(v)], + ['uuid', (v) => /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i.test(v)], + ['uri', (v) => /^[a-z][a-z0-9+.-]*:/i.test(v)], + ['date-time', (v) => !Number.isNaN(Date.parse(v))], + ['date', (v) => /^\d{4}-\d{2}-\d{2}$/.test(v)], +]); /** The `required` names a normalised schema declares. */ function requiredKeys(schema: Record | undefined): string[] { @@ -226,8 +405,27 @@ function nodeToZod(node: unknown, path: string): z.ZodType { } } - // An enum or const pins the value set outright, whatever `type` says. - // + // A keyword whose value is the wrong type is dropped by the branches below and + // advertised all the same — `exclusiveMinimum: true`, the draft-4 spelling many + // generators still emit, would vanish from the check and stay in the + // advertisement. Same failure as an unsupported keyword, so same answer. + for (const [key, expected] of NUMERIC_KEYWORDS) { + if (node[key] !== undefined && typeof node[key] !== expected) { + throw new McpToolSchemaError(`${path}: "${key}" must be a ${expected}`); + } + } + + // An enum or const pins the value set outright, whatever `type` says — and + // returns before the children below are built, so a sibling `properties` or + // `items` would be advertised and enforced by nothing. + if (node.enum !== undefined || node.const !== undefined) { + for (const key of ['properties', 'items', 'additionalProperties']) { + if (node[key] !== undefined) { + throw new McpToolSchemaError(`${path}: "${key}" cannot sit beside an enum or const`); + } + } + } + // Built from real literal types rather than a predicate: z.custom() cannot be // represented in JSON Schema, and the SDK renders tools/list by walking this // schema, so a predicate here throws for the whole session the first time a @@ -264,7 +462,7 @@ function nodeToZod(node: unknown, path: string): z.ZodType { sc = sc.regex(re); } if (typeof node.format === 'string') { - const check = FORMAT_CHECKS[node.format]; + const check = FORMAT_CHECKS.get(node.format); if (!check) throw new McpToolSchemaError(`${path}: unsupported format "${node.format}"`); out = sc.refine(check, { message: `must be a valid ${node.format}` }); break; @@ -301,9 +499,16 @@ function nodeToZod(node: unknown, path: string): z.ZodType { break; } default: { + // Only the two booleans, because only they are enforceable here. A + // subschema would be advertised as a constraint on the extra properties + // and then checked by nothing, which is the exact drift this file exists + // to prevent. + if (node.additionalProperties !== undefined && typeof node.additionalProperties !== 'boolean') { + throw new McpToolSchemaError(`${path}: additionalProperties must be true or false`); + } const props = isPlainObject(node.properties) ? node.properties : {}; const required = new Set(requiredKeys(node)); - const shape: Record = {}; + const shape: Record = Object.create(null) as Record; for (const [name, child] of Object.entries(props)) { // Every field is optional in the SHAPE, with presence enforced by the // check below. A required field declared non-optional here would fail @@ -318,7 +523,10 @@ function nodeToZod(node: unknown, path: string): z.ZodType { out = required.size ? base.check((ctx) => { for (const name of required) { - if (!(name in (ctx.value as Record))) { + // hasOwnProperty, not `in`: `in` walks the prototype chain, so a + // required field named `toString` would read as present on every + // call and never be asked for. + if (!Object.prototype.hasOwnProperty.call(ctx.value as object, name)) { ctx.issues.push({ code: 'custom', input: ctx.value, @@ -393,6 +601,12 @@ function isReadOnlyGrant(g: string): boolean { // Host-to-plugin entry points. These grant the plugin nothing: they are the // host's permission to CALL it. A photo provider is not a writer. if (g.startsWith('hook:')) return true; + // The same thing, spelled differently. mcp:tools is the hook permission for + // mcpToolProvider (PLUGIN_HOOK_PERMISSION in shared/src/plugin-permissions.ts) + // and is simply not hook:*-shaped. Counting it as a side effect makes + // readOnlyHint unreachable for EVERY plugin tool, since holding it is what + // lets a plugin advertise one at all. + if (g === 'mcp:tools') return true; // Receiving events is a read; acting on one needs a separate grant. if (g === 'events:subscribe') return true; return false; diff --git a/server/tests/unit/plugins/mcp-tool-annotations.test.ts b/server/tests/unit/plugins/mcp-tool-annotations.test.ts index 61f010c24..ea1af9b46 100644 --- a/server/tests/unit/plugins/mcp-tool-annotations.test.ts +++ b/server/tests/unit/plugins/mcp-tool-annotations.test.ts @@ -28,6 +28,10 @@ const EXPECTED_READ_ONLY = new Set([ 'geolocation:read', 'rates:read', 'weather:read', + // The hook permission for mcpToolProvider, just not hook:*-shaped. Every + // plugin that can advertise a tool holds it, so classifying it as a side + // effect would make readOnlyHint unreachable for all of them. + 'mcp:tools', ...KNOWN_PERMISSIONS.filter((p) => p.startsWith('db:read:')), ...KNOWN_PERMISSIONS.filter((p) => p.startsWith('hook:')), ]); @@ -57,6 +61,16 @@ describe('grant classification', () => { expect(readOnly('some:future:grant')).toBe(false); }); + it('MCPANN-006: a read-only plugin keeps the hint despite the grant that lets it advertise', () => { + // Regression: mcp:tools is the permission every publishing plugin holds, so + // counting it as a side effect put readOnlyHint out of reach for all of them + // and cost every legitimate read tool its auto-approval. + expect(readOnly('mcp:tools')).toBe(true); + const out = clampToolAnnotations({ readOnlyHint: true }, new Set(['mcp:tools', 'weather:read'])); + expect(out.readOnlyHint).toBe(true); + expect(out.destructiveHint).toBe(false); + }); + it('MCPANN-005: a per-host egress grant is a side effect and forces openWorldHint', () => { const out = clampToolAnnotations({ readOnlyHint: true, openWorldHint: false }, new Set(['http:outbound:api.example.com'])); expect(out.readOnlyHint).toBe(false); diff --git a/server/tests/unit/plugins/mcp-tool-schema.test.ts b/server/tests/unit/plugins/mcp-tool-schema.test.ts index daaf8d8520180d95a7e6b44734e2bde5ba27c393..f7a4ed4882fb47a494fba4f13cb429fa47f798a9 100644 GIT binary patch delta 6501 zcmb_h&u<&Y6^3Bc4P+#7Y+16c)EUuANLmynTgng9QC$VKf&8>nIRO%h6^GoBxaD$Z zJ-aK4QB)>9_mu6iKznI{0zCu>3iOy$uTB4hHus`wuL072Z+wBJ3#ZMup9OQzJvEwKx^``nU1JZr zg2ib&Q9{J56-rM$knc;c#X_+s6dUlM7uT_n*1npsWEf#xf=#rP`3du5w(IYSknzxC zanFxpHt5Pgu(sl1r;F8Je)KCmBm*t(d4A%{kdK1*=%(Q=S(H0FjI%gV*ivV$n1`?*i?`rhi|zTbpTvv@0oIYz zeeGnc-WQ{USE?<>JsgrY=vBs`URYzX9Q#7&O$L&Ae$00fDaP9}O_&l~ACMmR@y39q zbmC4=coedwNl(Nb<+nwpOiS`Gk@EY9=AMX`vMOMqO9d7=|= zrGu;ijLZG8)Bx64N2ue0P%-OiguU zzb`@rqr4SfWgG8pJh<~__WtCRQ}wMdJ2ms-$gHnc$KgoBIE^By5}ec>zO*%OEtQt4 zx7-Qd2|ZlA1G#p)h6wnbZs7T zMq5x?dW^>|tnn@L4j0dN*WY;O&W1@|jUkh|6g&NA%=_*QeljFO&u~D9GC3mdD#p>U z7I091T$?9^kh$zrR(Z;vAE+1z4!2eI=H$Yw^>Qs=%9x9a&PE&jGM$4v_Zg)D6063_ zQNaChE3|vEpU+iK;kGk5fAJa3oMDt9s(mEa@2);icdaC{DNKWaw-LdC_A8H@-q??j<|dOhmxypjst&~g zOcG?#`ODeYQ&(q9HAaPT#%xeOzH+{Pyxi&6=B$JZCNLN$G(~Q#Ah3wE3>VuHatS9m zsah8h$do1#1X{1P!o`E;unv1R)RlFR#Yx6aj05pz(|nSe{t*r!Xmy)wR(BR6*ZW>|u_AV6*D0cJRJ2SP4;^83?t8$Zf# z!$RtS@dmpN`N9s1-kop_w4szMn$-h?7@&^i%XjS6QNreL&R+aMh&lmL67eJvDn#i* zf`CmENg&Q9Zd@6?KsRg&cF1TwT_;Wg$cBl(;|pbI1*-xM#DM|^3bmLv#p1Y_xo;G( zv1N)SaS3SCQY$_JA9xA8>W&C1z`hnyePI-0C!Pk|B~mDcR*)8Jq92lKg{(QdMtTT}s33{1YORWsD$(19?2b`Tw-ea|fl!^> z3d`Bw&Ydqv%{Mlkz2?dAr8~0)b*!#T?J(6~9PIqkt&ANJ5!IE5P@XFLkXX;K0S*P8EyOCBM+-i2ZhP=}_BE3#Lp?;6^79;Q5~Y?kd?%FXQU+1b&`iyEm0$p$r)aBbe{B?|vRMU4P5 zU=}He<7h-3>3(>EW2xhaZT+;u7pe>MP<^|ow-9M7E2tS9R*^NpDZXPP12dnntHm4m zm-YQHO+;L$4i%!%j~!$~$r8UWNau1zB9Z+>?B3arY-t7fyO116B`pO_FM%|OxB>~& zP7{23N5x#gZJWc?C2p3j$2bdpyjHz7Z$Ey>?dQt=!afc)EH|sV(a64P7&wCR(GS9{B|2v2$egViPR=v{9t+U=5}+z#As{F zJ6Jtj#IIKmZs_0mWy7cC(uX@ph@fh|qTS7f%A)yQy*=;ZNuai=q2;a14HMsy^TEVd z=Yu5x=GYcf1xlcY^q)M2MpcYeaGp6N!%q^#59@Ys&hm73lhDQX`>ZiNH(I%QjOIPI zy{${Z_V%L+cnm!kCJ25Y5jFt6+hwn9+X&VkRU@1M(_e_9UMX=Q~TDTGD)U&0F4R< zZ%|JM(CHF&#EB7>qLQLy8TAayT0gg}%X+|M<#Vd`{5>n6TS`1keI+pJqRNdoIuEt( z$R#3LNs6u>)u0>X2>Y{ zFeXFaU`fj`91O6y%h7k}+igp9q`4nC|B<&{8Ojv&39};_uXRNT`Cuj|wIf0?f;q(` z)S|fvlUH^kf-2(%Flz-oRJ8Aml@Cf3|5Br5uv4m)#z8GL4oU==x1o5px9Kmt)}&eK zuvU`26TZgxF7l#o@7&A&iZM2&PU)+&x5SO-Gu&K7Dqy zx=h@GGxSlRXo8hSR#CBl?irY|^?ZDWqa&`u`K_6gB9O=p-Fj0rP#y0L!uup8pH<0_ zzVf5J7hG({c4nt=tH`WgD2v#&AiYnq3gK=tq~`}f3!8^vy+W=~kCe8usXF}Orz zdUKT~p8&E+O`F&hTx6&{gVxZlbnzI&Rx(e$&tnCTFK54ccw3}Ic6jdMCCxgfCR7fJZ40vJLhZ=YJaXe62TYY?loa6N zD#o!P>IZ^}3c`n&8$w#)<#NE4XXYZFh(Jgk9$_XoJ<`kUke|5$h7ujn5=hPs)653goyIcV;ogRJH1oRhL>JGy-ClSg`VQ<__FB{#++ZDnysDCLeNt%Cw1F~Ybb5$ G-~R#dM}y@6 delta 27 jcmaF6l5tY%h6CJe3`sF1$?}^oa4!{QWY}D-s3r&inZgN^