diff options
| -rw-r--r-- | CLAUDE.md | 8 | ||||
| -rw-r--r-- | docs/desktop-client-v1.md | 13 | ||||
| -rw-r--r-- | packages/meshbay-client/src/main.js | 65 | ||||
| -rw-r--r-- | packages/meshbay-hub/src/meshbay_hub/static/app.js | 2 | ||||
| -rw-r--r-- | packages/meshbay-hub/tests/test_desktop_shell.py | 65 |
5 files changed, 136 insertions, 17 deletions
@@ -402,6 +402,14 @@ anything that assumes one key per person. for it would have failed these tests for no visible reason. Use `Object.defineProperty`; the suite is now green on 18 and 24 +- **A refusal that never rejects.** Denying Chromium's `fullscreen` permission + does not make `requestFullscreen()` throw — the promise never settles. The + deny-everything handler was written from a true sentence ("nothing here needs + a camera") and quietly broke watching a film full-screen, with no error + anywhere to lead back to it. Prefer enumerating what is *granted*: the list + is short, and the next thing Chromium invents arrives refused rather than + silently allowed + - **A fallback chain reaches its floor silently.** `_openDownloadTarget` tries a granted folder, then a service worker, then "collect it in memory and hand the browser a blob". In the desktop application the first two do not exist — diff --git a/docs/desktop-client-v1.md b/docs/desktop-client-v1.md index 23287dd..c0bbb90 100644 --- a/docs/desktop-client-v1.md +++ b/docs/desktop-client-v1.md @@ -181,7 +181,18 @@ Electron it is nearly all of it. ### 3.1 What running it changed -Three of the statements above were wrong, and only launching the application found them. +Four of the statements above were wrong, and only launching the application found them. + +**"Nothing here needs a camera, a microphone or a location" was true, and the handler +written from it was still wrong.** Denying every permission also denied `fullscreen`, and +Chromium's own video controls ask for it — so a film could not be watched full-screen. +What makes this worth recording rather than just fixing: **a denied `fullscreen` does not +reject.** `requestFullscreen()` returns a promise that never settles. No error, no console +message, nothing in the renderer that names a permission; the button simply does nothing, +and the operator reported it as "impossible to go full-screen" with no lead to follow. The +probe reported `NEVER SETTLED` while the main process logged `PERMISSION ASKED: +fullscreen`, which is what tied the two ends together. The handler now enumerates what is +*granted* — one entry — so anything Chromium adds later still arrives refused. **The CSP cannot live in a `<meta>` tag.** `frame-ancestors` is ignored there — Chromium says so in the console — so a policy carrying it has one directive that silently does diff --git a/packages/meshbay-client/src/main.js b/packages/meshbay-client/src/main.js index f97be8e..8e11f1f 100644 --- a/packages/meshbay-client/src/main.js +++ b/packages/meshbay-client/src/main.js @@ -25,6 +25,7 @@ const { app, BrowserWindow, dialog, ipcMain, protocol, safeStorage, shell } = const crypto = require('node:crypto'); const fs = require('node:fs'); const fsp = require('node:fs/promises'); +const os = require('node:os'); const path = require('node:path'); const { pathToFileURL } = require('node:url'); @@ -247,6 +248,10 @@ function publicKeyB64(privateKey) { return der.subarray(der.length - 32).toString('base64'); } +// Everything the interface is allowed to ask Chromium for. Watching a film +// full-screen is the whole list. +const GRANTED_PERMISSIONS = new Set(['fullscreen']); + // ── Window ────────────────────────────────────────────────────────────────── let mainWindow = null; @@ -297,9 +302,23 @@ function createWindow() { 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. + // Deny by default, with one exception, and the exception is the point of the + // comment. A blanket `callback(false)` here is what stopped a film going + // fullscreen: Chromium's own video controls ask for `fullscreen`, and a + // **denial does not reject** — `requestFullscreen()` returns a promise that + // never settles, so the button simply does nothing and there is no error + // anywhere to find. Measured, not deduced: the probe reported `NEVER SETTLED` + // and the main process logged `PERMISSION ASKED: fullscreen`. + // + // So: enumerate what is granted rather than what is refused. A camera, a + // microphone, a location, notifications and MIDI are all still refused, and + // anything Chromium adds later arrives refused rather than quietly allowed. win.webContents.session.setPermissionRequestHandler( - (_wc, _permission, callback) => callback(false)); + (_wc, permission, callback) => callback(GRANTED_PERMISSIONS.has(permission))); + // `Permissions.query` takes the other handler; same answer, or the two can + // disagree about what the page is allowed to do. + win.webContents.session.setPermissionCheckHandler( + (_wc, permission) => GRANTED_PERMISSIONS.has(permission)); win.on('close', () => { if (!win.isMinimized() && !win.isFullScreen()) { @@ -496,12 +515,27 @@ function registerBridge() { return config.downloadDir; }); - ipcMain.handle('folder:get', () => { + // Where downloads land when nobody has chosen anywhere. A browser does not + // make you pick a folder before it will save a file, and neither should this + // — "save automatically" that opens a dialog is not automatic. + function defaultDownloadDir() { + try { return app.getPath('downloads'); } catch { return os.homedir(); } + } + + function chosenDownloadDir() { const dir = config.downloadDir; // A folder that has been removed or unmounted is not a folder any more, // and saying so beats failing on the first chunk of a download. if (!dir) return null; try { return fs.statSync(dir).isDirectory() ? dir : null; } catch { return null; } + } + + ipcMain.handle('folder:get', () => { + const chosen = chosenDownloadDir(); + // `name` is what the settings row already renders, for the browser's + // directory handle as much as for this. `isDefault` is how it knows not to + // offer "forget" for a folder nobody chose. + return { name: chosen || defaultDownloadDir(), isDefault: !chosen }; }); ipcMain.handle('folder:forget', () => { @@ -512,22 +546,31 @@ function registerBridge() { ipcMain.handle('save:begin', async (_e, suggestedName, opts) => { const wanted = path.basename(String(suggestedName || 'download')); - const remembered = config.downloadDir; + const chosen = chosenDownloadDir(); let target = null; - // "Save automatically" means exactly that: no dialog, into the folder that - // was chosen once. - if (opts && opts.auto && remembered) { + // "Save automatically" means exactly that: no dialog. Into the chosen + // folder if there is one, otherwise the system's Downloads folder — the + // first version required a folder to have been picked first, so the very + // first automatic download opened a dialog, which is the one thing the + // setting says it will not do. + // + // The one case that still asks: a folder *was* chosen and has since gone. + // Redirecting those files somewhere else without saying so is worse than a + // dialog — someone who picked an external drive wants to know it is not + // there, not to find the film in their home directory a week later. + if (opts && opts.auto && !(config.downloadDir && !chosen)) { + const dir = chosen || defaultDownloadDir(); try { - if (fs.statSync(remembered).isDirectory()) { - target = path.join(remembered, freeName(remembered, wanted)); - } + fs.mkdirSync(dir, { recursive: true }); + target = path.join(dir, freeName(dir, wanted)); } catch { target = null; } } if (!target) { + const dir = chosen || defaultDownloadDir(); const result = await dialog.showSaveDialog(mainWindow, { - defaultPath: remembered ? path.join(remembered, wanted) : wanted, + defaultPath: path.join(dir, wanted), }); if (result.canceled || !result.filePath) return null; target = result.filePath; diff --git a/packages/meshbay-hub/src/meshbay_hub/static/app.js b/packages/meshbay-hub/src/meshbay_hub/static/app.js index f49d412..07fc781 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/app.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/app.js @@ -4096,7 +4096,7 @@ function SettingsPage({ user, theme, onThemeChange, groups }) { <button class="admin-btn" onClick=${pickFolder}> ${dlDir ? t('settings.dl_change') : t('settings.dl_choose')} </button> - ${dlDir && html` + ${dlDir && !dlDir.isDefault && html` <button class="btn-secondary" onClick=${async () => { if (platform.folder.available) await platform.folder.forget(); else await downloads.forgetDirectory(); diff --git a/packages/meshbay-hub/tests/test_desktop_shell.py b/packages/meshbay-hub/tests/test_desktop_shell.py index c4f5b08..0abdb68 100644 --- a/packages/meshbay-hub/tests/test_desktop_shell.py +++ b/packages/meshbay-hub/tests/test_desktop_shell.py @@ -13,6 +13,7 @@ Every assertion here corresponds to a sentence in `docs/desktop-client-v1.md` person with an installed client is what confirms the rest. """ +import re from pathlib import Path import pytest @@ -69,10 +70,40 @@ def test_navigation_away_from_the_package_is_refused(): assert "event.preventDefault()" in source -def test_no_permission_is_granted_to_the_page(): - """Nothing here needs a camera, a microphone or a location.""" - assert "setPermissionRequestHandler" in _main() - assert "callback(false)" in _main() +def _granted_permissions() -> set[str]: + """The allowlist, read out of the source rather than described here.""" + match = re.search(r"GRANTED_PERMISSIONS = new Set\(\[([^\]]*)\]\)", _main()) + assert match, "the permission allowlist is gone or was renamed" + return set(re.findall(r"'([^']+)'", match.group(1))) + + +def test_the_page_gets_no_camera_microphone_or_location(): + denied = {"media", "geolocation", "midi", "midiSysex", "notifications", + "pointerLock", "openExternal", "clipboard-read", "hid", "serial", + "usb", "idle-detection", "window-management"} + assert not (_granted_permissions() & denied) + + +def test_video_may_go_fullscreen(): + """ + The regression this replaced a blanket denial to fix, and the reason it is + worth a test: **a denied `fullscreen` does not reject.** Chromium's own + video controls ask for it, `requestFullscreen()` returns a promise that + never settles, and the button does nothing with no error raised anywhere. + Nothing observable says "permission" — so nothing would have led back here. + """ + assert "fullscreen" in _granted_permissions() + + +def test_both_permission_handlers_answer_from_the_same_list(): + """`Permissions.query` takes the check handler and a request takes the + other; two lists would eventually disagree about what the page may do.""" + source = _main() + for handler in ("setPermissionRequestHandler", "setPermissionCheckHandler"): + assert handler in source + after = source.split(handler, 1)[1][:400] + assert "GRANTED_PERMISSIONS" in after, ( + f"{handler} decides on its own rather than from the allowlist") # ── The custom scheme ─────────────────────────────────────────────────────── @@ -241,3 +272,29 @@ def test_the_packaged_page_loads_the_shared_modules(): "style.css", "argon2.min.js"): assert module in page, f"{module} is not loaded by the packaged page" assert "/a/" not in page, "the packaged page points at the hub's asset prefix" + + +# ── Where downloads land ──────────────────────────────────────────────────── + +def test_automatic_saving_never_opens_a_dialog_for_want_of_a_folder(): + """ + "Save automatically" opened a Save As dialog on the first download, because + the automatic path required a folder to have been chosen first and nobody + had chosen one. A browser does not ask before it will save a file; the + system Downloads folder is the answer when there is no other. + """ + source = _main() + begin = source.split("ipcMain.handle('save:begin'", 1)[1].split("ipcMain.handle", 1)[0] + assert "defaultDownloadDir()" in begin, ( + "the automatic path has no destination when no folder was chosen") + assert "app.getPath('downloads')" in source + + +def test_a_chosen_folder_that_has_gone_is_not_silently_replaced(): + """Someone who picked an external drive should be told it is not there, + not find the film in their home directory a week later.""" + source = _main() + begin = source.split("ipcMain.handle('save:begin'", 1)[1].split("ipcMain.handle", 1)[0] + assert "config.downloadDir && !chosen" in begin, ( + "a chosen-but-missing folder falls through to the default instead of asking") + assert "showSaveDialog" in begin |