From 06062d2e8352a205fa634970d260ab0f5de97f04 Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Tue, 18 Aug 2026 12:54:29 +0200 Subject: fix(client): three defects a real desktop found in ten minutes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All three came from the operator running the application on Ubuntu GNOME. None would have been found by anything already in the suite. **A second copy of the hub address.** `keyderive.js` carried `const HUB = '' // same origin` — true of a page the hub served, false of one loaded from a package, where the origin is `app://meshbay` and `/v1/users/register` resolves against the application's own protocol handler. **Sign-up and sign-in, the first two things anybody does, failed with "Not found."** The seam was changed in `app.js` and in the signalling call and this was missed: the same shape as the duplicate `MNP_VERSION` in `protocol.py`, a second copy of a constant that is harmless until the context changes. `test_hub_address_seam.py` refuses any file that decides where the hub is, and any `fetch('/v1/…')` relative to the page origin. **A window handler reading a variable another path reassigns.** Changing the hub closes one window and opens another; `closed` arrives *after* the replacement is assigned, so the outgoing window nulled the reference to the incoming one and its `ready-to-show` crashed on it — a modal "A JavaScript error occurred in the main process". Every handler now belongs to the window it was created with. The CDP test wrote `config.json` in advance, so it never took the one path that creates a second window; it does now, starting from an empty user-data dir. **A first run that could not be undone.** The hub address was accepted on anything URL-shaped and there was no way to change it afterwards — the prompt only appears when none is set, so a typo meant editing JSON by hand. `https` typed at a hub speaking `http` produced `TypeError: fetch failed`, which names nothing. Now: the address is probed before being written, failures say which URL and why ("does not speak https. If this hub is on your own machine, it is probably http"), Settings can change it, and Electron's "Error invoking remote method" wrapper is stripped from what a person reads. Verified on the operator's desktop: **safeStorage really uses the GNOME keyring** — Settings reports `gnome-libsecret`, and `secrets.bin` is written 0600 with Chromium's `v11` prefix, the marker for keyring-backed encryption (the fixed-key fallback writes `v10`). Headless, the same code reports `unavailable` and refuses to store rather than downgrading in silence, which is now explained in Settings instead of shown as a bare word. Unrelated but found while testing: `test_locales.py` assigned to `globalThis.navigator`, which is read-only from Node 22. The client's build already requires Node 22+, so the first CI machine configured for it would have failed these tests for no visible reason. 809 tests pass on Node 18 and Node 24. Co-Authored-By: Claude Opus 5 --- packages/meshbay-client/src/main.js | 115 +++++++++++++++++++++++++++++------- 1 file changed, 95 insertions(+), 20 deletions(-) (limited to 'packages/meshbay-client/src') diff --git a/packages/meshbay-client/src/main.js b/packages/meshbay-client/src/main.js index 82dfec9..4bc6392 100644 --- a/packages/meshbay-client/src/main.js +++ b/packages/meshbay-client/src/main.js @@ -256,7 +256,7 @@ function createWindow() { width: 1200, height: 800, }; - mainWindow = new BrowserWindow({ + const win = new BrowserWindow({ ...bounds, minWidth: 320, show: false, @@ -282,7 +282,7 @@ function createWindow() { }, }); - mainWindow.once('ready-to-show', () => mainWindow.show()); + win.once('ready-to-show', () => win.show()); // The hub must never become the document origin. Anything that would navigate // away from the packaged interface is refused, and an external link opens in @@ -290,26 +290,35 @@ function createWindow() { const isOurs = (target) => { try { return new URL(target).protocol === `${SCHEME}:`; } catch { return false; } }; - mainWindow.webContents.on('will-navigate', (event, target) => { + win.webContents.on('will-navigate', (event, target) => { if (!isOurs(target)) event.preventDefault(); }); - mainWindow.webContents.setWindowOpenHandler(({ url }) => { + win.webContents.setWindowOpenHandler(({ url }) => { if (/^https?:$/.test(new URL(url).protocol)) shell.openExternal(url); return { action: 'deny' }; }); // Nothing in this application needs a camera, a microphone or a location. - mainWindow.webContents.session.setPermissionRequestHandler( + win.webContents.session.setPermissionRequestHandler( (_wc, _permission, callback) => callback(false)); - mainWindow.on('close', () => { - if (!mainWindow.isMinimized() && !mainWindow.isFullScreen()) { - config = { ...config, window: mainWindow.getBounds() }; + win.on('close', () => { + if (!win.isMinimized() && !win.isFullScreen()) { + config = { ...config, window: win.getBounds() }; writeConfig(config); } }); - mainWindow.on('closed', () => { mainWindow = null; }); - mainWindow.loadURL(`${SCHEME}://meshbay/index.html`); + // `win`, not `mainWindow`. Changing the hub closes one window and opens + // another, and `closed` arrives *after* the replacement has been assigned — + // so a handler reading the module variable nulls out the new window, and the + // new window's `ready-to-show` then crashes on it. Every handler here belongs + // to the window it was created with, and only the current one may clear the + // reference. + win.on('closed', () => { if (mainWindow === win) mainWindow = null; }); + + mainWindow = win; + win.loadURL(`${SCHEME}://meshbay/index.html`); + return win; } // ── Bridge ────────────────────────────────────────────────────────────────── @@ -320,7 +329,7 @@ function createWindow() { // input, so it is treated as hostile even though it is our own code. function registerBridge() { - ipcMain.handle('hub:set', (_e, base) => { + ipcMain.handle('hub:set', async (_e, base) => { const url = String(base || '').trim().replace(/\/+$/, ''); if (url && !/^https:\/\//.test(url) && !/^http:\/\/(localhost|127\.)/.test(url)) { // http is allowed only to a loopback address, for someone running a hub @@ -328,15 +337,35 @@ function registerBridge() { // the wire in clear. throw new Error('The hub address must be https'); } + // Ask the hub whether it is one, before writing the address down. + // + // Without this the first-run screen accepts anything shaped like a URL and + // the application is then broken with no way back — there was no way to + // change the hub once it was set, so a typo meant editing a JSON file by + // hand. `https` typed at an `http` hub is the obvious case and it fails + // with a TLS error that says nothing to anyone. + let version; + try { + const probe = await fetch(`${url}/v1/hub/version`, + { signal: AbortSignal.timeout(10000) }); + if (!probe.ok) throw new Error(`answered ${probe.status}`); + version = await probe.json(); + if (!version || !version.hub) throw new Error('did not answer as a hub'); + } catch (e) { + throw new Error(describeUnreachable(url, e)); + } + config = { ...config, hubBase: url }; writeConfig(config); // The renderer reads the address from a process argument, so the window has // to be rebuilt for a change to take. Reloading in place would leave the // interface talking to the old hub with no sign of it. - if (mainWindow) { - mainWindow.close(); - createWindow(); - } + // Build the replacement first, then close the old one: the new window is + // what `mainWindow` points at, so the outgoing window's `closed` handler + // finds a reference that is no longer its own and leaves it alone. + const outgoing = mainWindow; + createWindow(); + if (outgoing && !outgoing.isDestroyed()) outgoing.close(); return url; }); @@ -360,11 +389,18 @@ function registerBridge() { if (!base || target.origin !== base.origin) { throw new Error('Refused: not this hub'); } - const response = await fetch(target, { - method: (init && init.method) || 'GET', - headers: (init && init.headers) || {}, - body: (init && init.body) || undefined, - }); + let response; + try { + response = await fetch(target, { + method: (init && init.method) || 'GET', + headers: (init && init.headers) || {}, + body: (init && init.body) || undefined, + }); + } catch (e) { + // Node's fetch says "fetch failed" for everything from a refused + // connection to a TLS mismatch, which tells a person nothing at all. + throw new Error(describeUnreachable(target.origin, e)); + } return { status: response.status, ok: response.ok, @@ -373,6 +409,16 @@ function registerBridge() { }; }); + ipcMain.handle('hub:probe', async (_e, url) => { + const target = String(url || config.hubBase || '').replace(/\/+$/, ''); + if (!target) return null; + try { + const r = await fetch(`${target}/v1/hub/version`, + { signal: AbortSignal.timeout(10000) }); + return r.ok ? await r.json() : null; + } catch { return null; } + }); + ipcMain.handle('device:ensure', () => ensureDeviceKey()); ipcMain.handle('device:public', () => { const key = deviceKey(); @@ -446,6 +492,35 @@ function registerBridge() { }); } +/** + * Why the hub could not be reached, in words somebody can act on. + * + * `TypeError: fetch failed` is what Node says for a refused connection, a DNS + * failure and a TLS mismatch alike. The most common mistake by far is `https` + * typed at a hub speaking plain `http`, so that one is named outright. + */ +function describeUnreachable(url, error) { + const cause = (error && error.cause) || {}; + const code = cause.code || ''; + const detail = cause.message || error.message || String(error); + + if (/^https:/.test(url) && + (code === 'ECONNRESET' || /wrong version|SSL|TLS|EPROTO/i.test(detail))) { + return `${url} does not speak https. If this hub is on your own machine, ` + + `it is probably http — try http:// instead.`; + } + if (code === 'ECONNREFUSED') { + return `Nothing is listening at ${url}. Is the hub running?`; + } + if (code === 'ENOTFOUND' || code === 'EAI_AGAIN') { + return `${url} could not be found. Check the address.`; + } + if (error && error.name === 'TimeoutError') { + return `${url} did not answer in time.`; + } + return `Could not reach ${url}: ${detail}`; +} + // ── Lifecycle ─────────────────────────────────────────────────────────────── // One instance. Two would fight over the config file and the secrets blob, and -- cgit v1.2.3