Skip to content

fix(mcp): keep country brief context out of signed URL#4508

Merged
koala73 merged 4 commits into
mainfrom
codex/fix-mcp-country-brief-context-body
Jun 28, 2026
Merged

fix(mcp): keep country brief context out of signed URL#4508
koala73 merged 4 commits into
mainfrom
codex/fix-mcp-country-brief-context-body

Conversation

@koala73

@koala73 koala73 commented Jun 28, 2026

Copy link
Copy Markdown
Owner

Summary

Pro/OAuth get_country_brief no longer signs its grounding context in the URL, so large country brief requests avoid proxy/CDN request-line truncation that could invalidate the downstream HMAC. The tool now signs a short /api/intelligence/v1/get-country-intel-brief URL and sends the bounded grounding context in the existing JSON POST body, which the gateway already promotes after internal-MCP verification. If this endpoint still returns a non-2xx response, the thrown error now includes the gateway error code so Sentry can distinguish invalid_internal_mcp_signature from entitlement failures.

Fixes #4503.

Validation

  • git diff --check
  • node --check tests/mcp.test.mjs
  • node --check tests/mcp-news-auth-docs-contract.test.mjs
  • Attempted node --test tests/mcp-news-auth-docs-contract.test.mjs; blocked before assertions because plain Node cannot resolve this repo's extensionless TypeScript imports.
  • Attempted npm ci --ignore-scripts --cache /tmp/worldmonitor-npm-cache and npm ci --ignore-scripts --omit=optional --no-audit --cache /tmp/worldmonitor-npm-cache; both failed with ENOSPC: no space left on device, so the tsx --test suites could not be run locally in this worktree.

Compound Engineering
GPT-5

@vercel

vercel Bot commented Jun 28, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
worldmonitor Ready Ready Preview, Comment Jun 28, 2026 5:41pm

Request Review

@greptile-apps

greptile-apps Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a URL-length bug on the get_country_brief MCP tool where large grounding contexts, serialised as a URL query parameter, could cause the signed request line to exceed CDN/proxy limits and invalidate the downstream HMAC. Context is now kept entirely in the POST body alongside the existing country_code and framework fields, and the gateway's POST-to-GET promotion path handles it transparently.

  • api/mcp/registry/rpc-tools.ts: Removes the ?context= query-string path, adds context as an optional field on the JSON payload, and improves the error handler to extract structured error codes from gateway JSON responses for better Sentry signal.
  • tests/mcp.test.mjs: Adds a Pro-path end-to-end test that verifies the short URL, correct body payload, HMAC validity for the signed request, and a tamper-proof assertion that re-injecting context into the URL breaks the signature.
  • tests/mcp-news-auth-docs-contract.test.mjs: Updates the existing context-location assertion and adds a large-context scenario to confirm the signed URL stays under 512 characters.

Confidence Score: 4/5

Safe to merge — the core signing change is correct, well-tested with a tamper-proof HMAC assertion, and all three changed files are internally consistent.

The logic change is narrow and the test suite directly exercises the HMAC validity and the tamper-proof scenario. The only open nit is the HTML-fallback slice in the new error handler, which affects Sentry readability but not correctness or security.

No files require special attention beyond the minor error-message quality nit in api/mcp/registry/rpc-tools.ts.

Important Files Changed

Filename Overview
api/mcp/registry/rpc-tools.ts Core fix: moves grounding context from URL query param to POST body, keeping the signed URL short to avoid CDN/proxy truncation; adds structured error code extraction from gateway responses.
tests/mcp-news-auth-docs-contract.test.mjs Updates existing context-location assertion from URL search params to POST body; adds a new test verifying that a large (≥3900 char) grounding context produces a short (<512 char) signed URL while the context lands in the body.
tests/mcp.test.mjs Adds an end-to-end Pro-path test that verifies short URL, correct body payload, HMAC verification on the signed request, and a tamper-proof assertion that moving context back to the URL invalidates the signature.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Client as MCP Client
    participant Tool as get_country_brief (_execute)
    participant Digest as /api/news/v1/list-feed-digest
    participant Gateway as /api/intelligence/v1/get-country-intel-brief

    Client->>Tool: get_country_brief(country_code, framework)
    Tool->>Digest: GET (signed, 2 s timeout)
    Digest-->>Tool: headlines + sources

    Note over Tool: Build contextSnapshot (≤4000 chars)<br/>kept OUT of the signed URL

    Tool->>Tool: "briefPayload = { country_code, framework, context? }"
    Tool->>Tool: "briefBody = JSON.stringify(briefPayload)"
    Tool->>Tool: buildAuthHeaders(POST, briefUrl, briefBody) → HMAC over short URL + body

    Tool->>Gateway: "POST briefUrl Body: { country_code, framework, context }"
    Note over Gateway: gateway verifies HMAC<br/>(URL + body hash)<br/>then promotes body fields for GET handler

    alt res.ok
        Gateway-->>Tool: "{ brief, sources }"
        Tool-->>Client: result + merged sources
    else !res.ok
        Gateway-->>Tool: "{ error: invalid_internal_mcp_signature } or HTML"
        Tool->>Tool: parse error code (JSON → .error, else slice 120)
        Tool-->>Client: Error get-country-intel-brief HTTP 4xx code
    end
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Client as MCP Client
    participant Tool as get_country_brief (_execute)
    participant Digest as /api/news/v1/list-feed-digest
    participant Gateway as /api/intelligence/v1/get-country-intel-brief

    Client->>Tool: get_country_brief(country_code, framework)
    Tool->>Digest: GET (signed, 2 s timeout)
    Digest-->>Tool: headlines + sources

    Note over Tool: Build contextSnapshot (≤4000 chars)<br/>kept OUT of the signed URL

    Tool->>Tool: "briefPayload = { country_code, framework, context? }"
    Tool->>Tool: "briefBody = JSON.stringify(briefPayload)"
    Tool->>Tool: buildAuthHeaders(POST, briefUrl, briefBody) → HMAC over short URL + body

    Tool->>Gateway: "POST briefUrl Body: { country_code, framework, context }"
    Note over Gateway: gateway verifies HMAC<br/>(URL + body hash)<br/>then promotes body fields for GET handler

    alt res.ok
        Gateway-->>Tool: "{ brief, sources }"
        Tool-->>Client: result + merged sources
    else !res.ok
        Gateway-->>Tool: "{ error: invalid_internal_mcp_signature } or HTML"
        Tool->>Tool: parse error code (JSON → .error, else slice 120)
        Tool-->>Client: Error get-country-intel-brief HTTP 4xx code
    end
Loading

Reviews (1): Last reviewed commit: "fix(mcp): keep country brief context out..." | Re-trigger Greptile

Comment thread api/mcp/registry/rpc-tools.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(mcp): get_country_brief 401 recurs after ?rpc-echo HMAC fix — only tool with its own ?context= query (WORLDMONITOR-T8)

1 participant