diff --git a/CHANGELOG.md b/CHANGELOG.md index 51cbda5e3..747c6227d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index 730586df9..ee3bfbdb2 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -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, diff --git a/scripts/format-scope.json b/scripts/format-scope.json index 7062704a7..693263205 100644 --- a/scripts/format-scope.json +++ b/scripts/format-scope.json @@ -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", diff --git a/server/mcp/panelKey.js b/server/mcp/panelKey.js new file mode 100644 index 000000000..8b34657f7 --- /dev/null +++ b/server/mcp/panelKey.js @@ -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 }); + } +} diff --git a/server/mcp/server.js b/server/mcp/server.js index 0926c865e..3c69dc584 100644 --- a/server/mcp/server.js +++ b/server/mcp/server.js @@ -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({ diff --git a/server/mcp/stdio.js b/server/mcp/stdio.js index 26f1da8c4..dd0f3fee2 100644 --- a/server/mcp/stdio.js +++ b/server/mcp/stdio.js @@ -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}`), diff --git a/src/tools/mcpPanelKey.test.mjs b/src/tools/mcpPanelKey.test.mjs new file mode 100644 index 000000000..d70fdb3e2 --- /dev/null +++ b/src/tools/mcpPanelKey.test.mjs @@ -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', + ]); +});