From 03a213091973515d8698c90607856628883ded65 Mon Sep 17 00:00:00 2001 From: Jakob Wennberg <149234542+jakobwennberg@users.noreply.github.com> Date: Mon, 8 Jun 2026 09:14:46 +0200 Subject: [PATCH] feat(mcp): Origin-header validation + serverInfo title + connect-claude docs export (P0-4 follow-up) (#684) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the two code-side gaps found while auditing the Claude Connectors Directory submission checklist after #682/#683: 1. Origin-header validation on the /mcp endpoint (POST/GET/DELETE) — an explicit directory submission requirement and an MCP spec MUST for the Streamable HTTP transport (DNS-rebinding defense). Requests without an Origin header (claude.ai backend, Claude Desktop, npx gnubok-mcp, Claude Code, MCP Inspector's proxy — every known client) pass through unchanged. A present Origin is allowed only when its host matches the request Host (covers Vercel previews + self-hosted without hardcoding) or NEXT_PUBLIC_APP_URL (proxy-rewritten Host); anything else is 403 with a JSON-RPC error envelope. The endpoint sets no CORS headers, so no currently-working browser flow is affected. 2. serverInfo.title: 'Accounted' (MCP 2025-06-18 display name). name stays 'gnubok' — stable identifier clients may key state on. 3. export-docs-to-website.mts now also exports CONNECT_CLAUDE_MD to the gnubok-website repo, so docs.gnubok.se/connect-claude (the target of the canonical /docs/api redirect) stays in sync. Companion website PR: jakobwennberg/gnubok-website#1. Tests: new origin-guard.test.ts (10 tests — no-Origin pass-through, same-origin, preview host, proxy host via env, foreign/port-mismatch/ null/malformed rejection, 403 envelope, and per-method enforcement on the registered apiRoutes). Full MCP suite 295/295 green. Co-authored-by: Claude Opus 4.8 (1M context) --- .../mcp-server/__tests__/origin-guard.test.ts | 118 ++++++++++++++++++ extensions/general/mcp-server/index.ts | 16 ++- extensions/general/mcp-server/origin-guard.ts | 59 +++++++++ extensions/general/mcp-server/server.ts | 4 + scripts/export-docs-to-website.mts | 17 ++- 5 files changed, 209 insertions(+), 5 deletions(-) create mode 100644 extensions/general/mcp-server/__tests__/origin-guard.test.ts create mode 100644 extensions/general/mcp-server/origin-guard.ts diff --git a/extensions/general/mcp-server/__tests__/origin-guard.test.ts b/extensions/general/mcp-server/__tests__/origin-guard.test.ts new file mode 100644 index 00000000..feb9ab6a --- /dev/null +++ b/extensions/general/mcp-server/__tests__/origin-guard.test.ts @@ -0,0 +1,118 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest' +import { isForbiddenOrigin, forbiddenOriginResponse } from '../origin-guard' + +const ENDPOINT = 'https://app.gnubok.se/api/extensions/ext/mcp-server/mcp' + +function makeRequest(headers: Record = {}, url = ENDPOINT): Request { + return new Request(url, { method: 'POST', headers }) +} + +describe('isForbiddenOrigin', () => { + const originalAppUrl = process.env.NEXT_PUBLIC_APP_URL + + beforeEach(() => { + delete process.env.NEXT_PUBLIC_APP_URL + }) + + afterEach(() => { + if (originalAppUrl === undefined) { + delete process.env.NEXT_PUBLIC_APP_URL + } else { + process.env.NEXT_PUBLIC_APP_URL = originalAppUrl + } + }) + + it('allows requests without an Origin header (server-to-server clients)', () => { + // claude.ai backend, Claude Desktop, npx gnubok-mcp, Claude Code — none + // send Origin. This is the path every known MCP client takes. + expect(isForbiddenOrigin(makeRequest())).toBe(false) + }) + + it('allows a same-origin browser request (Origin host matches Host header)', () => { + expect( + isForbiddenOrigin( + makeRequest({ origin: 'https://app.gnubok.se', host: 'app.gnubok.se' }), + ), + ).toBe(false) + }) + + it('allows same-origin on a Vercel preview host', () => { + expect( + isForbiddenOrigin( + makeRequest( + { origin: 'https://erp-base-abc123.vercel.app', host: 'erp-base-abc123.vercel.app' }, + 'https://erp-base-abc123.vercel.app/api/extensions/ext/mcp-server/mcp', + ), + ), + ).toBe(false) + }) + + it('allows an Origin matching NEXT_PUBLIC_APP_URL even when Host was rewritten by a proxy', () => { + process.env.NEXT_PUBLIC_APP_URL = 'https://app.gnubok.se' + expect( + isForbiddenOrigin( + makeRequest( + { origin: 'https://app.gnubok.se', host: 'internal-proxy.local' }, + 'https://internal-proxy.local/api/extensions/ext/mcp-server/mcp', + ), + ), + ).toBe(false) + }) + + it('rejects a foreign Origin (DNS-rebinding / cross-site browser request)', () => { + expect( + isForbiddenOrigin( + makeRequest({ origin: 'https://evil.example.com', host: 'app.gnubok.se' }), + ), + ).toBe(true) + }) + + it('rejects a foreign Origin that only differs by port', () => { + expect( + isForbiddenOrigin( + makeRequest({ origin: 'https://app.gnubok.se:8443', host: 'app.gnubok.se' }), + ), + ).toBe(true) + }) + + it('rejects an opaque "null" Origin', () => { + expect(isForbiddenOrigin(makeRequest({ origin: 'null', host: 'app.gnubok.se' }))).toBe(true) + }) + + it('rejects a malformed Origin header', () => { + expect( + isForbiddenOrigin(makeRequest({ origin: 'not a url', host: 'app.gnubok.se' })), + ).toBe(true) + }) +}) + +describe('forbiddenOriginResponse', () => { + it('returns a 403 JSON-RPC error envelope', async () => { + const res = forbiddenOriginResponse() + expect(res.status).toBe(403) + const body = await res.json() + expect(body).toEqual({ + jsonrpc: '2.0', + id: null, + error: { code: -32600, message: 'Origin not allowed' }, + }) + }) +}) + +describe('mcp-server apiRoutes origin enforcement', () => { + it('rejects foreign-Origin requests on every /mcp method before dispatch', async () => { + const { mcpServerExtension } = await import('../index') + const routes = (mcpServerExtension.apiRoutes ?? []).filter((r) => r.path === '/mcp') + expect(routes.map((r) => r.method).sort()).toEqual(['DELETE', 'GET', 'POST']) + + for (const route of routes) { + const res = await route.handler( + new Request(ENDPOINT, { + method: route.method, + headers: { origin: 'https://evil.example.com', host: 'app.gnubok.se' }, + }), + ) + expect(res.status, `${route.method} /mcp`).toBe(403) + } + }) +}) diff --git a/extensions/general/mcp-server/index.ts b/extensions/general/mcp-server/index.ts index 67d292e1..f42d2816 100644 --- a/extensions/general/mcp-server/index.ts +++ b/extensions/general/mcp-server/index.ts @@ -1,5 +1,6 @@ import type { Extension } from '@/lib/extensions/types' import { handleMcpRequest, tools as mcpTools } from './server' +import { isForbiddenOrigin, forbiddenOriginResponse } from './origin-guard' import { registerAgentTools } from '@/lib/agent/tools/registry' import type { AgentTool } from '@/lib/agent/tools/types' @@ -24,14 +25,19 @@ export const mcpServerExtension: Extension = { method: 'POST', path: '/mcp', skipAuth: true, // Auth handled via API key in the handler - handler: handleMcpRequest, + handler: async (request: Request) => { + // MCP spec MUST: validate Origin (DNS-rebinding defense). See origin-guard.ts. + if (isForbiddenOrigin(request)) return forbiddenOriginResponse() + return handleMcpRequest(request) + }, }, // MCP Streamable HTTP also needs GET for SSE and DELETE for session termination { method: 'GET', path: '/mcp', skipAuth: true, - handler: async () => { + handler: async (request: Request) => { + if (isForbiddenOrigin(request)) return forbiddenOriginResponse() const appUrl = process.env.NEXT_PUBLIC_APP_URL || 'http://localhost:3000' return new Response('Authorization required', { status: 401, @@ -45,7 +51,11 @@ export const mcpServerExtension: Extension = { method: 'DELETE', path: '/mcp', skipAuth: true, - handler: async () => new Response(null, { status: 204 }), // Stateless — no sessions to terminate + // Stateless — no sessions to terminate + handler: async (request: Request) => { + if (isForbiddenOrigin(request)) return forbiddenOriginResponse() + return new Response(null, { status: 204 }) + }, }, ], diff --git a/extensions/general/mcp-server/origin-guard.ts b/extensions/general/mcp-server/origin-guard.ts new file mode 100644 index 00000000..06c19211 --- /dev/null +++ b/extensions/general/mcp-server/origin-guard.ts @@ -0,0 +1,59 @@ +/** + * Origin-header validation for the MCP Streamable HTTP endpoint. + * + * MCP spec (2025-06-18, Streamable HTTP transport): "Servers MUST validate + * the Origin header on all incoming connections to prevent DNS rebinding + * attacks." Also an explicit Claude Connectors Directory submission + * requirement. + * + * Non-browser clients send no Origin header and are allowed: claude.ai's + * backend connector, Claude Desktop, the npx gnubok-mcp bridge, Claude Code, + * and MCP Inspector (whose Node proxy makes the actual call). A browser page + * sends its own origin: allowed only when it matches the deployment's own + * host — compared against the request Host (covers Vercel previews and + * self-hosted domains without hardcoding) and NEXT_PUBLIC_APP_URL (covers + * proxies that rewrite Host). Anything else is a cross-site browser request + * the endpoint never serves (it sets no CORS headers), so reject explicitly. + */ +export function isForbiddenOrigin(request: Request): boolean { + const origin = request.headers.get('origin') + if (!origin) return false + + let originHost: string + try { + originHost = new URL(origin).host + } catch { + // Malformed Origin (including the literal "null" some browsers send for + // sandboxed/opaque contexts) — treat as foreign. + return true + } + + const allowedHosts = new Set() + const hostHeader = request.headers.get('host') + if (hostHeader) allowedHosts.add(hostHeader) + try { + allowedHosts.add(new URL(request.url).host) + } catch { + // request.url should always parse; ignore if not. + } + if (process.env.NEXT_PUBLIC_APP_URL) { + try { + allowedHosts.add(new URL(process.env.NEXT_PUBLIC_APP_URL).host) + } catch { + // Misconfigured env var — fall through to the request-derived hosts. + } + } + + return !allowedHosts.has(originHost) +} + +export function forbiddenOriginResponse(): Response { + return Response.json( + { + jsonrpc: '2.0', + id: null, + error: { code: -32600, message: 'Origin not allowed' }, + }, + { status: 403 }, + ) +} diff --git a/extensions/general/mcp-server/server.ts b/extensions/general/mcp-server/server.ts index 94cf1db6..fb386af9 100644 --- a/extensions/general/mcp-server/server.ts +++ b/extensions/general/mcp-server/server.ts @@ -9003,7 +9003,11 @@ export const tools: McpTool[] = [ // ── MCP Protocol Handler ───────────────────────────────────── const SERVER_INFO = { + // `name` is a stable identifier clients may key state on — stays 'gnubok' + // per the rebrand rule. `title` is the human-readable display name + // (MCP spec 2025-06-18). name: 'gnubok', + title: 'Accounted', version: '1.0.0', } diff --git a/scripts/export-docs-to-website.mts b/scripts/export-docs-to-website.mts index 325421b5..45787118 100644 --- a/scripts/export-docs-to-website.mts +++ b/scripts/export-docs-to-website.mts @@ -1,15 +1,18 @@ /** * One-shot script that exports the registry-derived docs content (errors + - * reference) as static TypeScript modules into the gnubok-website repo. + * reference) plus the static Connect-with-Claude page as TypeScript modules + * into the gnubok-website repo. * * Run with `npx tsx scripts/export-docs-to-website.mts`. Re-run whenever - * structured-errors or the v1 endpoint registry materially changes. + * structured-errors, the v1 endpoint registry, or connect-claude materially + * changes. */ import { writeFileSync, mkdirSync } from 'node:fs' import { dirname, resolve } from 'node:path' const errors = await import('@/lib/docs/content/errors') const reference = await import('@/lib/docs/content/reference') +const connectClaude = await import('@/lib/docs/content/connect-claude') const buildErrorReferenceMd = errors.buildErrorReferenceMd ?? (errors as any).default?.buildErrorReferenceMd const buildResourcePages = reference.buildResourcePages ?? (reference as any).default?.buildResourcePages @@ -56,4 +59,14 @@ write( )} as const\n\nexport const RESOURCE_PAGES: ResourcePage[] = ${JSON.stringify(pagesPayload, null, 2)}\n\nexport function findResourcePage(slug: string): ResourcePage | undefined {\n return RESOURCE_PAGES.find((p) => p.slug === slug)\n}\n`, ) +const connectClaudeMd = connectClaude.CONNECT_CLAUDE_MD +if (!connectClaudeMd) { + console.error('Missing CONNECT_CLAUDE_MD export. Inspect:', { connectClaudeKeys: Object.keys(connectClaude) }) + process.exit(1) +} +write( + 'lib/docs/content/connect-claude.generated.ts', + `// AUTO-GENERATED from erp-base — do not hand-edit.\n// Regenerate via \`npx tsx scripts/export-docs-to-website.mts\` in erp-base.\nexport const CONNECT_CLAUDE_MD = ${JSON.stringify(connectClaudeMd)}\n`, +) + console.log('done.')