Skip to content

Commit 2c9eb68

Browse files
authored
fix(mcp): keep country brief context out of signed URL (koala73#4508)
* fix(mcp): keep country brief context out of signed URL * fix(mcp): normalize country brief error fallback * test(mcp): cover country brief error details
1 parent aaf0f44 commit 2c9eb68

4 files changed

Lines changed: 221 additions & 25 deletions

File tree

api/mcp/registry/rpc-tools.ts

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ export const RPC_TOOLS: ToolDef[] = [
238238
// Fetch current geopolitical headlines to ground the LLM (budget: 2 s — cached endpoint).
239239
// Without context the model hallucinates events — real headlines anchor it.
240240
// 2 s + 22 s brief = 24 s worst-case; 6 s margin before the 30 s Edge kill.
241-
let contextParam = '';
241+
let contextSnapshot = '';
242242
let sources: McpBriefSource[] = [];
243243
try {
244244
const digestUrl = `${base}/api/news/v1/list-feed-digest?variant=full&lang=en`;
@@ -263,15 +263,19 @@ export const RPC_TOOLS: ToolDef[] = [
263263
const sourceLines = sources.length > 0 ? ['Brief source articles:', ...briefSourceContextLines(sources)] : [];
264264
const headlineLines = groundingItems.map(item => item.title ?? '').filter(Boolean);
265265
const contextLines = [...sourceLines, 'Headlines:', ...headlineLines].join('\n');
266-
if (contextLines.trim()) contextParam = encodeURIComponent(contextLines.slice(0, 4000));
266+
if (contextLines.trim()) contextSnapshot = contextLines.slice(0, 4000);
267267
}
268268
} catch { /* proceed without context — better than failing */ }
269269

270-
const briefUrl = contextParam
271-
? `${base}/api/intelligence/v1/get-country-intel-brief?context=${contextParam}`
272-
: `${base}/api/intelligence/v1/get-country-intel-brief`;
273-
274-
const briefBody = JSON.stringify({ country_code: countryCode, framework: String(params.framework ?? '') });
270+
const briefUrl = `${base}/api/intelligence/v1/get-country-intel-brief`;
271+
// Keep grounding context out of the signed URL; the gateway's POST-to-GET
272+
// compatibility path promotes scalar JSON body fields for this GET handler.
273+
const briefPayload: { country_code: string; framework: string; context?: string } = {
274+
country_code: countryCode,
275+
framework: String(params.framework ?? ''),
276+
};
277+
if (contextSnapshot) briefPayload.context = contextSnapshot;
278+
const briefBody = JSON.stringify(briefPayload);
275279
const briefAuth = await buildAuthHeaders(context, 'POST', briefUrl, briefBody);
276280
const res = await fetch(briefUrl, {
277281
method: 'POST',
@@ -281,22 +285,20 @@ export const RPC_TOOLS: ToolDef[] = [
281285
});
282286
if (!res.ok) {
283287
// Surface the gateway's error code in the thrown message so Sentry
284-
// groups the failure by ROOT CAUSE, not just status. This is the only
285-
// tool that appends its own `?context=` query param to the signed URL,
286-
// so a residual internal-HMAC drift (canonicalisation / URL-length
287-
// truncation in transit) surfaces as `invalid_internal_mcp_signature`,
288-
// while an expired/free caller surfaces as `insufficient_entitlement`
289-
// — previously indistinguishable from the bare `HTTP 401`
290-
// (WORLDMONITOR-T8 — recurred after the 2026-06-12 ?rpc-echo fix). Body
291-
// read is best-effort; a read failure must not mask the status.
288+
// groups the failure by root cause, not just status. Body reads are
289+
// best-effort; a read failure must not mask the HTTP status.
292290
const detail = await res.text().catch(() => '');
293291
let code = '';
294-
// `error` is a string today (e.g. `invalid_internal_mcp_signature`,
295-
// `insufficient_entitlement`), but JSON.stringify any non-string shape so
296-
// an object envelope renders readable JSON instead of `[object Object]`,
297-
// which would defeat the whole point of surfacing the code. Bound BOTH
298-
// shapes so the Sentry title can't bloat on a long body.
299-
try { const e = (JSON.parse(detail) as { error?: unknown }).error ?? ''; code = (typeof e === 'string' ? e : JSON.stringify(e)).slice(0, 120); } catch { code = detail.slice(0, 120); }
292+
// `error` is usually a string (for example,
293+
// `invalid_internal_mcp_signature`), but stringify non-string shapes so
294+
// object envelopes remain readable. Bound both paths so Sentry titles
295+
// cannot bloat on a long body.
296+
try {
297+
const error = (JSON.parse(detail) as { error?: unknown }).error ?? '';
298+
code = (typeof error === 'string' ? error : JSON.stringify(error)).slice(0, 120);
299+
} catch {
300+
code = detail.replace(/<[^>]*>/g, ' ').replace(/\s+/g, ' ').trim().slice(0, 120);
301+
}
300302
throw new Error(`get-country-intel-brief HTTP ${res.status}${code ? `: ${code}` : ''}`);
301303
}
302304
const result = await res.json() as Record<string, unknown>;

server/worldmonitor/intelligence/v1/get-country-intel-brief.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,8 @@ export async function getCountryIntelBrief(
9797
let lang = 'en';
9898
try {
9999
const url = new URL(ctx.request.url);
100+
// MCP sends `context` in the signed POST body; the gateway promotes scalar
101+
// body fields into query params before this generated GET handler runs.
100102
const rawContextSnapshot = (url.searchParams.get('context') || '').trim().slice(0, 4000);
101103
sources = parseCountryBriefSources(rawContextSnapshot);
102104
contextSnapshot = sanitizeForPrompt(rawContextSnapshot);

tests/mcp-news-auth-docs-contract.test.mjs

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ function rpcTool(name) {
2121
return tool;
2222
}
2323

24-
async function captureRpcFetches(toolName, params) {
24+
async function captureRpcFetches(toolName, params, opts = {}) {
2525
const calls = [];
2626
let result;
2727
globalThis.fetch = async (input, init = {}) => {
@@ -35,6 +35,13 @@ async function captureRpcFetches(toolName, params) {
3535
categories: {
3636
world: {
3737
items: [
38+
...(opts.longCountryHeadline ? [{
39+
title: `United States ${'%'.repeat(3900)}`,
40+
source: 'Long Wire',
41+
link: 'https://example.com/long-context',
42+
publishedAt: '2026-06-07T00:00:00.000Z',
43+
snippet: 'Large context headline used to prove the signed URL stays short.',
44+
}] : []),
3845
{
3946
title: 'United States headline used for MCP grounding',
4047
source: 'Example Wire',
@@ -122,9 +129,24 @@ describe('MCP news/auth public contract', () => {
122129
assert.deepEqual(countryResult.sources, worldResult.sources);
123130
const countryBriefCall = countryCalls.find((call) => new URL(call.url).pathname === '/api/intelligence/v1/get-country-intel-brief');
124131
assert.ok(countryBriefCall, 'get_country_brief should call country brief endpoint');
125-
const context = new URL(countryBriefCall.url).searchParams.get('context') || '';
126-
assert.match(decodeURIComponent(context), /Source \[1\]: \{"title":"United States headline used for MCP grounding","source":"Example Wire","url":"https:\/\/example\.com\/world-grounding","publishedAt":"2026-06-07T00:00:00.000Z"\}/);
127-
assert.doesNotMatch(decodeURIComponent(context), /russia-house/);
132+
const context = JSON.parse(String(countryBriefCall.init.body)).context || '';
133+
assert.match(context, /Source \[1\]: \{"title":"United States headline used for MCP grounding","source":"Example Wire","url":"https:\/\/example\.com\/world-grounding","publishedAt":"2026-06-07T00:00:00.000Z"\}/);
134+
assert.doesNotMatch(context, /russia-house/);
135+
});
136+
137+
it('get_country_brief keeps large grounding context out of the signed URL', async () => {
138+
const { calls } = await captureRpcFetches('get_country_brief', { country_code: 'US' }, { longCountryHeadline: true });
139+
const countryBriefCall = calls.find((call) => new URL(call.url).pathname === '/api/intelligence/v1/get-country-intel-brief');
140+
assert.ok(countryBriefCall, 'get_country_brief should call country brief endpoint');
141+
142+
const url = new URL(countryBriefCall.url);
143+
assert.equal(url.searchParams.has('context'), false, 'grounding context must not travel in the signed URL query');
144+
assert.ok(countryBriefCall.url.length < 512, `signed URL should stay short, got ${countryBriefCall.url.length} chars`);
145+
146+
const body = JSON.parse(String(countryBriefCall.init.body));
147+
assert.equal(body.country_code, 'US');
148+
assert.match(body.context, /Brief source articles:/);
149+
assert.ok(body.context.length >= 3900, `expected large body context, got ${body.context.length} chars`);
128150
});
129151

130152
it('RPC brief output schemas expose structured sources', () => {

tests/mcp.test.mjs

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2694,6 +2694,176 @@ describe('api/mcp.ts — U7 Pro-path', () => {
26942694
assert.match(sig, /^\d{10}\.[A-Za-z0-9_-]+$/, 'signature must be <ts>.<base64url-sig>');
26952695
});
26962696

2697+
it('edge: Pro get_country_brief signs a short URL and sends grounding context in the POST body', async () => {
2698+
const { deps } = makeProDeps();
2699+
const captured = [];
2700+
globalThis.fetch = async (url, init = {}) => {
2701+
const call = {
2702+
url: String(url),
2703+
method: init.method || 'GET',
2704+
headers: new Headers(init.headers),
2705+
body: typeof init.body === 'string' ? init.body : '',
2706+
};
2707+
captured.push(call);
2708+
const { pathname } = new URL(call.url);
2709+
2710+
if (pathname === '/api/news/v1/list-feed-digest') {
2711+
const items = Array.from({ length: 15 }, (_, index) => ({
2712+
title: `Iran ${index} ${'%'.repeat(300)}`,
2713+
source: 'Context Wire',
2714+
link: `https://example.com/iran-${index}`,
2715+
publishedAt: '2026-06-07T00:00:00.000Z',
2716+
snippet: 'Long Iran grounding item used to keep the MCP signed URL below proxy limits.',
2717+
}));
2718+
return new Response(JSON.stringify({ categories: { world: { items } } }), {
2719+
status: 200,
2720+
headers: { 'Content-Type': 'application/json' },
2721+
});
2722+
}
2723+
2724+
if (pathname === '/api/intelligence/v1/get-country-intel-brief') {
2725+
return new Response(JSON.stringify({ brief: 'Grounded country brief.' }), {
2726+
status: 200,
2727+
headers: { 'Content-Type': 'application/json' },
2728+
});
2729+
}
2730+
2731+
throw new Error(`Unexpected fetch URL: ${call.url}`);
2732+
};
2733+
2734+
const res = await mcpHandler(proReq('POST', callBody('get_country_brief', {
2735+
country_code: 'IR',
2736+
framework: 'PMESII-PT',
2737+
})), deps);
2738+
assert.equal(res.status, 200);
2739+
const rpc = await res.json();
2740+
assert.ok(rpc.result?.content, 'tool call must return content');
2741+
2742+
const countryCall = captured.find((call) => new URL(call.url).pathname === '/api/intelligence/v1/get-country-intel-brief');
2743+
assert.ok(countryCall, 'country brief fetch must run');
2744+
const countryUrl = new URL(countryCall.url);
2745+
assert.equal(countryUrl.searchParams.has('context'), false, 'context must not be signed in the URL query');
2746+
assert.ok(countryCall.url.length < 200, `signed URL should stay short, got ${countryCall.url.length} chars`);
2747+
2748+
const body = JSON.parse(countryCall.body);
2749+
assert.equal(body.country_code, 'IR');
2750+
assert.equal(body.framework, 'PMESII-PT');
2751+
assert.match(body.context, /Brief source articles:/);
2752+
assert.match(body.context, /Headlines:/);
2753+
assert.match(body.context, /Iran/);
2754+
assert.ok(body.context.length > 1000, `expected large grounding context, got ${body.context.length} chars`);
2755+
assert.ok(body.context.length <= 4000, `grounding context should be bounded to 4000 chars, got ${body.context.length}`);
2756+
2757+
assert.ok(countryCall.headers.get('x-wm-mcp-internal'), 'X-WM-MCP-Internal must be set');
2758+
assert.equal(countryCall.headers.get('x-wm-mcp-user-id'), PRO_USER_ID);
2759+
assert.equal(countryCall.headers.get('x-worldmonitor-key'), null, 'X-WorldMonitor-Key must NOT be set for Pro');
2760+
2761+
const { verifyInternalMcpRequest } = await import(`../server/_shared/mcp-internal-hmac.ts?t=${Date.now()}`);
2762+
const signedReq = new Request(countryCall.url, {
2763+
method: 'POST',
2764+
headers: countryCall.headers,
2765+
body: countryCall.body,
2766+
});
2767+
assert.ok(await verifyInternalMcpRequest(signedReq, HMAC_SECRET), 'signature must verify for the short URL plus context body');
2768+
2769+
const tamperedUrl = `${countryCall.url}?context=${encodeURIComponent(body.context)}`;
2770+
const tamperedReq = new Request(tamperedUrl, {
2771+
method: 'POST',
2772+
headers: countryCall.headers,
2773+
body: countryCall.body,
2774+
});
2775+
assert.equal(await verifyInternalMcpRequest(tamperedReq, HMAC_SECRET), null, 'same signature must not verify if context is moved back into the URL');
2776+
});
2777+
2778+
it('edge: Pro get_country_brief surfaces gateway error detail for Sentry grouping', async () => {
2779+
const scenarios = [
2780+
{
2781+
name: 'structured HMAC 401',
2782+
expectedLog: 'warn',
2783+
makeResponse: () => new Response(
2784+
JSON.stringify({ error: 'invalid_internal_mcp_signature' }),
2785+
{ status: 401, headers: { 'Content-Type': 'application/json' } },
2786+
),
2787+
assertMessage: (message) => {
2788+
assert.equal(message, 'get-country-intel-brief HTTP 401: invalid_internal_mcp_signature');
2789+
},
2790+
},
2791+
{
2792+
name: 'HTML CDN 503',
2793+
expectedLog: 'error',
2794+
makeResponse: () => new Response(
2795+
'<!DOCTYPE html><html><head><title>Bad Gateway</title></head><body>Cloudflare outage</body></html>',
2796+
{ status: 503, headers: { 'Content-Type': 'text/html' } },
2797+
),
2798+
assertMessage: (message) => {
2799+
assert.equal(message, 'get-country-intel-brief HTTP 503: Bad Gateway Cloudflare outage');
2800+
assert.doesNotMatch(message, /<[^>]*>/, 'HTML tags must not leak into the Sentry/log title');
2801+
},
2802+
},
2803+
{
2804+
name: 'unreadable body 503',
2805+
expectedLog: 'error',
2806+
makeResponse: () => ({ ok: false, status: 503, text: async () => { throw new Error('body locked'); } }),
2807+
assertMessage: (message) => {
2808+
assert.equal(message, 'get-country-intel-brief HTTP 503');
2809+
},
2810+
},
2811+
];
2812+
2813+
for (const scenario of scenarios) {
2814+
const { deps } = makeProDeps();
2815+
const warnCalls = [];
2816+
const errorCalls = [];
2817+
const origWarn = console.warn;
2818+
const origError = console.error;
2819+
console.warn = (...args) => { warnCalls.push(args); };
2820+
console.error = (...args) => { errorCalls.push(args); };
2821+
try {
2822+
globalThis.fetch = async (url) => {
2823+
const { pathname } = new URL(String(url));
2824+
if (pathname === '/api/news/v1/list-feed-digest') {
2825+
return new Response(JSON.stringify({
2826+
categories: {
2827+
world: {
2828+
items: [{
2829+
title: 'Iran headline for failing country brief',
2830+
source: 'Context Wire',
2831+
link: 'https://example.com/iran-failure',
2832+
publishedAt: '2026-06-07T00:00:00.000Z',
2833+
snippet: 'Iran context item used before the brief endpoint fails.',
2834+
}],
2835+
},
2836+
},
2837+
}), { status: 200, headers: { 'Content-Type': 'application/json' } });
2838+
}
2839+
if (pathname === '/api/intelligence/v1/get-country-intel-brief') {
2840+
return scenario.makeResponse();
2841+
}
2842+
return new Response('', { status: 200 });
2843+
};
2844+
2845+
const res = await mcpHandler(proReq('POST', callBody('get_country_brief', {
2846+
country_code: 'IR',
2847+
framework: 'PMESII-PT',
2848+
})), deps);
2849+
assert.equal(res.status, 200, `${scenario.name}: JSON-RPC tool errors stay HTTP 200`);
2850+
const rpc = await res.json();
2851+
assert.equal(rpc.error?.code, -32603, `${scenario.name}: tool failure should be a JSON-RPC internal error`);
2852+
2853+
const expectedCalls = scenario.expectedLog === 'warn' ? warnCalls : errorCalls;
2854+
const unexpectedCalls = scenario.expectedLog === 'warn' ? errorCalls : warnCalls;
2855+
const mcpLogs = expectedCalls.filter((args) => args[0] === '[mcp] tool execution error:');
2856+
assert.equal(mcpLogs.length, 1, `${scenario.name}: expected one MCP execution log`);
2857+
assert.equal(unexpectedCalls.some((args) => args[0] === '[mcp] tool execution error:'), false, `${scenario.name}: wrong console severity`);
2858+
const loggedError = mcpLogs[0][1];
2859+
scenario.assertMessage(loggedError instanceof Error ? loggedError.message : String(loggedError));
2860+
} finally {
2861+
console.warn = origWarn;
2862+
console.error = origError;
2863+
}
2864+
}
2865+
});
2866+
26972867
it('edge: cache-only tool for Pro user goes through INCR/DECR path (counts toward 50/day)', async () => {
26982868
const { deps, pipe } = makeProDeps();
26992869
process.env.UPSTASH_REDIS_REST_URL = 'https://stub.upstash';

0 commit comments

Comments
 (0)