From 7a876ec3346354903aeb8dad5e95029607df1558 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 31 Jul 2026 04:57:05 +0000 Subject: [PATCH] refactor(notifications): bell loaders read through owner access, no bypass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit countUnreadNotifications/loadNotificationList took a caller-supplied recipientID with module-level admin bypass — one misuse away from reading someone else's inbox. They now take the actor (CampaignUser) and read with overrideAccess: false under canReadOwnNotifications, keeping the explicit recipient filter as a second fence. Pin: owner isolation at the loader level in campaignNotifications.int.spec.ts. Co-authored-by: Francisco Solla --- .../campanha/actions/notifications.ts | 4 +- .../notification/notificationList.ts | 27 ++++++++----- tests/int/campaignNotifications.int.spec.ts | 40 +++++++++++++++++++ 3 files changed, 59 insertions(+), 12 deletions(-) diff --git a/src/app/(campaign)/campanha/actions/notifications.ts b/src/app/(campaign)/campanha/actions/notifications.ts index a31c3902..ecefb661 100644 --- a/src/app/(campaign)/campanha/actions/notifications.ts +++ b/src/app/(campaign)/campanha/actions/notifications.ts @@ -43,7 +43,7 @@ export const loadNotificationBellData = async (): Promise diff --git a/src/utilities/notification/notificationList.ts b/src/utilities/notification/notificationList.ts index 1a49b9dc..bf98dd92 100644 --- a/src/utilities/notification/notificationList.ts +++ b/src/utilities/notification/notificationList.ts @@ -1,13 +1,13 @@ import 'server-only' -/** Trusted server reads in this module use admin bypass (`overrideAccess: true`). */ +import type { Payload } from 'payload' import type { NotificationListItem, NotificationPayload, NotificationType, } from '@/lib/notificationContract' -import type { Notification } from '@/payload-types' +import type { CampaignUser, Notification } from '@/payload-types' export type { NotificationListItem } from '@/lib/notificationContract' @@ -33,33 +33,40 @@ const toNotificationListItem = (doc: Notification): NotificationListItem | null } } +// Reads go through collection access (`canReadOwnNotifications` scopes to the +// recipient) with the explicit recipient filter as a second fence: the actor +// is the only thing these loaders accept, so a caller cannot ask for someone +// else's inbox. No admin bypass — that is what the create/update paths are +// for, and they live in the domain hooks/actions with their own justifications. export const countUnreadNotifications = async ( - payload: import('payload').Payload, - recipientID: number, + payload: Payload, + user: CampaignUser, ): Promise => { const result = await payload.count({ collection: 'notification', where: { - and: [{ recipient: { equals: recipientID } }, { readAt: { exists: false } }], + and: [{ recipient: { equals: user.id } }, { readAt: { exists: false } }], }, - overrideAccess: true, + user, + overrideAccess: false, }) return result.totalDocs } export const loadNotificationList = async ( - payload: import('payload').Payload, - recipientID: number, + payload: Payload, + user: CampaignUser, limit = 40, ): Promise => { const result = await payload.find({ collection: 'notification', - where: { recipient: { equals: recipientID } }, + where: { recipient: { equals: user.id } }, depth: 0, limit, sort: '-createdAt', - overrideAccess: true, + user, + overrideAccess: false, }) return result.docs diff --git a/tests/int/campaignNotifications.int.spec.ts b/tests/int/campaignNotifications.int.spec.ts index fb244b26..526c2422 100644 --- a/tests/int/campaignNotifications.int.spec.ts +++ b/tests/int/campaignNotifications.int.spec.ts @@ -4,6 +4,10 @@ import { getPayload, type Payload } from 'payload' import { beforeAll, describe, expect, it } from 'vitest' import config from '@/payload.config' +import { + countUnreadNotifications, + loadNotificationList, +} from '@/utilities/notification/notificationList' import { installCampaignFixtures } from '../helpers/campaignFixtures' @@ -69,6 +73,42 @@ describe('notification access', () => { expect(visible.docs.map((doc) => doc.id)).toEqual([own.id]) }) + + it('reads the bell loaders through owner-scoped access (no bypass)', async () => { + const fixtures = campaignFixtures() + const [owner, other] = await Promise.all([ + fixtures.createCampaignUser('advisor'), + fixtures.createCampaignUser('advisor'), + ]) + + const own = await payload.create({ + collection: 'notification', + data: { + recipient: owner.id, + type: 'municipality_update', + payload: { title: 'Sino — própria', detail: 'Detalhe', href: '/campanha' }, + }, + overrideAccess: true, + }) + await payload.create({ + collection: 'notification', + data: { + recipient: other.id, + type: 'new_supporter', + payload: { title: 'Sino — alheia', detail: 'Detalhe', href: '/campanha' }, + }, + overrideAccess: true, + }) + + // The loaders take the actor, not a recipientID — a caller cannot ask for + // someone else's inbox (Pass 4: the previous recipientID+bypass shape + // trusted every call site). + await expect(loadNotificationList(payload, owner)).resolves.toEqual([ + expect.objectContaining({ id: own.id }), + ]) + await expect(countUnreadNotifications(payload, owner)).resolves.toBe(1) + await expect(countUnreadNotifications(payload, other)).resolves.toBe(1) + }) }) describe('pushSubscription access', () => {