From 026ae46530bce0deeb3a970f6b512df4273e09a3 Mon Sep 17 00:00:00 2001 From: Nariman Jelveh Date: Wed, 12 Aug 2026 17:35:54 -0700 Subject: [PATCH] fix: let an app reopen the feedback dialog after the user closes it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The abuse guard counted every dismissal that sent nothing as a strike against the app, so closing the dialog blocked the app's next showFeedbackDialog() for 10s, then 60s, then for the rest of the page's life. The key is the app uuid, so relaunching the app did not clear it, and a blocked call replies sent:false — the app just sees nothing happen. Key the guard off how fast the app comes back instead. A reopen within a second of the app's last dialog activity is machine-paced and gets the same escalating backoff; attempts made while backed off count as activity too, so a loop cannot wait out a tier and start from zero strikes. Any human-paced reopen clears the app's record, so the tiers are only reached by an app that reopens at machine speed several times running. One dialog at a time, and a successful send clearing the record, are unchanged. --- src/gui/src/IPC.js | 50 ++----- src/gui/src/util/feedbackDialogGuard.js | 119 ++++++++++++++++ src/gui/src/util/feedbackDialogGuard.test.js | 139 +++++++++++++++++++ 3 files changed, 270 insertions(+), 38 deletions(-) create mode 100644 src/gui/src/util/feedbackDialogGuard.js create mode 100644 src/gui/src/util/feedbackDialogGuard.test.js diff --git a/src/gui/src/IPC.js b/src/gui/src/IPC.js index 709f3a354..da125459a 100644 --- a/src/gui/src/IPC.js +++ b/src/gui/src/IPC.js @@ -39,41 +39,16 @@ import UINotification from './UI/UINotification.js'; import { PROCESS_IPC_ATTACHED } from './definitions.js'; import TeePromise from './util/TeePromise.js'; +import { createFeedbackDialogGuard } from './util/feedbackDialogGuard.js'; window.ipc_handlers = {}; // Abuse guard for the app-triggered feedback dialog (`showFeedbackDialog` -// handler below). The dialog is a full-viewport modal reachable from app IPC -// with no user-gesture requirement, so an unguarded loop of -// `showFeedbackDialog()` calls would cover the desktop — taskbar and the -// app's own close button included — forever. One dialog may be open at a -// time, and every dismissal that sent nothing backs off the app's next open -// (10s, then 60s, then blocked until the page reloads); a successful send -// resets the backoff. -const FEEDBACK_DISMISS_BACKOFF_MS = [10_000, 60_000, Infinity]; -let feedback_dialog_open = false; -const feedback_dialog_backoff = new Map(); // app uid/name -> { dismissals, until } +// handler below): one dialog at a time, and an app that reopens it at machine +// speed gets backed off. Closing the dialog costs the user nothing — the next +// human-paced open always goes through. See feedbackDialogGuard.js. +const feedback_dialog_guard = createFeedbackDialogGuard(); -const feedback_dialog_may_open = (app_key) => { - if ( feedback_dialog_open ) return false; - const entry = feedback_dialog_backoff.get(app_key); - return ! entry || Date.now() >= entry.until; -}; - -const record_feedback_dialog_outcome = (app_key, sent) => { - if ( sent ) { - feedback_dialog_backoff.delete(app_key); - return; - } - const dismissals = (feedback_dialog_backoff.get(app_key)?.dismissals ?? 0) + 1; - const backoff = FEEDBACK_DISMISS_BACKOFF_MS[ - Math.min(dismissals, FEEDBACK_DISMISS_BACKOFF_MS.length) - 1 - ]; - feedback_dialog_backoff.set(app_key, { - dismissals, - until: Date.now() + backoff, - }); -}; /** * In Puter, apps are loaded in iframes and communicate with the graphical user interface (GUI), and each other, using the postMessage API. * The following sets up an Inter-Process Messaging System between apps and the GUI that enables communication @@ -1440,17 +1415,17 @@ const ipc_listener = async (event, handled) => { }, '*'); }; - // Re-entry / dismissal-backoff guard (see the state at the top of - // this file). Checked before the auth gate too: for a signed-out - // user the gate opens a full-page signup window, which an - // unguarded loop could spam just as effectively as the dialog. + // Re-entry / reopen-rate guard (see the state at the top of this + // file). Checked before the auth gate too: for a signed-out user the + // gate opens a full-page signup window, which an unguarded loop could + // spam just as effectively as the dialog. const guard_key = app_uuid || app_name; - if ( ! feedback_dialog_may_open(guard_key) ) { + if ( ! feedback_dialog_guard.mayOpen(guard_key) ) { respond(false); return; } - feedback_dialog_open = true; + feedback_dialog_guard.markOpened(); let sent = false; try { // auth @@ -1484,8 +1459,7 @@ const ipc_listener = async (event, handled) => { respond(sent === true); $(target_iframe).get(0)?.focus({ preventScroll: true }); } finally { - feedback_dialog_open = false; - record_feedback_dialog_outcome(guard_key, sent); + feedback_dialog_guard.markClosed(guard_key, sent); } } //-------------------------------------------------------- diff --git a/src/gui/src/util/feedbackDialogGuard.js b/src/gui/src/util/feedbackDialogGuard.js new file mode 100644 index 000000000..25c1426b9 --- /dev/null +++ b/src/gui/src/util/feedbackDialogGuard.js @@ -0,0 +1,119 @@ +/* + * Copyright (C) 2024-present Puter Technologies Inc. + * + * This file is part of Puter. + * + * Puter is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +// A reopen this soon after the app's last dialog activity is a loop, not a +// person: a human has to notice the dialog is gone, find the app's "Send +// feedback" control and click it, which no click-driven flow does inside a +// second. +export const IMMEDIATE_REOPEN_MS = 1000; + +// What an app pays for each machine-speed reopen, in order. +export const BACKOFF_MS = [10_000, 60_000, Infinity]; + +/** + * Abuse guard for the app-triggered feedback dialog (`showFeedbackDialog`). + * + * The dialog is a full-viewport modal an app can open with no user gesture, so + * an unguarded loop of `showFeedbackDialog()` calls would cover the desktop — + * taskbar and the app's own close button included — forever. + * + * The guard keys off *how fast the app comes back*, never off how many times + * the user said no. Closing the dialog must not cost the user their next one: + * people close it by accident, or open it to see what it asks before they have + * anything to write, and the app's own "Send feedback" button has to keep + * working when they come back to it. + * + * So a reopen that lands within `IMMEDIATE_REOPEN_MS` of the app's last dialog + * activity — its last dismissal, or its last attempt while backed off — is + * refused and backs the app off (10s, then 60s, then blocked until the page + * reloads). Hammering the guard while it is closed counts as activity too, so + * waiting a backoff out is not a way around the escalation. Any human-paced + * reopen clears the app's record: the tiers are only ever reached by an app + * that reopens at machine speed several times over. + * + * One dialog may be open at a time, across all apps. + * + * @param {{ now?: () => number }} [deps] - `now` is injectable for tests. + */ +export const createFeedbackDialogGuard = ({ now = () => Date.now() } = {}) => { + let dialog_open = false; + // app uid/name -> { strikes, until, last_activity } + const records = new Map(); + + return { + /** + * May this app open the dialog right now? Records the attempt either + * way, so call it once per request and honor the answer. + * + * @param {string} key - App uid, or name when there is no uid. + * @returns {boolean} + */ + mayOpen (key) { + // Not the app's doing — another dialog is up — so this attempt + // earns no strike and does not count as activity. + if ( dialog_open ) return false; + + const record = records.get(key); + if ( ! record ) return true; + + const t = now(); + const quiet_for = t - record.last_activity; + record.last_activity = t; + + if ( t < record.until ) return false; + + if ( quiet_for > IMMEDIATE_REOPEN_MS ) { + records.delete(key); + return true; + } + + record.strikes += 1; + record.until = t + BACKOFF_MS[ + Math.min(record.strikes, BACKOFF_MS.length) - 1 + ]; + return false; + }, + + /** + * The dialog is up for this app (call only after `mayOpen` said yes). + */ + markOpened () { + dialog_open = true; + }, + + /** + * The dialog is down. A submitted message clears the app's record + * outright — an app whose users are actually sending feedback is not + * the app this guard is for. + * + * @param {string} key - The key passed to `mayOpen`. + * @param {boolean} sent - Did the user submit feedback? + */ + markClosed (key, sent) { + dialog_open = false; + if ( sent === true ) { + records.delete(key); + return; + } + const record = records.get(key) ?? { strikes: 0, until: 0 }; + record.last_activity = now(); + records.set(key, record); + }, + }; +}; diff --git a/src/gui/src/util/feedbackDialogGuard.test.js b/src/gui/src/util/feedbackDialogGuard.test.js new file mode 100644 index 000000000..c00e40217 --- /dev/null +++ b/src/gui/src/util/feedbackDialogGuard.test.js @@ -0,0 +1,139 @@ +/* + * Copyright (C) 2024-present Puter Technologies Inc. + * + * This file is part of Puter. + * + * Puter is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { describe, it, expect } from 'vitest'; +import { createFeedbackDialogGuard } from './feedbackDialogGuard.js'; + +const APP = 'app-uid-1'; + +// A guard on a clock the test drives by hand. +const makeGuard = () => { + let t = 1_000_000; + const guard = createFeedbackDialogGuard({ now: () => t }); + return { + guard, + advance: (ms) => { t += ms; }, + // One full open-and-dismiss cycle, the user taking `duration` to + // close the dialog. Returns false if the guard refused to open it. + cycle: (key = APP, { duration = 5_000, sent = false } = {}) => { + if ( ! guard.mayOpen(key) ) return false; + guard.markOpened(); + t += duration; + guard.markClosed(key, sent); + return true; + }, + }; +}; + +describe('createFeedbackDialogGuard', () => { + it('opens for an app it has never seen', () => { + const { guard } = makeGuard(); + expect(guard.mayOpen(APP)).toBe(true); + }); + + it('reopens after a dismissal, as many times as the user asks', () => { + const { guard, cycle, advance } = makeGuard(); + // The bug this guards against: closing the dialog used to cost the + // app its next open for 10s, then 60s, then the rest of the session. + for ( let i = 0; i < 5; i++ ) { + expect(cycle(), `open #${i + 1}`).toBe(true); + advance(2_000); // the user goes back and clicks "Send feedback" + } + expect(guard.mayOpen(APP)).toBe(true); + }); + + it('reopens a second after the dialog closed', () => { + const { guard, cycle, advance } = makeGuard(); + expect(cycle()).toBe(true); + advance(1_001); + expect(guard.mayOpen(APP)).toBe(true); + }); + + it('refuses a second dialog while one is open', () => { + const { guard } = makeGuard(); + expect(guard.mayOpen(APP)).toBe(true); + guard.markOpened(); + expect(guard.mayOpen(APP)).toBe(false); + expect(guard.mayOpen('some-other-app')).toBe(false); + guard.markClosed(APP, false); + }); + + it('backs off an app that reopens the instant it is dismissed', () => { + const { guard, cycle, advance } = makeGuard(); + expect(cycle()).toBe(true); + advance(1); + expect(guard.mayOpen(APP)).toBe(false); + + // ...and holds it off for the first tier. + advance(9_000); + expect(guard.mayOpen(APP)).toBe(false); + }); + + it('escalates while the app keeps hammering, and blocks it for good', () => { + const { guard, cycle, advance } = makeGuard(); + expect(cycle()).toBe(true); + + // A loop calling every 50ms. Waiting out a backoff is no escape: the + // attempts made during it are activity of their own, so the next tier + // applies rather than a clean slate. + const hammer = (ms) => { + for ( let elapsed = 0; elapsed < ms; elapsed += 50 ) { + advance(50); + if ( guard.mayOpen(APP) ) return true; + } + return false; + }; + + expect(hammer(10_000), 'first tier').toBe(false); + expect(hammer(60_000), 'second tier').toBe(false); + expect(hammer(10 * 60_000), 'blocked for the session').toBe(false); + }); + + it('lets a hammering app back in once it goes quiet', () => { + const { guard, cycle, advance } = makeGuard(); + expect(cycle()).toBe(true); + advance(1); + expect(guard.mayOpen(APP)).toBe(false); // strike: 10s + + // The user closes the app's own dialog loop by leaving it alone, then + // comes back later and asks for the form themselves. + advance(30_000); + expect(guard.mayOpen(APP)).toBe(true); + }); + + it('keeps each app on its own record', () => { + const { guard, cycle, advance } = makeGuard(); + expect(cycle('noisy-app')).toBe(true); + advance(1); + expect(guard.mayOpen('noisy-app')).toBe(false); + expect(guard.mayOpen('quiet-app')).toBe(true); + }); + + it('clears the record when the user actually sends feedback', () => { + const { guard, cycle, advance } = makeGuard(); + expect(cycle()).toBe(true); + advance(1); + expect(guard.mayOpen(APP)).toBe(false); // strike: 10s + + advance(30_000); + expect(cycle(APP, { sent: true })).toBe(true); + // A send wipes the strikes, so even an immediate reopen is allowed. + expect(guard.mayOpen(APP)).toBe(true); + }); +});