From 5ce131517bf3e09af2f1eb22348bd974e03323dc Mon Sep 17 00:00:00 2001 From: Juan Castro Date: Tue, 27 Oct 2026 14:07:25 -0400 Subject: [PATCH 1/2] fix(notifications): refuse an app message before it spends the day window The minute window is spent first, so a request the minute window refuses between the read and the spend leaves the day window untouched. --- .../NotificationDriver.appMessage.test.ts | 82 ++++++++++++++++++- .../notification/NotificationDriver.ts | 17 ++-- 2 files changed, 92 insertions(+), 7 deletions(-) diff --git a/src/backend/drivers/notification/NotificationDriver.appMessage.test.ts b/src/backend/drivers/notification/NotificationDriver.appMessage.test.ts index 712900197..52dcea270 100644 --- a/src/backend/drivers/notification/NotificationDriver.appMessage.test.ts +++ b/src/backend/drivers/notification/NotificationDriver.appMessage.test.ts @@ -17,7 +17,7 @@ * along with this program. If not, see . */ -import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; import { v4 as uuidv4 } from 'uuid'; import { makeActor, type Actor } from '../../core/actor.js'; import { runWithContext } from '../../core/context.js'; @@ -28,6 +28,42 @@ import type { IConfig } from '../../types.js'; import { peekRateLimit } from '../../core/http/middleware/rateLimit.js'; import type { NotificationDriver } from './NotificationDriver.js'; +/** + * Charges to a held key wait here until the test lets them go, so two requests + * can both be read before either is spent. Every charge is recorded by key. + */ +const charges = vi.hoisted(() => ({ + held: new Set(), + waiting: [] as Array<() => void>, + made: [] as string[], +})); + +vi.mock('../../core/http/middleware/rateLimit.js', async (importOriginal) => { + const actual = + await importOriginal< + typeof import('../../core/http/middleware/rateLimit.js') + >(); + return { + ...actual, + checkRateLimit: async (key: string, ...rest: unknown[]) => { + if (charges.held.has(key)) { + await new Promise((go) => charges.waiting.push(go)); + } + charges.made.push(key); + return ( + actual.checkRateLimit as (...a: unknown[]) => Promise + )(key, ...rest); + }, + }; +}); + +const until = async (ready: () => boolean): Promise => { + for (let i = 0; i < 400 && !ready(); i++) { + await new Promise((resolve) => setTimeout(resolve, 5)); + } + if (!ready()) throw new Error('timed out'); +}; + const makeUser = async (server: PuterServer): Promise => { const username = `am-${Math.random().toString(36).slice(2, 10)}`; const created = await server.stores.user.create({ @@ -347,6 +383,50 @@ describe('app.message with live delivery over the socket wire', () => { ).resolves.toMatchObject({ uid: expect.any(String) }); }); + it('spends the day window only on a request the minute window admits', async () => { + const holder = await makeUser(state.server); + const app = await makeApp(state.server, holder.user.id!); + const minuteKey = `notif:app-message:minute:${holder.user.id}:${app.uid}`; + const dayKey = `notif:app-message:day:${holder.user.id}:${app.uid}`; + for (let i = 0; i < 2; i++) { + await withActor(asApp(holder, app), () => + state.driver.create(message()), + ); + } + + charges.held.add(minuteKey).add(dayKey); + charges.made.length = 0; + let settled = false; + const outcome = Promise.allSettled([ + withActor(asApp(holder, app), () => state.driver.create(message())), + withActor(asApp(holder, app), () => state.driver.create(message())), + ]).finally(() => { + settled = true; + }); + try { + // Both requests read the windows as open before either spends. + await until(() => charges.waiting.length === 2); + while (!settled) { + await until(() => settled || charges.waiting.length > 0); + charges.waiting.shift()?.(); + } + } finally { + charges.held.clear(); + } + + const results = await outcome; + expect(results.map((r) => r.status).sort()).toEqual([ + 'fulfilled', + 'rejected', + ]); + const refused = results.find((r) => r.status === 'rejected') as + PromiseRejectedResult | undefined; + expect(refused?.reason).toMatchObject({ statusCode: 429 }); + // The refused request left the day window alone. + expect(charges.made.filter((k) => k === dayKey)).toEqual([dayKey]); + expect(await peekRateLimit(dayKey, 4, 24 * 60 * 60_000)).toBe(true); + }); + it("reads back the app's own messages and no other app's", async () => { const holder = await makeUser(state.server); const app = await makeApp(state.server, holder.user.id!); diff --git a/src/backend/drivers/notification/NotificationDriver.ts b/src/backend/drivers/notification/NotificationDriver.ts index 887a31e6d..7e58f3a80 100644 --- a/src/backend/drivers/notification/NotificationDriver.ts +++ b/src/backend/drivers/notification/NotificationDriver.ts @@ -340,23 +340,28 @@ export class NotificationDriver extends PuterDriver { }); } - /** Both windows are read before either is spent, so a refusal costs none. */ + /** + * Both windows are read before either is spent, so a refusal costs none. + * The minute window is spent first: it is the one most likely to run out + * between the read and the spend, and a refusal there leaves the day + * alone. + */ async #withinAppMessageLimits( userId: number, appUid: string, ): Promise { const configured = this.config.notificationAppMessageLimits ?? {}; const all: Array<[string, number, number]> = [ - [ - `notif:app-message:day:${userId}:${appUid}`, - configured.perDay ?? APP_MESSAGE_RATE_LIMITS.perDay, - DAY_MS, - ], [ `notif:app-message:minute:${userId}:${appUid}`, configured.perMinute ?? APP_MESSAGE_RATE_LIMITS.perMinute, MINUTE_MS, ], + [ + `notif:app-message:day:${userId}:${appUid}`, + configured.perDay ?? APP_MESSAGE_RATE_LIMITS.perDay, + DAY_MS, + ], ]; const windows = all.filter(([, limit]) => limit > 0); for (const [key, limit, windowMs] of windows) { From f0d7a69dd4da4f2516e86dbad8970063d496f114 Mon Sep 17 00:00:00 2001 From: Juan Castro Date: Tue, 27 Oct 2026 14:07:27 -0400 Subject: [PATCH 2/2] fix(notifications): cap the sender name on app messages The app's title or name is cut to the title length, counting code points, before it is stored on the message. --- src/backend/services/notification/appMessage.test.ts | 10 ++++++++++ src/backend/services/notification/appMessage.ts | 7 ++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/backend/services/notification/appMessage.test.ts b/src/backend/services/notification/appMessage.test.ts index 48d4df0f1..717362010 100644 --- a/src/backend/services/notification/appMessage.test.ts +++ b/src/backend/services/notification/appMessage.test.ts @@ -212,4 +212,14 @@ describe('appMessageSender', () => { ).toEqual({ uid: 'app-1', name: 'chat' }); expect(appMessageSender('app-1', null)).toEqual({ uid: 'app-1' }); }); + + it('caps the name at the title length, counting an emoji once', () => { + const long = '😀'.repeat(500); + const { name } = appMessageSender('app-1', { title: long }); + expect([...name!].length).toBe(120); + expect(name).toBe('😀'.repeat(120)); + expect(appMessageSender('app-1', { title: 'a'.repeat(300) }).name).toBe( + 'a'.repeat(120), + ); + }); }); diff --git a/src/backend/services/notification/appMessage.ts b/src/backend/services/notification/appMessage.ts index a79155b65..a13bb84a4 100644 --- a/src/backend/services/notification/appMessage.ts +++ b/src/backend/services/notification/appMessage.ts @@ -69,7 +69,12 @@ export const appMessageSender = ( const name = [row?.title, row?.name].find( (v): v is string => typeof v === 'string' && v.trim() !== '', ); - if (name !== undefined) sender.name = cleanText(name).trim(); + if (name !== undefined) { + sender.name = [...cleanText(name)] + .slice(0, APP_MESSAGE_TITLE_MAX) + .join('') + .trim(); + } const icon = row?.icon; if ( typeof icon === 'string' &&