From fec93aa817c5ae1ee62709877b4ec6fabcb5e7f3 Mon Sep 17 00:00:00 2001 From: Sameh Khamis Date: Fri, 2 Oct 2026 19:28:18 -0700 Subject: [PATCH] fix(mcp): accept only direct local requests at /mcp The /mcp route now applies the same proxy and sharing checks as Provider Settings: requests carrying forwarding headers, or arriving while launcher sharing is on, are refused. The Host must name the port the request reached and a browser Origin must match that host exactly. The proxy and sharing checks move into shared helpers in keySetupCore; Provider Settings behaves as before. Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/CURRENT-STATE.md | 4 ++- docs/TOOLS.md | 6 ++-- server/mcp/plugin.js | 33 +++++++++++++++--- src/keySetupCore.mjs | 65 ++++++++++++++++++++++++----------- src/tools/localRoute.test.mjs | 37 ++++++++++++++++++-- 5 files changed, 113 insertions(+), 32 deletions(-) diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index 7e1a0cc10..fc5a026fe 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -21,7 +21,9 @@ and `gods-eye-view/tools/mcp` exposes a composed catalog over the Model Context Protocol. `npm run mcp` serves Core's tools over stdio to a local MCP client, reading from a running app's `/api` routes (default `http://localhost:4173`). The development and preview servers also serve the tools over HTTP at `/mcp`, -accepting only local requests with loopback hosts and origins. +accepting only direct local requests: a loopback host on the port reached, +an `Origin` (when sent) from that same host, no proxy forwarding headers, and +launcher sharing off. Queries cover earthquakes, active fires, recent launches, aircraft (in an area, by identifier, tracks, and type and route lookups), ships (in an area, by identifier, and tracks), satellites (next diff --git a/docs/TOOLS.md b/docs/TOOLS.md index 1e2397c0c..7e1642389 100644 --- a/docs/TOOLS.md +++ b/docs/TOOLS.md @@ -241,8 +241,10 @@ claude mcp add --transport http gods-eye-view http://localhost:4173/mcp ``` The route accepts only requests from this machine that name a loopback host -and, when a browser sends an `Origin`, come from a loopback origin. This is -local transport safety, not authentication. The +on the port they reached and, when a browser sends an `Origin`, come from +that same host. It refuses requests a proxy forwarded and refuses all +requests while launcher sharing is on. This is local transport safety, not +authentication. The local server makes no requests other than to the app's `/api` routes and the public feeds the sources already use. diff --git a/server/mcp/plugin.js b/server/mcp/plugin.js index c1d3c6c94..974ca9abd 100644 --- a/server/mcp/plugin.js +++ b/server/mcp/plugin.js @@ -2,10 +2,12 @@ * Serves the local MCP server at /mcp on the development and preview servers. * * This is local transport safety, not authentication: requests must come from - * this machine, name a loopback host (blocking DNS rebinding) and, when a - * browser sends an Origin, come from a loopback origin. + * this machine, carry no proxy forwarding headers, arrive while sharing is + * off, name a loopback host on the port they arrived at (blocking DNS + * rebinding) and, when a browser sends an Origin, come from that same host. */ +import { hasProxySignals, isSharingEnabled } from '../../src/keySetupCore.mjs'; import { createMcpHttpHandler } from '../../src/tools/mcp/index.js'; import { createLocalMcpServer } from './server.js'; @@ -13,9 +15,23 @@ const LOOPBACK_HOSTS = new Set(['localhost', '127.0.0.1', '[::1]']); const LOOPBACK_ADDRESSES = new Set(['127.0.0.1', '::1', '::ffff:127.0.0.1']); const MAX_BODY_BYTES = 1024 * 1024; -/** Whether a request may reach the local MCP server. */ -export function isLocalMcpRequest({ remoteAddress, host, origin }) { +/** + * Whether a request may reach the local MCP server. `localPort` is the port + * the connection arrived at; `headers` are the request's, keyed by lower-case + * name; `env` is checked for launcher sharing. + */ +export function isLocalMcpRequest({ + remoteAddress, + localPort, + host, + origin, + headers = {}, + env = {}, +}) { if (!LOOPBACK_ADDRESSES.has(remoteAddress)) return false; + // A proxy or tunnel in front of the server can make a remote request look + // local, and sharing exposes the server beyond this machine. + if (hasProxySignals(headers) || isSharingEnabled(env)) return false; let hostUrl; try { hostUrl = new URL(`http://${host}`); @@ -23,12 +39,16 @@ export function isLocalMcpRequest({ remoteAddress, host, origin }) { return false; } if (!LOOPBACK_HOSTS.has(hostUrl.hostname)) return false; + // The Host names the port the request reached, so tools are never pointed + // at another local service. + if (localPort != null && (hostUrl.port || '80') !== String(localPort)) + return false; if (origin == null) return true; try { const originUrl = new URL(origin); return ( ['http:', 'https:'].includes(originUrl.protocol) && - LOOPBACK_HOSTS.has(originUrl.hostname) + originUrl.host === hostUrl.host ); } catch { return false; @@ -49,8 +69,11 @@ export function localMcpPlugin({ createServer = createLocalMcpServer } = {}) { if ( !isLocalMcpRequest({ remoteAddress: req.socket?.remoteAddress, + localPort: req.socket?.localPort, host, origin: req.headers.origin, + headers: req.headers, + env: process.env, }) ) { res.writeHead(403, { diff --git a/src/keySetupCore.mjs b/src/keySetupCore.mjs index e8bfef92c..a98aadc4e 100644 --- a/src/keySetupCore.mjs +++ b/src/keySetupCore.mjs @@ -184,6 +184,48 @@ export function parseWindowsUserSid(stdout) { : null; } +/** Headers that reverse proxies, tunnels and CDNs add to a request. */ +export const PROXY_SIGNAL_HEADERS = Object.freeze([ + 'forwarded', + 'via', + 'x-forwarded-for', + 'x-forwarded-host', + 'x-forwarded-port', + 'x-forwarded-proto', + 'x-real-ip', + 'cf-connecting-ip', + 'cf-ray', +]); + +/** + * Whether a request carries reverse-proxy or CDN forwarding headers, so it + * did not originate on this machine whatever its socket says. `headers` is + * keyed by lower-case header name. + */ +export function hasProxySignals(headers = {}) { + return PROXY_SIGNAL_HEADERS.some( + (name) => String(headers[name] || '').trim() !== '', + ); +} + +/** + * Whether the launcher has sharing on. Every sharing signal the launcher + * recognizes (scripts/pinokio-preflight.mjs) counts, so the two sets cannot + * drift apart. One DELIBERATE divergence: preflight is a boot check that + * treats an empty PINOKIO_SHARE_VAR as sharing-on (fail closed before Start), + * but here an empty/unset value is the NORMAL git-clone and Pinokio state, so + * a bare/sentinel value is not sharing; only a real tunnel var is. + */ +export function isSharingEnabled(env = {}) { + const shareVar = String(env.PINOKIO_SHARE_VAR ?? '').trim(); + return ( + ['PINOKIO_SHARE_CLOUDFLARE', 'PINOKIO_SHARE_LOCAL'].some((name) => + /^(1|true)$/i.test(String(env[name] || '').trim()), + ) || + (shareVar !== '' && shareVar !== '__gev_sharing_disabled__') + ); +} + /** * The admission gate for the Provider Settings endpoints — pure, exported so * every refusal below is pinned by a unit assertion rather than a review note. @@ -219,20 +261,7 @@ export function admitKeySetupRequest({ // on this machine, whatever its socket says. Refuse them outright as defense // in depth — the shipped tunnel (Pinokio) is force-closed at boot, so these // only appear when someone has deliberately fronted the dev server. - const PROXY_SIGNALS = [ - 'forwarded', - 'via', - 'x-forwarded-for', - 'x-forwarded-host', - 'x-forwarded-port', - 'x-forwarded-proto', - 'x-real-ip', - 'cf-connecting-ip', - 'cf-ray', - ]; - if ( - PROXY_SIGNALS.some((name) => String(proxyHeaders[name] || '').trim() !== '') - ) { + if (hasProxySignals(proxyHeaders)) { return { ok: false, status: 403, @@ -249,13 +278,7 @@ export function admitKeySetupRequest({ // tunnel var is. This is defense in depth regardless: the loopback+Host checks // below independently refuse LAN/tunnel traffic, and under Pinokio the launcher // refuses to boot at all when sharing is genuinely on. - const shareVar = String(env.PINOKIO_SHARE_VAR ?? '').trim(); - const sharingEnabled = - ['PINOKIO_SHARE_CLOUDFLARE', 'PINOKIO_SHARE_LOCAL'].some((name) => - /^(1|true)$/i.test(String(env[name] || '').trim()), - ) || - (shareVar !== '' && shareVar !== '__gev_sharing_disabled__'); - if (sharingEnabled) { + if (isSharingEnabled(env)) { return { ok: false, status: 403, diff --git a/src/tools/localRoute.test.mjs b/src/tools/localRoute.test.mjs index ed4129885..9fe113c86 100644 --- a/src/tools/localRoute.test.mjs +++ b/src/tools/localRoute.test.mjs @@ -3,15 +3,23 @@ import { createServer, request as httpRequest } from 'node:http'; import test from 'node:test'; import { isLocalMcpRequest, localMcpPlugin } from '../../server/mcp/plugin.js'; -test('only loopback connections with loopback hosts and origins are local', () => { - const local = { remoteAddress: '127.0.0.1', host: 'localhost:4173' }; +test('only direct loopback connections to this server are local', () => { + const local = { + remoteAddress: '127.0.0.1', + localPort: 4173, + host: 'localhost:4173', + }; assert.equal(isLocalMcpRequest(local), true); assert.equal( isLocalMcpRequest({ ...local, remoteAddress: '::1', host: '[::1]:4173' }), true, ); assert.equal( - isLocalMcpRequest({ ...local, origin: 'http://127.0.0.1:5173' }), + isLocalMcpRequest({ ...local, origin: 'http://localhost:4173' }), + true, + ); + assert.equal( + isLocalMcpRequest({ ...local, env: { PINOKIO_SHARE_LOCAL: 'false' } }), true, ); for (const request of [ @@ -19,9 +27,20 @@ test('only loopback connections with loopback hosts and origins are local', () = { ...local, host: 'attacker.example:4173' }, { ...local, host: 'localhost.attacker.example' }, { ...local, host: '' }, + // Another local port, as the Host or as the page's origin. + { ...local, host: 'localhost:9000' }, + { ...local, localPort: 80 }, + { ...local, origin: 'http://127.0.0.1:5173' }, + { ...local, origin: 'http://localhost:5173' }, { ...local, origin: 'https://attacker.example' }, { ...local, origin: 'null' }, { ...local, origin: 'file:///tmp/page.html' }, + // Proxied or shared: not from this machine, whatever the socket says. + { ...local, headers: { 'x-forwarded-for': '203.0.113.9' } }, + { ...local, headers: { forwarded: 'for=203.0.113.9' } }, + { ...local, headers: { 'cf-connecting-ip': '203.0.113.9' } }, + { ...local, env: { PINOKIO_SHARE_LOCAL: 'true' } }, + { ...local, env: { PINOKIO_SHARE_VAR: 'GEV_SHARE_URL' } }, ]) assert.equal(isLocalMcpRequest(request), false, JSON.stringify(request)); }); @@ -111,6 +130,18 @@ test('the /mcp route answers local MCP requests and refuses others', async (t) = (await post({}, { Host: `localhost.attacker.example:${port}` })).status, 403, ); + // A Host naming another local port, a page on another port, and a request + // a proxy forwarded are all refused, and no server is made for them. + assert.equal((await post({}, { Host: `localhost:${port + 1}` })).status, 403); + assert.equal( + (await post({}, { Origin: `http://localhost:${port + 1}` })).status, + 403, + ); + assert.equal( + (await post({}, { 'X-Forwarded-For': '203.0.113.9' })).status, + 403, + ); + assert.deepEqual(created, [`http://localhost:${port}`]); assert.equal((await post('x'.repeat(1024 * 1024 + 1))).status, 413); assert.equal( (