mirror of
https://github.com/bilawalsidhu/gods-eye-view.git
synced 2026-11-05 10:53:59 +00:00
Merge pull request #958 from jibraaan/fix/mcp-shared-panel-key
fix(mcp): share one panel key across an install's stdio servers
This commit is contained in:
7 files changed
+350
-5
No files matched your search
@@ -63,6 +63,14 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
- Stdio servers from one install share a panel key so a page read from one
|
||||
process can make requests through another. Concurrent malformed-key repairs
|
||||
now select one winner; unavailable storage or a busy repair retains the
|
||||
logged per-process fallback. Thanks to [Jibran Tahir](https://github.com/jibraaan)
|
||||
for #958, [MarvinNL046](https://github.com/MarvinNL046) for the report and
|
||||
proposed diagnosis in #927, and [kvnloo](https://github.com/kvnloo) for identifying
|
||||
the repair race. Windows/Claude Desktop Cowork confirmation remains outstanding.
|
||||
|
||||
- Street Level: a street-level imagery layer modelled on the iD editor's photo
|
||||
overlay, with Mapillary as its first provider (free client token, CC BY-SA 4.0
|
||||
imagery). One right-rail panel holds a chip per provider, shared 360°/flat and
|
||||
|
||||
@@ -14,6 +14,19 @@ new views from the page that frames it; framing is off unless
|
||||
`GEV_EMBED_FRAME_ANCESTORS` allows the framing page. MCP leads with the tools that find
|
||||
what to show. See [tools and the MCP server](TOOLS.md).
|
||||
|
||||
Stdio processes from one install share the panel key in
|
||||
`.gev-cache/mcp-panel-key`, resolved from the checkout rather than the client's
|
||||
working directory. First creation is atomic; malformed-file repairs select one
|
||||
winner and concurrent processes re-read it. A busy repair waits at most two
|
||||
seconds before falling back. If the cache cannot be read/written or repair remains
|
||||
busy, stderr reports the fallback to a per-process key; cross-process panels can
|
||||
still fail in that mode. An abandoned `.repair-lock` is left intact: after
|
||||
confirming no repair process is running, remove it to allow repair on restart.
|
||||
HTTP MCP servers retain per-server keys. This key is a panel routing guard, not
|
||||
authentication. Large-response continuations remain process-local and require
|
||||
consistent tool-call routing. Two-process reproduction is covered locally;
|
||||
Windows/Claude Desktop Cowork confirmation remains outstanding (#927).
|
||||
|
||||
## Tools and local MCP server — October 1, 2026
|
||||
|
||||
`gods-eye-view/tools` defines queries that answer questions from the app's data,
|
||||
|
||||
@@ -940,6 +940,7 @@
|
||||
"src/tools/queries/queries.test.mjs",
|
||||
"src/tools/mcp/mcp.test.mjs",
|
||||
"src/tools/localServer.test.mjs",
|
||||
"src/tools/mcpPanelKey.test.mjs",
|
||||
"src/tools/queries/aviation.test.mjs",
|
||||
"src/tools/queries/space.test.mjs",
|
||||
"src/tools/queries/media.test.mjs",
|
||||
|
||||
@@ -0,0 +1,117 @@
|
||||
import fs from 'node:fs';
|
||||
import path from 'node:path';
|
||||
import { randomBytes, randomUUID } from 'node:crypto';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
|
||||
/** Where the install's panel key lives: beside the other local state, which
|
||||
* git ignores, at the checkout root rather than the client's working
|
||||
* directory (an MCP client may start the server anywhere). */
|
||||
export const DEFAULT_PANEL_KEY_FILE = fileURLToPath(
|
||||
new URL('../../.gev-cache/mcp-panel-key', import.meta.url),
|
||||
);
|
||||
|
||||
const KEY_PATTERN = /^[A-Za-z0-9_-]{43}$/;
|
||||
const newKey = () => randomBytes(32).toString('base64url');
|
||||
const notShared = (error) =>
|
||||
`panel key not shared (${error.code || error.message}); using a key of this process`;
|
||||
|
||||
function readKey(file) {
|
||||
const text = fs.readFileSync(file, 'utf8').trim();
|
||||
return KEY_PATTERN.test(text) ? text : null;
|
||||
}
|
||||
|
||||
/** Serialize malformed-file repair; ordinary first creation still uses link(). */
|
||||
function repairKey(file, tmp, key) {
|
||||
const lock = `${file}.repair-lock`;
|
||||
const deadline = Date.now() + 2000;
|
||||
const pause = new Int32Array(new SharedArrayBuffer(4));
|
||||
let fd;
|
||||
for (;;) {
|
||||
try {
|
||||
fd = fs.openSync(lock, 'wx', 0o600);
|
||||
break;
|
||||
} catch (error) {
|
||||
if (error.code !== 'EEXIST') throw error;
|
||||
// Another repair may already have published a usable key.
|
||||
const winner = readKey(file);
|
||||
if (winner) return winner;
|
||||
if (Date.now() >= deadline) {
|
||||
throw Object.assign(new Error('panel key repair is busy'), {
|
||||
code: 'ETIMEDOUT',
|
||||
});
|
||||
}
|
||||
Atomics.wait(pause, 0, 0, 10);
|
||||
}
|
||||
}
|
||||
try {
|
||||
// A repair completed between our earlier read and acquiring the lock.
|
||||
const winner = readKey(file);
|
||||
if (winner) return winner;
|
||||
fs.renameSync(tmp, file);
|
||||
return key;
|
||||
} finally {
|
||||
fs.closeSync(fd);
|
||||
fs.rmSync(lock, { force: true });
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The panel key every stdio server of this install shares.
|
||||
*
|
||||
* A client can start more than one server process for one connection: Claude
|
||||
* Desktop was reported to start two processes; serving the panel's page from
|
||||
* one and sending its `panel_request` calls to the other reproduces #927.
|
||||
* Per-process keys then mismatch, and the panel refuses the request.
|
||||
* The first process to run creates the key, without replacing one
|
||||
* another process created at the same moment; the rest read it. When the file
|
||||
* cannot be read or written, or repair stays busy for two seconds, the process
|
||||
* falls back to its own key, as every process did before.
|
||||
*
|
||||
* As before, the key keeps `panel_request` from clients that only list it; it
|
||||
* is not access control.
|
||||
*
|
||||
* @param {{ file?: string, log?: (line: string) => void }} [options]
|
||||
* @returns {string}
|
||||
*/
|
||||
export function sharedPanelKey({
|
||||
file = DEFAULT_PANEL_KEY_FILE,
|
||||
log = () => {},
|
||||
} = {}) {
|
||||
try {
|
||||
const existing = readKey(file);
|
||||
if (existing) return existing;
|
||||
// Present but not a key: replace it below.
|
||||
} catch (error) {
|
||||
if (error.code !== 'ENOENT') {
|
||||
log(notShared(error));
|
||||
return newKey();
|
||||
}
|
||||
}
|
||||
const key = newKey();
|
||||
let tmp = null;
|
||||
try {
|
||||
fs.mkdirSync(path.dirname(file), { recursive: true });
|
||||
tmp = path.join(
|
||||
path.dirname(file),
|
||||
`.${path.basename(file)}.${randomUUID().slice(0, 8)}.tmp`,
|
||||
);
|
||||
fs.writeFileSync(tmp, `${key}\n`, { flag: 'wx', mode: 0o600 });
|
||||
try {
|
||||
// link() publishes the whole file at once and fails if another process
|
||||
// got there first, so a reader never sees a half-written key and two
|
||||
// processes starting together cannot end up with different keys.
|
||||
fs.linkSync(tmp, file);
|
||||
return key;
|
||||
} catch (error) {
|
||||
if (error.code !== 'EEXIST') throw error;
|
||||
const winner = readKey(file);
|
||||
if (winner) return winner;
|
||||
return repairKey(file, tmp, key);
|
||||
}
|
||||
} catch (error) {
|
||||
log(notShared(error));
|
||||
return key;
|
||||
} finally {
|
||||
if (tmp) fs.rmSync(tmp, { force: true });
|
||||
}
|
||||
}
|
||||
@@ -27,14 +27,16 @@ const INSTRUCTIONS =
|
||||
export function createLocalMcpServer({
|
||||
apiBase = DEFAULT_API_BASE,
|
||||
fetchImpl,
|
||||
panelKey = randomBytes(32).toString('base64url'),
|
||||
} = {}) {
|
||||
const { version } = JSON.parse(
|
||||
readFileSync(new URL('../../package.json', import.meta.url), 'utf8'),
|
||||
);
|
||||
// Each server makes a new key for its panel. The panel's page carries it
|
||||
// and panel_request requires it; any client may read the page, so it keeps
|
||||
// the tool from clients that only list it, and is not access control.
|
||||
const panelKey = randomBytes(32).toString('base64url');
|
||||
// Unless given one, each server makes a new key for its panel. The panel's
|
||||
// page carries it and panel_request requires it; any client may read the
|
||||
// page, so it keeps the tool from clients that only list it, and is not
|
||||
// access control. The stdio entry passes the install's shared key
|
||||
// (panelKey.js), so separate processes of one client agree.
|
||||
return createMcpServer({
|
||||
catalog: catalogForSurface(
|
||||
composeCatalog({
|
||||
|
||||
+5
-1
@@ -11,6 +11,7 @@
|
||||
import { createInterface } from 'node:readline';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import { parseErrorResponse } from '../../src/tools/mcp/index.js';
|
||||
import { sharedPanelKey } from './panelKey.js';
|
||||
import { createLocalMcpServer } from './server.js';
|
||||
import { DEFAULT_API_BASE } from './services.js';
|
||||
|
||||
@@ -97,7 +98,10 @@ if (process.argv[1] && fileURLToPath(import.meta.url) === process.argv[1]) {
|
||||
console.error(
|
||||
`God's Eye View MCP server reading ${options.apiBase || DEFAULT_API_BASE}`,
|
||||
);
|
||||
await serveStdio(createLocalMcpServer(options), {
|
||||
// A client may start several of these for one connection; they share
|
||||
// the install's panel key so the panel works whichever one it reaches.
|
||||
const panelKey = sharedPanelKey({ log: (line) => console.error(line) });
|
||||
await serveStdio(createLocalMcpServer({ ...options, panelKey }), {
|
||||
input: process.stdin,
|
||||
output: process.stdout,
|
||||
log: (line) => console.error(`<- ${line}`),
|
||||
|
||||
@@ -0,0 +1,200 @@
|
||||
import assert from 'node:assert/strict';
|
||||
import test from 'node:test';
|
||||
import fs from 'node:fs';
|
||||
import os from 'node:os';
|
||||
import path from 'node:path';
|
||||
import { spawn } from 'node:child_process';
|
||||
import { sharedPanelKey } from '../../server/mcp/panelKey.js';
|
||||
import { createLocalMcpServer } from '../../server/mcp/server.js';
|
||||
|
||||
const PANEL_KEY_MODULE = new URL(
|
||||
'../../server/mcp/panelKey.js',
|
||||
import.meta.url,
|
||||
).href;
|
||||
|
||||
function tempKeyFile(t) {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gev-panel-key-'));
|
||||
t.after(() => fs.rmSync(dir, { recursive: true, force: true }));
|
||||
return path.join(dir, 'state', 'mcp-panel-key');
|
||||
}
|
||||
|
||||
const readPage = async (server) => {
|
||||
const page = await server.handle({
|
||||
jsonrpc: '2.0',
|
||||
id: 1,
|
||||
method: 'resources/read',
|
||||
params: { uri: 'ui://gods-eye-view/globe' },
|
||||
});
|
||||
return page.result.contents[0].text.match(/"panelKey":"([^"]+)"/)[1];
|
||||
};
|
||||
const panelRequest = (server, key) =>
|
||||
server.handle({
|
||||
jsonrpc: '2.0',
|
||||
id: 2,
|
||||
method: 'tools/call',
|
||||
params: {
|
||||
name: 'panel_request',
|
||||
arguments: { key, path: '/panel/index.html' },
|
||||
},
|
||||
});
|
||||
const serverWith = (panelKey) =>
|
||||
createLocalMcpServer({
|
||||
apiBase: 'http://127.0.0.1:5000',
|
||||
fetchImpl: async () => new Response('ok'),
|
||||
...(panelKey ? { panelKey } : {}),
|
||||
});
|
||||
|
||||
test('a panel page from one server process is refused by another without a shared key (#927)', async () => {
|
||||
const pageServer = serverWith();
|
||||
const requestServer = serverWith();
|
||||
const key = await readPage(pageServer);
|
||||
const refused = await panelRequest(requestServer, key);
|
||||
assert.equal(refused.result.isError, true);
|
||||
assert.match(
|
||||
refused.result.content[0].text,
|
||||
/Only the God's Eye View panel may make this request/,
|
||||
);
|
||||
});
|
||||
|
||||
test('servers sharing the install key accept each other’s panel page', async (t) => {
|
||||
const file = tempKeyFile(t);
|
||||
const pageServer = serverWith(sharedPanelKey({ file }));
|
||||
const requestServer = serverWith(sharedPanelKey({ file }));
|
||||
const key = await readPage(pageServer);
|
||||
const opened = await panelRequest(requestServer, key);
|
||||
assert.equal(opened.result.isError, false);
|
||||
});
|
||||
|
||||
test('the shared key is created once, kept, and private to the user', (t) => {
|
||||
const file = tempKeyFile(t);
|
||||
const first = sharedPanelKey({ file });
|
||||
assert.match(first, /^[A-Za-z0-9_-]{43}$/);
|
||||
assert.equal(sharedPanelKey({ file }), first);
|
||||
assert.equal(fs.readFileSync(file, 'utf8').trim(), first);
|
||||
if (process.platform !== 'win32')
|
||||
assert.equal(fs.statSync(file).mode & 0o777, 0o600);
|
||||
// No staging files are left beside it.
|
||||
assert.deepEqual(fs.readdirSync(path.dirname(file)), ['mcp-panel-key']);
|
||||
});
|
||||
|
||||
test('a malformed key file is replaced', (t) => {
|
||||
const file = tempKeyFile(t);
|
||||
fs.mkdirSync(path.dirname(file), { recursive: true });
|
||||
fs.writeFileSync(file, 'not a key\n');
|
||||
const key = sharedPanelKey({ file });
|
||||
assert.match(key, /^[A-Za-z0-9_-]{43}$/);
|
||||
assert.equal(sharedPanelKey({ file }), key);
|
||||
});
|
||||
|
||||
test('when the key cannot be stored, the process uses a key of its own', (t) => {
|
||||
const file = tempKeyFile(t);
|
||||
// A file where the directory should be makes every write fail.
|
||||
fs.writeFileSync(path.dirname(path.dirname(file)) + '/state', 'blocker');
|
||||
const logged = [];
|
||||
const a = sharedPanelKey({ file, log: (line) => logged.push(line) });
|
||||
const b = sharedPanelKey({ file, log: (line) => logged.push(line) });
|
||||
assert.match(a, /^[A-Za-z0-9_-]{43}$/);
|
||||
assert.notEqual(a, b);
|
||||
assert.equal(logged.length, 2);
|
||||
assert.match(logged[0], /panel key not shared/);
|
||||
});
|
||||
|
||||
test('processes starting at the same moment agree on one key', async (t) => {
|
||||
const file = tempKeyFile(t);
|
||||
const script =
|
||||
`const { sharedPanelKey } = await import(${JSON.stringify(PANEL_KEY_MODULE)});` +
|
||||
`process.stdout.write(sharedPanelKey({ file: ${JSON.stringify(file)} }));`;
|
||||
const run = () =>
|
||||
new Promise((resolve, reject) => {
|
||||
const child = spawn(
|
||||
process.execPath,
|
||||
['--input-type=module', '-e', script],
|
||||
{ stdio: ['ignore', 'pipe', 'inherit'] },
|
||||
);
|
||||
let out = '';
|
||||
child.stdout.on('data', (chunk) => (out += chunk));
|
||||
child.on('error', reject);
|
||||
child.on('close', (code) =>
|
||||
code === 0 ? resolve(out) : reject(new Error(`exit ${code}`)),
|
||||
);
|
||||
});
|
||||
const keys = await Promise.all(Array.from({ length: 8 }, run));
|
||||
assert.equal(new Set(keys).size, 1);
|
||||
assert.equal(fs.readFileSync(file, 'utf8').trim(), keys[0]);
|
||||
});
|
||||
|
||||
test('concurrent malformed-file repairs return the same persisted key', async (t) => {
|
||||
const file = tempKeyFile(t);
|
||||
fs.mkdirSync(path.dirname(file), { recursive: true });
|
||||
fs.writeFileSync(file, 'not a key\n');
|
||||
const script = `
|
||||
import fs from 'node:fs';
|
||||
const { sharedPanelKey } = await import(${JSON.stringify(PANEL_KEY_MODULE)});
|
||||
const [file, marker, other] = process.argv.slice(1);
|
||||
const original = fs.readFileSync;
|
||||
let reads = 0;
|
||||
fs.readFileSync = function (target, ...args) {
|
||||
const text = original.call(this, target, ...args);
|
||||
if (target === file && ++reads === 2) {
|
||||
// Both processes capture the actual malformed contents before either
|
||||
// can replace them. The old unchecked rename returns two different keys.
|
||||
fs.writeFileSync(marker, 'ready');
|
||||
const deadline = Date.now() + 5000;
|
||||
while (!fs.existsSync(other)) {
|
||||
if (Date.now() >= deadline) throw new Error('repair barrier timed out');
|
||||
Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 5);
|
||||
}
|
||||
}
|
||||
return text;
|
||||
};
|
||||
process.stdout.write(sharedPanelKey({ file }));
|
||||
`;
|
||||
const run = (index) =>
|
||||
new Promise((resolve, reject) => {
|
||||
const marker = (id) => path.join(path.dirname(file), `ready-${id}`);
|
||||
const child = spawn(
|
||||
process.execPath,
|
||||
[
|
||||
'--input-type=module',
|
||||
'-e',
|
||||
script,
|
||||
file,
|
||||
marker(index),
|
||||
marker(1 - index),
|
||||
],
|
||||
{ stdio: ['ignore', 'pipe', 'inherit'] },
|
||||
);
|
||||
t.after(() => child.kill());
|
||||
let out = '';
|
||||
child.stdout.on('data', (chunk) => (out += chunk));
|
||||
child.on('error', reject);
|
||||
child.on('close', (code) =>
|
||||
code === 0 ? resolve(out) : reject(new Error(`exit ${code}`)),
|
||||
);
|
||||
});
|
||||
const keys = await Promise.all([run(0), run(1)]);
|
||||
assert.equal(new Set(keys).size, 1);
|
||||
assert.equal(fs.readFileSync(file, 'utf8').trim(), keys[0]);
|
||||
assert.equal(fs.existsSync(`${file}.repair-lock`), false);
|
||||
assert.equal(
|
||||
fs.readdirSync(path.dirname(file)).some((name) => name.endsWith('.tmp')),
|
||||
false,
|
||||
);
|
||||
});
|
||||
|
||||
test('an unavailable repair lock falls back without hanging or removing its owner’s lock', (t) => {
|
||||
const file = tempKeyFile(t);
|
||||
fs.mkdirSync(path.dirname(file), { recursive: true });
|
||||
fs.writeFileSync(file, 'not a key\n');
|
||||
fs.writeFileSync(`${file}.repair-lock`, 'held');
|
||||
const logged = [];
|
||||
const key = sharedPanelKey({ file, log: (line) => logged.push(line) });
|
||||
assert.match(key, /^[A-Za-z0-9_-]{43}$/);
|
||||
assert.match(logged[0], /ETIMEDOUT/);
|
||||
assert.equal(fs.readFileSync(file, 'utf8'), 'not a key\n');
|
||||
assert.equal(fs.readFileSync(`${file}.repair-lock`, 'utf8'), 'held');
|
||||
assert.deepEqual(fs.readdirSync(path.dirname(file)).sort(), [
|
||||
'mcp-panel-key',
|
||||
'mcp-panel-key.repair-lock',
|
||||
]);
|
||||
});
|
||||
Reference in new issue
Block a user