From 507001d5099b19fb5b14bcefb5135e158a34ab62 Mon Sep 17 00:00:00 2001 From: Nariman Jelveh Date: Sat, 25 Jul 2026 17:49:05 -0700 Subject: [PATCH] Identify permission requesters by origin only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The request-permission action took `app_uid` straight from the query string and used it as the grant target whenever the origin was absent or unresolvable. Now that /auth/grant-user-app accepts `origin` and prefers `app_uid` when both arrive, the displayed identity and the grant target could diverge; with only `app_uid` in the URL the dialog rendered with an empty name, so a link could produce a bare "Allow" prompt for an unnamed requester. Resolve the uid from the origin alone, and let the server resolve it from that same origin when the client lookup fails. Also give the no-gesture consent popup a unique window name. UI.js does this on the direct path because window.open() reuses a window with a matching name, but the PuterDialog fallback opened under the default 'Puter' — the same name sign-in uses, so a consent click could navigate an in-progress sign-in popup away. And guard the IPC responder: an app that closes its own window while the dialog is up leaves target_iframe.contentWindow null. --- src/gui/src/IPC.js | 6 ++++-- src/gui/src/initgui.js | 18 ++++++++++-------- src/puter-js/src/modules/PuterDialog.js | 19 +++++++++++++++++-- src/puter-js/src/modules/UI.js | 2 ++ .../tests/e2e/specs/requestPermission.spec.js | 17 +++++++++++++++++ 5 files changed, 50 insertions(+), 12 deletions(-) diff --git a/src/gui/src/IPC.js b/src/gui/src/IPC.js index 7991bd817..2475e9582 100644 --- a/src/gui/src/IPC.js +++ b/src/gui/src/IPC.js @@ -1314,8 +1314,10 @@ const ipc_listener = async (event, handled) => { else if ( event.data.msg === 'requestPermission' ) { // Always respond, even on validation/auth failure, so the SDK's // promise settles instead of hanging forever. + // The app can close its own window while the dialog is up, which + // tears down the iframe — posting into it must not throw. const respond = (granted) => { - target_iframe.contentWindow.postMessage({ + target_iframe?.contentWindow?.postMessage({ msg: 'permissionGranted', granted: granted, original_msg_id: msg_id, @@ -1351,7 +1353,7 @@ const ipc_listener = async (event, handled) => { // report the user's decision to the requester window respond(granted === true); - $(target_iframe).get(0).focus({ preventScroll: true }); + $(target_iframe).get(0)?.focus({ preventScroll: true }); } //-------------------------------------------------------- // showFontPicker diff --git a/src/gui/src/initgui.js b/src/gui/src/initgui.js index a02fa8b6b..629509499 100644 --- a/src/gui/src/initgui.js +++ b/src/gui/src/initgui.js @@ -465,15 +465,17 @@ const postAuthActions = async (action) => { // promise pending until the user closes it by hand. let granted = false; try { - // Identify the requesting app by its origin rather than trusting a - // caller-supplied uid, falling back to the query param otherwise. - let app_uid = window.url_query_params.get('app_uid') ?? undefined; + // The requesting app is identified by its origin only. A uid from + // the query string is never trusted: it is chosen by whoever + // opened this page, so honouring it would let a link grant a + // permission to an app the dialog never named. When the origin + // can't be resolved here, the uid is left unset and the server + // resolves it from the same origin the dialog displayed. + let app_uid; if ( origin ) { - try { - app_uid = window.host_app_uid ?? await window.getAppUIDFromOrigin(origin); - } catch (e) { - // Keep the query-param fallback. - } + app_uid = window.host_app_uid + ?? await window.getAppUIDFromOrigin(origin) + ?? undefined; } granted = await UIPermissionDialog({ diff --git a/src/puter-js/src/modules/PuterDialog.js b/src/puter-js/src/modules/PuterDialog.js index 1236366cd..c97ecc4ca 100644 --- a/src/puter-js/src/modules/PuterDialog.js +++ b/src/puter-js/src/modules/PuterDialog.js @@ -16,6 +16,10 @@ class PuterDialog extends (globalThis.HTMLElement || Object) { // It will fall b * implicit-auth URL), skips the `puter.token` message handling and * `puterAuthState` bookkeeping (the caller owns the auth result), and * reports cancellation through `options.onCancel`. + * @param {string} [options.popupName] - Window name for the popup. Give + * each concurrent launcher a distinct name: `window.open()` reuses a + * window that already carries the requested name, so sharing one name + * navigates (and hijacks) a popup another pending flow is waiting on. * @param {Function} [options.onLaunch] - Called with the opened popup * window (or null if the browser blocked it) right after launch. * @param {Function} [options.onCancel] - Called when the user dismisses @@ -528,7 +532,7 @@ class PuterDialog extends (globalThis.HTMLElement || Object) { // It will fall b // Wire the "Continue" button to open the auth popup. Opening here is // safe from being popup-blocked because it happens inside a click. this.shadowRoot.querySelector('#launch-auth-popup')?.addEventListener('click', () => { - const popup = openAuthPopup(this.#popupURL()); + const popup = this.#openPopup(); // Pinned as the expected event.source in messageListener. this.authPopup = popup; @@ -553,9 +557,20 @@ class PuterDialog extends (globalThis.HTMLElement || Object) { // It will fall b this.shadowRoot.querySelector('.close-btn')?.addEventListener('click', this.cancelListener); } + /** + * Opens the popup under the caller's window name when one was given, so + * concurrent launchers don't reuse (and steal) each other's window. + * @returns {Window|null} + */ + #openPopup () { + return this.options.popupName + ? openAuthPopup(this.#popupURL(), this.options.popupName) + : openAuthPopup(this.#popupURL()); + } + open () { if ( hasUserActivation() ) { - const popup = openAuthPopup(this.#popupURL()); + const popup = this.#openPopup(); // Pinned as the expected event.source in messageListener. this.authPopup = popup; if ( this.options.popupURL && typeof this.options.onLaunch === 'function' ) { diff --git a/src/puter-js/src/modules/UI.js b/src/puter-js/src/modules/UI.js index d8671e287..6b0e4edd0 100644 --- a/src/puter-js/src/modules/UI.js +++ b/src/puter-js/src/modules/UI.js @@ -1245,6 +1245,8 @@ class UI extends EventListener { // the gesture the browser requires. const dialog = new PuterDialog(() => {}, () => {}, { popupURL: url, + // Same unique-name reasoning as the direct path above. + popupName: `puter-permission-${msg_id}`, onLaunch: (popup) => watchPopup(popup), onCancel: () => settle(false), }); diff --git a/src/puter-js/tests/e2e/specs/requestPermission.spec.js b/src/puter-js/tests/e2e/specs/requestPermission.spec.js index ca942279c..e49d391fa 100644 --- a/src/puter-js/tests/e2e/specs/requestPermission.spec.js +++ b/src/puter-js/tests/e2e/specs/requestPermission.spec.js @@ -209,3 +209,20 @@ test.describe('puter.ui.requestPermission (env=web popup)', () => { await expect(page.locator('#log [data-entry="perm:driver:false"]')).toBeVisible(); }); }); + +test.describe('request-permission action hardening', () => { + test('an app_uid in the URL never produces a prompt on its own', async ({ page }) => { + // The uid identifies who receives the grant, so it must come from the + // requesting origin — never from the link. A link carrying only a uid + // would otherwise prompt for an unnamed requester and grant to an app + // the dialog never showed the user. + await page.goto( + '/action/request-permission?permission=driver%3Aputer-image-generation%3Agenerate' + + '&app_uid=app-00000000-0000-4000-8000-000000000000', + ); + await page.waitForFunction(() => !!window.puter?.authToken, null, { timeout: 60_000 }); + // Give the post-auth action a chance to run before asserting absence. + await page.locator('.desktop').waitFor({ timeout: 60_000 }); + await expect(page.locator('dialog.perm-dialog')).toHaveCount(0); + }); +});