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); + }); +});