From 25a7dbe4019ca45c465aa93ae1151936db7f47e1 Mon Sep 17 00:00:00 2001 From: Jonathan-ai9 Date: Sat, 17 Oct 2026 21:33:29 +0100 Subject: [PATCH] fix(mcp): bound tool results and cap upstream reads (#416) A tool result is written straight into the agent's context window, so an unbounded one is self-inflicted denial of service whose failure gets blamed on the model. All four tools serialise compactly and are capped at MCP_MAX_RESULT_BYTES; a truncated result carries truncated, the total, the page returned, and a notice saying how to ask for more. bazaar_search gains limit/offset arguments, forwards them to the node and pages whatever comes back. lens_get_price keeps structuredContent only while the payload fits, since shipping it regardless would move the blow-up to another field. Upstream bodies are read through a capped stream reader that cancels at the cap, so an oversized body is never buffered; a discovery body too large to parse returns ERR_RESPONSE_TOO_LARGE, which is deliberately not one of the two pre-request refusals: we did fetch it, we will not dump it. Rebased onto main after #480 (structured paid-call errors), #487 (paid_call host allow-list) and #492 (spend ceiling) landed: the bounds now sit on top of those gates rather than beside them, and limit/offset go through the Zod tool schemas. --- .changeset/mcp-bounded-tool-results.md | 23 ++ .env.example | 15 ++ src/config.ts | 23 ++ src/mcp/codes.ts | 6 + src/mcp/limits.ts | 257 ++++++++++++++++++ src/mcp/server.ts | 139 ++++++++-- src/mcp/toolSchemas.ts | 18 ++ tests/mcp-bounded-results.test.ts | 353 +++++++++++++++++++++++++ tests/mcp.test.ts | 4 +- 9 files changed, 811 insertions(+), 27 deletions(-) create mode 100644 .changeset/mcp-bounded-tool-results.md create mode 100644 src/mcp/limits.ts create mode 100644 tests/mcp-bounded-results.test.ts diff --git a/.changeset/mcp-bounded-tool-results.md b/.changeset/mcp-bounded-tool-results.md new file mode 100644 index 00000000..80eda08a --- /dev/null +++ b/.changeset/mcp-bounded-tool-results.md @@ -0,0 +1,23 @@ +--- +'lens': patch +--- + +MCP tool results are size-bounded (#416). Every result was serialised +pretty-printed and in full, so one large — or hostile — response could consume an +agent's entire context window, and the failure looked like a bad model rather +than an unbounded tool. All four tools (`bazaar_search`, `paid_call`, +`payment_budget`, `lens_get_price`) now return compact JSON capped at +`MCP_MAX_RESULT_BYTES`, and a truncated result says so: `truncated: true`, the +total, the page returned, and a notice explaining how to ask for more. +`bazaar_search` gains `limit` / `offset` arguments (defaults and ceiling from +`MCP_DEFAULT_PAGE_LIMIT` / `MCP_MAX_PAGE_LIMIT`), forwards them to the node, and +pages whatever comes back even if the node ignores them. `lens_get_price` keeps +`structuredContent` only while the payload fits, since shipping it regardless +would move the blow-up into another field. + +Upstream bodies are read through a capped stream reader (`MCP_MAX_UPSTREAM_BYTES`) +that keeps the part that fits and cancels the rest, so an oversized body is never +buffered in full. A discovery body too large to parse returns +`ERR_RESPONSE_TOO_LARGE`, which — unlike `ERR_URL_BLOCKED` and +`ERR_HOST_NOT_ALLOWED` — is not a refusal to ask: the request succeeded, the +answer is simply too big to hand over. diff --git a/.env.example b/.env.example index bd71dc4b..6630e926 100644 --- a/.env.example +++ b/.env.example @@ -42,6 +42,21 @@ HTTP_TIMEOUT_MS=10000 # Example: OUTBOUND_ALLOWED_HOSTS=bazaar.example.com,lens.internal.io OUTBOUND_ALLOWED_HOSTS= +# --- MCP tool result bounds --- +# Tool results are written straight into an agent's context window, so they are +# size-bounded: an unbounded result is a self-inflicted denial of service whose +# failure is misread as a bad model. Truncated results say so in the payload. +# Maximum size, in bytes, of one MCP tool result (default: 16384; 1024..1048576) +MCP_MAX_RESULT_BYTES=16384 +# Maximum bytes read from a single upstream response body before the stream is +# cancelled, so an oversized body is never buffered in full +# (default: 262144; 4096..8388608) +MCP_MAX_UPSTREAM_BYTES=262144 +# Page size requested of a Bazaar node when the caller gives no limit, and the +# most a caller may ask for (defaults: 50 and 200, matching /discovery) +MCP_DEFAULT_PAGE_LIMIT=50 +MCP_MAX_PAGE_LIMIT=200 + # --- Active Network --- # Which network the node operates on: "testnet" or "mainnet" (default: testnet) # This drives all config.horizon / config.rpc / config.soroswap shortcut paths. diff --git a/src/config.ts b/src/config.ts index 35c54518..967d4367 100644 --- a/src/config.ts +++ b/src/config.ts @@ -84,6 +84,17 @@ function parseAllowedHosts(raw: string): string[] { return Array.from(seen) } +/** + * Parse a positive integer env var, clamped into [min, max]. Unset, unparsable + * or out-of-range values fall back to `fallback` rather than silently removing + * a bound: an operator typo should not turn a limit off altogether. + */ +function clampInt(raw: string | undefined, fallback: number, min: number, max: number): number { + const parsed = parseInt(raw ?? '', 10) + if (!Number.isFinite(parsed) || parsed <= 0) return fallback + return Math.min(Math.max(parsed, min), max) +} + const MAINNET_DEFAULT_PAIRS = 'USDC:GA5ZSEJYB37JRC5AVCIA5MOP4RHTM335X2KGX3IHOJAPP5RE34K4KZVN/XLM' @@ -286,6 +297,17 @@ const globalConfig = { // block is unconditional (see net/ssrf.ts). allowedHosts: parseAllowedHosts(process.env.OUTBOUND_ALLOWED_HOSTS ?? ''), }, + mcp: { + // Tool results land directly in an agent's context window, so both the body + // read from a node and the text handed back are capped (#416). + maxResultBytes: clampInt(process.env.MCP_MAX_RESULT_BYTES, 16_384, 1_024, 1_048_576), + maxUpstreamBytes: clampInt(process.env.MCP_MAX_UPSTREAM_BYTES, 262_144, 4_096, 8_388_608), + // Page size for collection results, mirroring the /discovery limits: a + // request without an explicit limit gets this many items, and an over-large + // ask is clamped rather than honoured. + defaultPageLimit: clampInt(process.env.MCP_DEFAULT_PAGE_LIMIT, 50, 1, 200), + maxPageLimit: clampInt(process.env.MCP_MAX_PAGE_LIMIT, 200, 1, 1000), + }, } as const // ── Per-network map ─────────────────────────────────────────────────────────── @@ -367,4 +389,5 @@ export const config = { cache: globalConfig.cache, http: globalConfig.http, outbound: globalConfig.outbound, + mcp: globalConfig.mcp, } diff --git a/src/mcp/codes.ts b/src/mcp/codes.ts index ca265a2d..caf9261d 100644 --- a/src/mcp/codes.ts +++ b/src/mcp/codes.ts @@ -10,6 +10,12 @@ export const MCP_ERROR_CODES = { ERR_PAIR_NOT_WATCHED: 'ERR_PAIR_NOT_WATCHED', ERR_ASSET_ISSUER_MISMATCH: 'ERR_ASSET_ISSUER_MISMATCH', ERR_BUDGET_EXCEEDED: 'ERR_BUDGET_EXCEEDED', + // A request that succeeded upstream and was still refused: the body came back + // too large to hand to an agent without consuming its whole context. Not a + // reachability answer (ERR_SERVICE_UNREACHABLE) and not a refusal to ask + // (ERR_URL_BLOCKED / ERR_HOST_NOT_ALLOWED) — we did fetch it, we will not dump + // it. Carries the cap in `details` so the caller can retry with a smaller page. + ERR_RESPONSE_TOO_LARGE: 'ERR_RESPONSE_TOO_LARGE', // Refused before any request was made, for two different reasons: // ERR_URL_BLOCKED the target resolves somewhere we will never fetch // (private, loopback, link-local, metadata) — #488 diff --git a/src/mcp/limits.ts b/src/mcp/limits.ts new file mode 100644 index 00000000..4b63d6f4 --- /dev/null +++ b/src/mcp/limits.ts @@ -0,0 +1,257 @@ +/** + * Size bounds for MCP tool results (#416). + * + * A tool result is written straight into the agent's context window, so an + * unbounded one is self-inflicted denial of service: a node with a large + * resource list — or a hostile one returning megabytes — leaves the agent no + * context to do its actual task, and the failure is blamed on the model. + * Everything here exists to keep a result small *and* to say so in the result: + * silent truncation would make a partial answer look like a complete one. + * + * Two independent ceilings: + * - the body read from upstream is capped while streaming, so an oversized + * response is never buffered in full (`readBodyCapped`); + * - the text handed to the agent is capped, and collections are additionally + * capped by item count and reported as a page with a total + * (`boundSerialized`). + */ + +/** Keys that hold a page's items when the payload is an object. */ +const COLLECTION_KEYS = ['resources', 'results', 'items'] + +/** + * Operator-configurable bounds, resolved from the `MCP_*` env in config.ts and + * overridable per server for tests. + */ +export interface McpLimits { + maxResultBytes: number + maxUpstreamBytes: number + defaultPageLimit: number + maxPageLimit: number +} + +export interface ResultBudget { + /** Maximum size, in UTF-8 bytes, of the text handed to the agent. */ + maxBytes: number + /** Maximum number of collection items echoed, whatever the byte headroom is. */ + maxItems: number + /** The page offset the result came from, echoed back so the agent can continue. */ + offset: number + /** How the agent asks for more. Included verbatim in every truncated result. */ + notice: string + /** + * True when the payload is already known to be incomplete — the body read was + * cut off upstream. Labels the result truncated even if what survives fits. + */ + knownTruncated?: boolean +} + +export interface BoundedResult { + text: string + truncated: boolean +} + +export interface CappedBody { + text: string + /** True when the upstream body was larger than the cap, so `text` is incomplete. */ + exceeded: boolean +} + +function byteLength(text: string): number { + return Buffer.byteLength(text, 'utf8') +} + +/** Cut `text` down to at most `maxBytes` without splitting a UTF-8 sequence. */ +function sliceToBytes(text: string, maxBytes: number): string { + if (byteLength(text) <= maxBytes) return text + const bytes = Buffer.from(text, 'utf8').subarray(0, maxBytes) + return new TextDecoder('utf-8').decode(bytes) +} + +interface Collection { + items: unknown[] + /** Item count as reported by the payload itself, when it knows better than us. */ + total: number + key: string | null + container: Record | null +} + +/** + * Find the collection to page through, if any: a top-level array, or an object + * with an array under a known collection key (`resources`, `results`, `items`) + * or, failing that, its first array-valued key. + */ +function findCollection(value: unknown): Collection | null { + if (Array.isArray(value)) { + return { items: value, total: value.length, key: null, container: null } + } + if (typeof value !== 'object' || value === null) return null + + const container = value as Record + const keys = Object.keys(container).filter((k) => Array.isArray(container[k])) + const key = COLLECTION_KEYS.find((k) => keys.includes(k)) ?? keys[0] + if (!key) return null + + const items = container[key] as unknown[] + const reportedTotal = typeof container.total === 'number' ? container.total : items.length + return { items, total: Math.max(reportedTotal, items.length), key, container } +} + +function assemblePage(collection: Collection, count: number, budget: ResultBudget): string { + const page = collection.items.slice(0, count) + const metadata = { + total: collection.total, + limit: count, + offset: budget.offset, + truncated: true, + notice: budget.notice, + } + if (collection.container && collection.key) { + return JSON.stringify({ ...collection.container, [collection.key]: page, ...metadata }) + } + return JSON.stringify({ items: page, ...metadata }) +} + +/** Largest page that fits the byte budget, or null when not even metadata fits. */ +function boundCollection(collection: Collection, budget: ResultBudget): BoundedResult | null { + const max = Math.min(collection.items.length, budget.maxItems) + let lo = 0 + let hi = max + while (lo < hi) { + const mid = Math.ceil((lo + hi) / 2) + if (byteLength(assemblePage(collection, mid, budget)) <= budget.maxBytes) lo = mid + else hi = mid - 1 + } + + const text = assemblePage(collection, lo, budget) + if (byteLength(text) > budget.maxBytes) return null + return { text, truncated: true } +} + +/** + * Last resort for payloads with no pageable collection (or whose items are + * individually enormous): keep a prefix of the compact serialization and label + * it, rather than returning a document that only parses by accident. + */ +function boundSerializedText(value: unknown, budget: ResultBudget): BoundedResult { + const compact = JSON.stringify(value) ?? 'null' + const body = typeof value === 'string' ? value : compact + const wrap = (chars: number) => + JSON.stringify({ + truncated: true, + fullBytes: byteLength(compact), + notice: budget.notice, + partial: body.slice(0, chars), + }) + + let lo = 0 + let hi = body.length + while (lo < hi) { + const mid = Math.ceil((lo + hi) / 2) + if (byteLength(wrap(mid)) <= budget.maxBytes) lo = mid + else hi = mid - 1 + } + + const text = wrap(lo) + return { text: sliceToBytes(text, budget.maxBytes), truncated: true } +} + +/** + * Serialize a tool result compactly (indentation is for humans, and costs + * context) and within `budget.maxBytes`. A truncated result always carries + * `truncated: true`, a total, the page that was returned, and the notice + * explaining how to ask for the rest. + */ +export function boundSerialized(value: unknown, budget: ResultBudget): BoundedResult { + if (budget.knownTruncated) { + // Already a fragment — the upstream read was cut off. Label it instead of + // risking that it reads back as a complete answer. + return boundSerializedText(value, budget) + } + + const compact = JSON.stringify(value) ?? 'null' + if (typeof value === 'string' && byteLength(value) <= budget.maxBytes) { + // Plain-text answers stay plain text: quoting them would change what the + // agent reads for a resource that never produced JSON. + return { text: value, truncated: false } + } + + const collection = findCollection(value) + if (byteLength(compact) <= budget.maxBytes && (!collection || collection.items.length <= budget.maxItems)) { + return { text: compact, truncated: false } + } + + if (collection) { + const paged = boundCollection(collection, budget) + if (paged) return paged + } + return boundSerializedText(value, budget) +} + +interface BodyReader { + read(): Promise<{ done?: boolean; value?: unknown }> + cancel(): Promise + releaseLock(): void +} + +interface ReadableBody { + getReader(): BodyReader +} + +function bodyStream(res: Response): ReadableBody | null { + const body = (res as { body?: unknown }).body + if (!body || typeof (body as ReadableBody).getReader !== 'function') return null + return body as ReadableBody +} + +/** + * Read a response body up to `maxBytes`, stopping the read (and cancelling the + * stream, which closes the connection) as soon as the cap is passed. A hostile + * server therefore never gets its whole body buffered here. + * + * Responses that expose no readable stream — test doubles, or a transport that + * has already buffered the body — fall back to `res.text()` and report the + * overflow; the cap is still honoured, only after the damage is done. + */ +export async function readBodyCapped(res: Response, maxBytes: number): Promise { + const stream = bodyStream(res) + if (!stream) { + const text = await res.text() + if (byteLength(text) <= maxBytes) return { text, exceeded: false } + return { text: sliceToBytes(text, maxBytes), exceeded: true } + } + + const reader = stream.getReader() + const decoder = new TextDecoder('utf-8') + let received = 0 + let text = '' + try { + for (;;) { + const { done, value } = await reader.read() + if (done) break + + const chunk = value as Uint8Array | undefined + if (!chunk || chunk.byteLength === 0) continue + + received += chunk.byteLength + if (received > maxBytes) { + // Keep the part that fits, drop the rest, and stop reading — the point + // is that the overshoot never lands in memory at all. + const kept = chunk.subarray(0, chunk.byteLength - (received - maxBytes)) + text += decoder.decode(kept) + await reader.cancel().catch(() => undefined) + return { text, exceeded: true } + } + text += decoder.decode(chunk, { stream: true }) + } + text += decoder.decode() + } finally { + try { + reader.releaseLock() + } catch { + // already released by cancel() + } + } + + return { text, exceeded: false } +} diff --git a/src/mcp/server.ts b/src/mcp/server.ts index bd918b00..5d4adfe0 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -11,6 +11,7 @@ import { zodToToolInputSchema, formatZodError, } from './toolSchemas.js'; +import { boundSerialized, readBodyCapped, type McpLimits, type ResultBudget } from './limits.js'; import { fetchWithTimeout } from '../fetch.js'; import { config } from '../config.js'; import { ssrfSafeFetch, SsrfError, type SsrfFetchOptions } from '../net/ssrf.js'; @@ -90,6 +91,12 @@ export interface McpServerConfig { * tests can drive DNS resolution and the transport without real sockets. */ bazaarFetch?: Partial; + /** + * Bounds on what a tool result may cost the agent in context (#416), from the + * `MCP_*` env by default. Overridable so a test can drive the limits without + * touching process state. + */ + limits?: Partial; } export class BazaarMcpServer { @@ -99,11 +106,13 @@ export class BazaarMcpServer { private bazaarFetchOptions: SsrfFetchOptions; private priceQueries?: LensPriceQueries; private budget?: SpendBudget; + private limits: McpLimits; constructor(config_: McpServerConfig) { this.fetchWithPayment = config_.fetchWithPayment; this.priceQueries = config_.priceQueries; this.budget = config_.budget; + this.limits = { ...config.mcp, ...config_.limits }; // Two independent gates, deliberately separate: this one decides which // hosts may be PAID (#404), the one below decides which addresses may be @@ -140,7 +149,9 @@ export class BazaarMcpServer { const tools: Record[] = [ { name: 'bazaar_search', - description: 'Search for resources in a Stellar Bazaar node', + description: + 'Search for resources in a Stellar Bazaar node. The result is a bounded page: ' + + 'check "truncated" and "total", and re-ask with limit and offset to page through the rest.', inputSchema: zodToToolInputSchema(BAZAAR_SEARCH_ARGUMENTS_SCHEMA), }, { @@ -196,11 +207,19 @@ export class BazaarMcpServer { } // structuredContent is what the agent reads; the text block keeps the - // same payload visible to clients that only render content. - return { - content: [{ type: 'text', text: JSON.stringify(outcome.result, null, 2) }], - structuredContent: outcome.result, - }; + // same payload visible to clients that only render content. When the + // payload does not fit the result limit the text carries the label and + // structuredContent is dropped — shipping it whole would keep the very + // context blow-up #416 is about, just in a different field. + const bounded = boundSerialized(outcome.result, { + maxBytes: this.limits.maxResultBytes, + maxItems: Number.MAX_SAFE_INTEGER, + offset: 0, + notice: `A Lens price was truncated to fit a ${this.limits.maxResultBytes}-byte tool output limit; request fewer venues or a narrower pair.`, + }); + return bounded.truncated + ? { content: [{ type: 'text', text: bounded.text }] } + : { content: [{ type: 'text', text: bounded.text }], structuredContent: outcome.result }; } catch (e: any) { // Prices come from Postgres and Horizon, so a throw here is the data // layer being unreachable — never an answer about the pair. @@ -215,12 +234,18 @@ export class BazaarMcpServer { } const { url, query } = parsed.data; - + const page = this.resolvePage(parsed.data); + try { - const endpoint = query - ? `${url.replace(/\/$/, '')}/discovery/search?q=${encodeURIComponent(query)}` - : `${url.replace(/\/$/, '')}/discovery/resources`; - + const search = new URLSearchParams(); + if (query) search.set('q', query); + search.set('limit', String(page.limit)); + search.set('offset', String(page.offset)); + + const endpoint = query + ? `${url.replace(/\/$/, '')}/discovery/search?${search}` + : `${url.replace(/\/$/, '')}/discovery/resources?${search}`; + const upstream = new URL(endpoint).host; const res = await ssrfSafeFetch( endpoint, @@ -240,9 +265,21 @@ export class BazaarMcpServer { ); } + // Cap the read, not only the reply: `res.json()` would buffer whatever a + // node chose to send before we got a say in the size. + const body = await readBodyCapped(res, this.limits.maxUpstreamBytes); + if (body.exceeded) { + return this.createErrorResponse( + 'ERR_RESPONSE_TOO_LARGE', + `Upstream ${upstream} sent more than ${this.limits.maxUpstreamBytes} bytes, which cannot be read as one document. ` + + `Retry with a smaller limit (this page asked for ${page.limit}) or a larger offset.`, + { maxUpstreamBytes: this.limits.maxUpstreamBytes, limit: page.limit, offset: page.offset }, + ); + } + let data: unknown; try { - data = await res.json(); + data = JSON.parse(body.text); } catch { return this.createErrorResponse('ERR_SERVICE_UNREACHABLE', `Upstream ${upstream} returned a body that is not valid JSON`); } @@ -253,10 +290,16 @@ export class BazaarMcpServer { if ((Array.isArray(data) && data.length === 0) || (hasResources(data) && data.resources.length === 0)) { return this.createErrorResponse('ERR_NO_RESULTS', 'No resources found'); } - - return { - content: [{ type: 'text', text: JSON.stringify(data, null, 2) }], - }; + + return this.createBoundedResult(data, { + maxBytes: this.limits.maxResultBytes, + maxItems: page.limit, + offset: page.offset, + notice: + `Result truncated to fit a ${this.limits.maxResultBytes}-byte tool output limit, ` + + `of which at most ${page.limit} items fit on one page. ` + + 'Ask for the rest with the limit and offset arguments.', + }); } catch (e: any) { if (e instanceof SsrfError) { return this.createErrorResponse('ERR_URL_BLOCKED', `Blocked request: ${e.message}`); @@ -275,9 +318,12 @@ export class BazaarMcpServer { if (!this.budget) { return this.createErrorResponse('ERR_BAD_REQUEST', 'This server has no spend budget configured.'); } - return { - content: [{ type: 'text', text: JSON.stringify(this.budget.snapshot(), null, 2) }], - }; + return this.createBoundedResult(this.budget.snapshot(), { + maxBytes: this.limits.maxResultBytes, + maxItems: Number.MAX_SAFE_INTEGER, + offset: 0, + notice: `The spend budget was truncated to fit a ${this.limits.maxResultBytes}-byte tool output limit.`, + }); } if (request.params.name === 'paid_call') { @@ -316,17 +362,34 @@ export class BazaarMcpServer { } this.settleReservation(reservationId, true); - const text = await res.text(); + const body = await readBodyCapped(res, this.limits.maxUpstreamBytes); + + const budget: ResultBudget = { + maxBytes: this.limits.maxResultBytes, + // paid_call has no pagination contract, so paging a collection would + // hide items with no way to ask for the next page; here only the + // byte ceiling applies. + maxItems: Number.MAX_SAFE_INTEGER, + offset: 0, + knownTruncated: body.exceeded, + notice: body.exceeded + ? `Only the first ${this.limits.maxUpstreamBytes} bytes this resource sent are shown, so this result is incomplete. ` + + 'Ask the resource for a smaller response or a paginated endpoint.' + : `Result truncated to fit a ${this.limits.maxResultBytes}-byte tool output limit. ` + + 'Ask the resource for a smaller response or a paginated endpoint.', + }; + let parsed; try { - parsed = JSON.parse(text); + parsed = JSON.parse(body.text); } catch { - parsed = text; + // Also the case of a body cut off mid-read, which cannot parse: + // knownTruncated labels the fragment instead of passing it off as an + // answer. + parsed = body.text; } - - return { - content: [{ type: 'text', text: typeof parsed === 'string' ? parsed : JSON.stringify(parsed, null, 2) }], - }; + + return this.createBoundedResult(parsed, budget); } catch (e: any) { this.settleReservation(reservationId, !this.paymentCouldNotHaveSettled(e)); return this.handlePaidCallError(e); @@ -337,6 +400,30 @@ export class BazaarMcpServer { }); } + /** + * Clamp a caller-requested page to the operator's bounds. Zod has already + * rejected non-positive or non-integer values; this is the ceiling an agent + * may not argue past, mirroring the node's own discovery limits. + */ + private resolvePage(requested: { limit?: number; offset?: number }): { limit: number; offset: number } { + const limit = + requested.limit !== undefined ? Math.min(requested.limit, this.limits.maxPageLimit) : this.limits.defaultPageLimit; + return { limit, offset: requested.offset ?? 0 }; + } + + /** + * Serialize a successful result compactly — indentation is for humans and + * costs the agent context — and within `budget`, so no upstream response, + * large or hostile, can fill a context window in one call. Truncation is + * stated in the payload rather than left to be inferred. + */ + private createBoundedResult(data: unknown, budget: ResultBudget) { + const { text } = boundSerialized(data, budget); + return { + content: [{ type: 'text', text }], + }; + } + /** * Turn a thrown paid-call error into an MCP error response. * diff --git a/src/mcp/toolSchemas.ts b/src/mcp/toolSchemas.ts index 4673b0b7..89c02cd0 100644 --- a/src/mcp/toolSchemas.ts +++ b/src/mcp/toolSchemas.ts @@ -1,4 +1,5 @@ import { z } from 'zod'; +import { config } from '../config.js'; /** * Supported HTTP methods for the `paid_call` tool. @@ -24,6 +25,23 @@ export const BAZAAR_SEARCH_ARGUMENTS_SCHEMA = z.object({ .min(1, 'url cannot be empty') .describe('The base URL of the Bazaar API'), query: z.string().optional().describe('Optional search query'), + // #416: results are a bounded page, so paging has to be something the agent + // can ask for. The ceiling is operator configuration and is enforced again on + // the server side — an ask over it is clamped, not honoured. + limit: z + .number() + .int() + .positive() + .optional() + .describe( + `Maximum number of resources to return (default ${config.mcp.defaultPageLimit}, capped at ${config.mcp.maxPageLimit})`, + ), + offset: z + .number() + .int() + .min(0) + .optional() + .describe('Number of resources to skip, to page past a truncated result (default 0)'), }); export type BazaarSearchArguments = z.infer; diff --git a/tests/mcp-bounded-results.test.ts b/tests/mcp-bounded-results.test.ts new file mode 100644 index 00000000..c68fdd9e --- /dev/null +++ b/tests/mcp-bounded-results.test.ts @@ -0,0 +1,353 @@ +import { describe, it, expect, vi } from 'vitest' +import { CallToolRequestSchema } from '@modelcontextprotocol/sdk/types.js' +import { BazaarMcpServer } from '../src/mcp/server' + +/** + * #416 — a tool result is written straight into an agent's context window, so an + * unbounded one is self-inflicted denial of service whose failure gets blamed on + * the model. These tests pin the four things that make a result safe to hand + * back: a byte ceiling, an explicit truncation label, no pretty-printing, and a + * bounded page plus a count wherever paging is something the agent can ask for. + * + * They also pin the second half of the fix, which is the part that cannot be + * undone by a client: the body read from upstream is capped while streaming, so + * an oversized response is never buffered in full here. + */ + +function captureHandler(server: BazaarMcpServer) { + let handler: any = null + ;(server as any).server.setRequestHandler = (schema: any, fn: any) => { + if (schema === CallToolRequestSchema) handler = fn + } + ;(server as any).setupHandlers() + return handler as (request: { params: { name: string; arguments?: Record } }) => Promise +} + +const publicLookup = { lookup: async () => ['93.184.216.34'] } + +function searchHandler(fetchImpl: (input: string | URL) => Promise, limits?: Record) { + const server = new BazaarMcpServer({ + fetchWithPayment: vi.fn(), + bazaarFetch: { allowedHosts: [], ...publicLookup, fetchImpl }, + ...(limits ? { limits } : {}), + }) + return captureHandler(server) +} + +/** A node that answers with JSON, as #480's content-type gate requires. */ +const jsonFetch = (payload: unknown) => async () => + new Response(JSON.stringify(payload), { + status: 200, + headers: { 'content-type': 'application/json' }, + }) + +const textOf = (response: any) => response.content[0].text + +describe('bazaar_search bounded results (#416)', () => { + it('pages a collection that exceeds the item limit and says so', async () => { + const resources = Array.from({ length: 500 }, (_, i) => ({ resource: { url: `https://r${i}.example` } })) + const handler = searchHandler(jsonFetch({ resources, total: resources.length })) + + const response = await handler({ params: { name: 'bazaar_search', arguments: { url: 'https://ok.example' } } }) + + expect(response.isError).toBeFalsy() + const text = textOf(response) + const parsed = JSON.parse(text) + expect(parsed.resources.length).toBeLessThanOrEqual(50) + expect(parsed.truncated).toBe(true) + expect(parsed.total).toBe(500) + expect(parsed.offset).toBe(0) + expect(parsed.notice).toMatch(/limit and offset/) + // Indentation is for humans; it costs the agent context for no benefit. + expect(text).not.toContain('\n ') + }) + + it('asks the node for the page it was given, and echoes it back', async () => { + const requested: string[] = [] + const resources = Array.from({ length: 30 }, (_, i) => ({ resource: { url: `https://r${i}.example` } })) + const handler = searchHandler(async (input: string | URL) => { + requested.push(String(input)) + return new Response(JSON.stringify({ resources }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }) + }) + + const response = await handler({ + params: { + name: 'bazaar_search', + arguments: { url: 'https://ok.example', query: 'xlm', limit: 7, offset: 14 }, + }, + }) + + expect(requested[0]).toContain('/discovery/search?') + expect(requested[0]).toContain('q=xlm') + expect(requested[0]).toContain('limit=7') + expect(requested[0]).toContain('offset=14') + + const parsed = JSON.parse(textOf(response)) + expect(parsed.resources.length).toBe(7) + expect(parsed.total).toBe(30) + expect(parsed.offset).toBe(14) + expect(parsed.truncated).toBe(true) + }) + + it('clamps a page an agent asks for beyond the operator ceiling', async () => { + const requested: string[] = [] + const handler = searchHandler(async (input: string | URL) => { + requested.push(String(input)) + return new Response(JSON.stringify({ resources: [{ resource: { url: 'https://a.example' } }] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }) + }, { maxPageLimit: 20 }) + + await handler({ params: { name: 'bazaar_search', arguments: { url: 'https://ok.example', limit: 5000 } } }) + + expect(requested[0]).toContain('limit=20') + }) + + it('keeps a response within the configured byte limit', async () => { + const resources = Array.from({ length: 1000 }, (_, i) => ({ + resource: { url: `https://example.com/${'y'.repeat(200)}${i}` }, + })) + const handler = searchHandler(jsonFetch({ resources }), { + maxResultBytes: 4096, + maxUpstreamBytes: 1_048_576, + }) + + const response = await handler({ params: { name: 'bazaar_search', arguments: { url: 'https://ok.example' } } }) + + const text = textOf(response) + expect(Buffer.byteLength(text, 'utf8')).toBeLessThanOrEqual(4096) + const parsed = JSON.parse(text) + expect(parsed.truncated).toBe(true) + expect(parsed.total).toBe(1000) + expect(parsed.resources.length).toBeGreaterThan(0) + expect(parsed.resources.length).toBeLessThan(1000) + }) + + it('truncates a payload with no pageable collection', async () => { + const handler = searchHandler(jsonFetch({ blob: 'z'.repeat(10_000) }), { maxResultBytes: 2048 }) + + const response = await handler({ params: { name: 'bazaar_search', arguments: { url: 'https://ok.example' } } }) + + const text = textOf(response) + expect(Buffer.byteLength(text, 'utf8')).toBeLessThanOrEqual(2048) + const parsed = JSON.parse(text) + expect(parsed.truncated).toBe(true) + expect(parsed.notice).toMatch(/limit and offset/) + expect(typeof parsed.partial).toBe('string') + expect(parsed.partial.length).toBeGreaterThan(0) + }) + + it('does not buffer an oversized upstream body in full', async () => { + const totalChunks = 64 + const chunk = new TextEncoder().encode('x'.repeat(4096)) + let produced = 0 + let cancelled = false + const stream = new ReadableStream({ + pull(controller) { + if (produced >= totalChunks) { + controller.close() + return + } + produced++ + controller.enqueue(chunk) + }, + cancel() { + cancelled = true + }, + }) + + const handler = searchHandler( + async () => + new Response(stream, { status: 200, headers: { 'content-type': 'application/json' } }), + { maxUpstreamBytes: 8192, maxResultBytes: 65_536 }, + ) + + const response = await handler({ params: { name: 'bazaar_search', arguments: { url: 'https://ok.example' } } }) + + expect(response.isError).toBe(true) + const payload = JSON.parse(textOf(response)) + expect(payload.code).toBe('ERR_RESPONSE_TOO_LARGE') + expect(payload.reason).toMatch(/smaller limit/) + expect(payload.details.maxUpstreamBytes).toBe(8192) + expect(produced).toBeLessThan(totalChunks) + expect(cancelled).toBe(true) + }) + + it('rejects a page size that is not a positive integer before fetching', async () => { + const fetchImpl = vi.fn(jsonFetch({ resources: [{ resource: { url: 'https://a.example' } }] })) + const server = new BazaarMcpServer({ + fetchWithPayment: vi.fn(), + bazaarFetch: { allowedHosts: [], ...publicLookup, fetchImpl }, + }) + const handler = captureHandler(server) + + const response = await handler({ params: { name: 'bazaar_search', arguments: { url: 'https://ok.example', limit: 0 } } }) + + expect(response.isError).toBe(true) + expect(JSON.parse(textOf(response)).code).toBe('ERR_BAD_REQUEST') + expect(fetchImpl).not.toHaveBeenCalled() + }) + + it('leaves a small response untouched', async () => { + const handler = searchHandler(jsonFetch({ resources: [{ resource: { url: 'https://a.example' } }] })) + + const response = await handler({ params: { name: 'bazaar_search', arguments: { url: 'https://ok.example' } } }) + + expect(response.isError).toBeFalsy() + const parsed = JSON.parse(textOf(response)) + expect(parsed.truncated).toBeUndefined() + expect(parsed.resources[0].resource.url).toBe('https://a.example') + }) +}) + +describe('lens_get_price bounded results (#416)', () => { + const TESTNET_USDC = 'GBBD47IF6LWK7P7MDEVSCWR7DPUWV3NY3DTQEVFL4NAT4AQH3ZLLFLA5' + + function priceHandler(limits?: Record, recommendation = 'AMM offers a better rate') { + const priceQueries: any = { + activeNetwork: 'testnet', + watchedPairs: () => [ + { + assetA: { code: 'USDC', issuer: TESTNET_USDC }, + assetB: { code: 'XLM', issuer: null }, + pairKey: `USDC:${TESTNET_USDC}/XLM`, + }, + ], + aggregatedPrice: async () => ({ + price: 0.185, + sdexPrice: 0.184, + ammPrice: 0.186, + volume24h: 1200, + sdexVolume24h: 900, + ammVolume24h: 300, + vwap1m: 0.1851, + vwap5m: 0.1852, + vwap1h: 0.185, + vwap24h: 0.184, + priceChange24h: -1.2, + sources: 3, + confidence: 'high', + lastTradeAgeSeconds: 12, + stale: false, + }), + bestRoute: async () => ({ + route: 'AMM', + sdexPrice: 0.184, + ammPrice: 0.186, + estimatedOutput: 184, + slippagePct: 0, + recommendation, + }), + } + + const server = new BazaarMcpServer({ + fetchWithPayment: vi.fn(), + priceQueries, + ...(limits ? { limits } : {}), + }) + return captureHandler(server) + } + + const args = { + assetA: `USDC:${TESTNET_USDC}`, + assetB: 'XLM', + } + + it('keeps structuredContent while the result fits, and does not pad it', async () => { + const handler = priceHandler() + + const response = await handler({ params: { name: 'lens_get_price', arguments: args } }) + + expect(response.structuredContent).toBeTruthy() + expect(textOf(response)).toBe(JSON.stringify(response.structuredContent)) + expect(textOf(response)).not.toContain('\n ') + }) + + it('drops structuredContent when the payload cannot fit, and labels the text', async () => { + const handler = priceHandler({ maxResultBytes: 1024 }, 'r'.repeat(4000)) + + const response = await handler({ params: { name: 'lens_get_price', arguments: args } }) + + const text = textOf(response) + expect(Buffer.byteLength(text, 'utf8')).toBeLessThanOrEqual(1024) + expect(response.structuredContent).toBeUndefined() + expect(JSON.parse(text).truncated).toBe(true) + }) +}) + +describe('paid_call bounded results (#416)', () => { + const allowlist = ['example.com'] + + function paidHandler(response: Response, limits?: Record) { + const fetchWithPayment = vi.fn().mockResolvedValue(response) + const server = new BazaarMcpServer({ + fetchWithPayment, + paidCallAllowlist: allowlist, + ...(limits ? { limits } : {}), + }) + return { handler: captureHandler(server), fetchWithPayment } + } + + it('bounds a collection result and reports the truncation', async () => { + const { handler } = paidHandler( + new Response(JSON.stringify({ rows: Array.from({ length: 500 }, (_, i) => i) }), { status: 200 }), + { maxResultBytes: 1024 }, + ) + + const response = await handler({ params: { name: 'paid_call', arguments: { url: 'http://example.com/paid' } } }) + + expect(response.isError).toBeFalsy() + const text = textOf(response) + expect(Buffer.byteLength(text, 'utf8')).toBeLessThanOrEqual(1024) + const parsed = JSON.parse(text) + expect(parsed.truncated).toBe(true) + // paid_call has no pagination contract, so items are cut by bytes only and + // the result says what to do about it instead of pretending to be a page. + expect(parsed.notice).toMatch(/paginated endpoint/) + }) + + it('flags a body that exceeds the read cap as incomplete', async () => { + const big = 'a'.repeat(5000) + const stream = new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode(big)) + controller.enqueue(new TextEncoder().encode(big)) + controller.close() + }, + }) + const { handler } = paidHandler(new Response(stream, { status: 200 }), { + maxUpstreamBytes: 4096, + maxResultBytes: 2048, + }) + + const response = await handler({ params: { name: 'paid_call', arguments: { url: 'http://example.com/paid' } } }) + + const text = textOf(response) + expect(Buffer.byteLength(text, 'utf8')).toBeLessThanOrEqual(2048) + const parsed = JSON.parse(text) + expect(parsed.truncated).toBe(true) + expect(parsed.notice).toMatch(/incomplete/) + expect(parsed.partial).toContain('aaaa') + }) + + it('passes a plain-text answer through as plain text', async () => { + const { handler } = paidHandler(new Response('the price is 0.185', { status: 200 })) + + const response = await handler({ params: { name: 'paid_call', arguments: { url: 'http://example.com/paid' } } }) + + expect(textOf(response)).toBe('the price is 0.185') + }) + + it('bounds a result that stays under the cap without labelling it', async () => { + const payload = { ok: true, value: 1 } + const { handler } = paidHandler(new Response(JSON.stringify(payload), { status: 200 })) + + const response = await handler({ params: { name: 'paid_call', arguments: { url: 'http://example.com/paid' } } }) + + expect(textOf(response)).toBe(JSON.stringify(payload)) + }) +}) diff --git a/tests/mcp.test.ts b/tests/mcp.test.ts index abd9aac1..5a029272 100644 --- a/tests/mcp.test.ts +++ b/tests/mcp.test.ts @@ -204,7 +204,9 @@ describe('BazaarMcpServer bazaar_search SSRF guard (#407)', () => { const response = await handler({ params: { name: 'bazaar_search', arguments: { url: 'https://ok.example' } } }); expect(response.isError).toBe(true); expect(JSON.parse(response.content[0].text).code).toBe('ERR_URL_BLOCKED'); - expect(calls).toEqual(['https://ok.example/discovery/resources']); + // #416: the page the server asks the node for is bounded by default, so the + // first hop carries limit/offset. + expect(calls).toEqual(['https://ok.example/discovery/resources?limit=50&offset=0']); }); it('returns results from a permitted public host', async () => {