Skip to content

Commit 8ccb47d

Browse files
ndemiancclaude
andcommitted
fix(mcp): cap the fallback tool description too (PR #31 review)
describeTool capped the server's own description but returned the no-description fallback uncapped. That fallback embeds `tool` — the server-chosen, untrusted tool name — so a server could ship a giant name with a blank description and sail past MAX_TOOL_DESC, re-bloating every turn's prompt and widening the G4 injection surface the cap exists to bound. Fix applies the cap to the final string, so the server's text and the fallback obey the same bound through one exit — which also drops the duplicated slice logic. (The namespaced name is separately truncated to 64; untouched.) Verified: 39 tests (1 new — a 6000-char tool name with no description must still yield a description within MAX_TOOL_DESC). Mutation-checked: restoring the uncapped-fallback branch fails the new test. Full gate: 18 suites, 0 failures. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 95014cc commit 8ccb47d

2 files changed

Lines changed: 20 additions & 3 deletions

File tree

‎extensions/levelcode-ai/mcpConfig.js‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -268,11 +268,17 @@ function schemaOf(raw) {
268268
return out;
269269
}
270270

271-
/** Capped description, with a fallback — the model needs *something* to decide with. */
271+
/**
272+
* A description the model can decide on, ALWAYS within MAX_TOOL_DESC. The cap is applied to the final
273+
* string — the server's own text AND the fallback — because the fallback embeds `tool`, which is the
274+
* server-chosen (untrusted) tool name: a server could send a giant name with a blank description and,
275+
* if only the real-description branch were capped, blow past the bound anyway (PR #31 review). One cap
276+
* at the exit covers every branch.
277+
*/
272278
function describeTool(spec, server, tool) {
273279
const raw = typeof spec.description === 'string' ? spec.description.trim() : '';
274-
if (!raw) { return 'The "' + tool + '" tool from the "' + server + '" MCP server (no description provided).'; }
275-
return raw.length > MAX_TOOL_DESC ? raw.slice(0, MAX_TOOL_DESC - 1) + '…' : raw;
280+
const desc = raw || ('The "' + tool + '" tool from the "' + server + '" MCP server (no description provided).');
281+
return desc.length > MAX_TOOL_DESC ? desc.slice(0, MAX_TOOL_DESC - 1) + '…' : desc;
276282
}
277283

278284
/**

‎extensions/levelcode-ai/test/mcpConfig.test.js‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,17 @@ test('BUILD: a description is capped, and a missing one still says something use
386386
}
387387
});
388388

389+
test('BUILD: the FALLBACK description is capped too — a giant tool name cannot defeat MAX_TOOL_DESC', () => {
390+
// PR #31 review: the no-description fallback embeds `tool`, which is the SERVER-chosen (untrusted)
391+
// tool name. A server could ship a huge name with a blank description to bloat every turn's prompt
392+
// past the bound. Capping only the real-description branch would miss it — the cap must cover the
393+
// fallback too. (The namespaced `name` is separately truncated to 64; this is about the DESCRIPTION.)
394+
const b = M.buildAgentTools([SRV('s', [{ name: 'z'.repeat(6000) }])]);
395+
assert.strictEqual(b.tools.length, 1);
396+
assert.ok(b.tools[0].description.length <= M.MAX_TOOL_DESC, 'fallback must obey MAX_TOOL_DESC');
397+
assert.ok(b.tools[0].name.length <= M.MAX_TOOL_NAME, 'and the name still stays legal');
398+
});
399+
389400
test('BUILD: annotations ride along so the policy can tighten on them', () => {
390401
const b = M.buildAgentTools([SRV('s', [{ name: 'rm', annotations: { destructiveHint: true } }])]);
391402
const route = b.routes.get('s__rm');

0 commit comments

Comments
 (0)