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) <[email protected]>
This commit is contained in:
Sameh KhamisandClaude Opus 5.5 committed 2026-10-02 19:47:37 -07:00
1 parent f7d3db5875
commit fec93aa817
5 files changed
+113 -32

No files matched your search

+44 -21
View File
@@ -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,
+34 -3
View File
@@ -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(
(