From c46935c176e79645ceeb7832d46c0b161919106a Mon Sep 17 00:00:00 2001 From: Tom Scanlan Date: Thu, 20 Aug 2026 08:37:48 -0400 Subject: [PATCH 1/2] fix(events): hide draft and private occurrences from anonymous series reads GET /api/events/series/:seriesSlug/events is a public route, but it had no status or visibility filter. Anonymous callers received draft and private occurrences of a series along with the published ones. VisibilityGuard is on the route but never checks it. The guard reads a :slug route param, and the only param on this route is :seriesSlug, so the guard finds no event to check and allows the request. findEventsBySeriesSlug now takes an optional publicOnly flag that adds the same predicate the guard applies to GET /events/:slug, which is published or cancelled status and public or unlisted visibility. The flag is opt-in because series materialization and template lookup call the same method and need to see every occurrence. If they could not see drafts they would create duplicates of occurrences that already exist. The controller sets the flag for callers with no user, so signed-in callers keep the results they get today. The response also carried two fields that anonymous callers should not get. Neither entity has @Exclude, so every loaded column was sent, including sourceData with the original poster's DID, handle, CID and rkey, and the internal matrixRoomId. Both are now removed for anonymous callers. sourceId is left alone because the web app shows it as public attribution for events imported from Bluesky. --- src/event/event.controller.spec.ts | 75 +++++++++++++++++++++++ src/event/event.controller.ts | 20 ++++++ src/event/services/event-query.service.ts | 23 ++++++- 3 files changed, 117 insertions(+), 1 deletion(-) diff --git a/src/event/event.controller.spec.ts b/src/event/event.controller.spec.ts index 216562ab..7e786ffd 100644 --- a/src/event/event.controller.spec.ts +++ b/src/event/event.controller.spec.ts @@ -85,6 +85,7 @@ const mockEventQueryService = { getEventsByCreator: jest.fn(), getEventsByAttendee: jest.fn(), showEventBySlug: jest.fn(), + findEventsBySeriesSlug: jest.fn(), }; const mockEventRecommendationService = { @@ -730,4 +731,78 @@ describe('EventController', () => { ).rejects.toThrow('Event not found'); }); }); + describe('getEventsBySeries', () => { + const buildEvent = () => + ({ + ...mockEvent, + sourceData: { did: 'did:plc:abc123', handle: 'someone.bsky.social' }, + matrixRoomId: '!room:matrix.openmeet.net', + }) as unknown as EventEntity; + + beforeEach(() => { + mockEventQueryService.findEventsBySeriesSlug.mockReset(); + }); + + it('should restrict the query to publicly visible occurrences for anonymous callers', async () => { + mockEventQueryService.findEventsBySeriesSlug.mockResolvedValue([[], 0]); + + await controller.getEventsBySeries('test-series', {} as any, { + user: undefined, + }); + + expect(mockEventQueryService.findEventsBySeriesSlug).toHaveBeenCalledWith( + 'test-series', + expect.objectContaining({ publicOnly: true }), + ); + }); + + it('should leave the query unrestricted for authenticated callers', async () => { + mockEventQueryService.findEventsBySeriesSlug.mockResolvedValue([[], 0]); + + await controller.getEventsBySeries('test-series', {} as any, { + user: mockUser, + }); + + expect(mockEventQueryService.findEventsBySeriesSlug).toHaveBeenCalledWith( + 'test-series', + expect.objectContaining({ publicOnly: false }), + ); + }); + + it('should withhold sourceData and matrixRoomId from anonymous callers', async () => { + mockEventQueryService.findEventsBySeriesSlug.mockResolvedValue([ + [buildEvent()], + 1, + ]); + + const result = await controller.getEventsBySeries( + 'test-series', + {} as any, + { user: undefined }, + ); + + expect(result).toHaveLength(1); + expect(result[0].sourceData).toBeNull(); + expect(result[0].matrixRoomId).toBeNull(); + }); + + it('should leave those fields intact for authenticated callers', async () => { + mockEventQueryService.findEventsBySeriesSlug.mockResolvedValue([ + [buildEvent()], + 1, + ]); + + const result = await controller.getEventsBySeries( + 'test-series', + {} as any, + { user: mockUser }, + ); + + expect(result[0].sourceData).toEqual({ + did: 'did:plc:abc123', + handle: 'someone.bsky.social', + }); + expect(result[0].matrixRoomId).toBe('!room:matrix.openmeet.net'); + }); + }); }); diff --git a/src/event/event.controller.ts b/src/event/event.controller.ts index b065cf88..8ae87ca1 100644 --- a/src/event/event.controller.ts +++ b/src/event/event.controller.ts @@ -452,11 +452,18 @@ export class EventController { `Starting database query for series ${seriesSlug} events at ${new Date().toISOString()}`, ); + // This route is @Public() and VisibilityGuard cannot check it. The guard + // reads a :slug param, and the only param here is :seriesSlug, so the + // guard falls through and allows the request. Anonymous callers get the + // same restriction the guard would have applied, enforced in the query. + const isAnonymous = !_req.user; + const findEventsPromise = this.eventQueryService.findEventsBySeriesSlug( seriesSlug, { page: +pagination.page || 1, limit: +pagination.limit || 10, + publicOnly: isAnonymous, }, ); @@ -486,6 +493,19 @@ export class EventController { this.logger.log( `Returning ${events.length} events for series ${seriesSlug}`, ); + + // Neither entity carries @Exclude, so every loaded column would be sent. + // sourceData holds the original poster's DID, handle, CID and rkey, and + // matrixRoomId is an internal room address. Remove both before they go + // to an unauthenticated caller. + if (isAnonymous) { + for (const event of events) { + event.sourceData = null; + // Column is nullable; the entity types it as a bare string. + event.matrixRoomId = null as unknown as string; + } + } + return events; } catch (error) { this.logger.error( diff --git a/src/event/services/event-query.service.ts b/src/event/services/event-query.service.ts index 38daf7e2..cb4d6c0a 100644 --- a/src/event/services/event-query.service.ts +++ b/src/event/services/event-query.service.ts @@ -1697,11 +1697,17 @@ export class EventQueryService { /** * Find all events (occurrences) that belong to a series by the series slug + * + * Pass publicOnly when the results are going to an unauthenticated caller. + * It limits the query to occurrences that are safe to show anonymously. + * Internal callers such as series materialization and template lookup must + * leave it unset. They need to see every occurrence, drafts included, or + * they will create duplicates of occurrences that already exist. */ @Trace('event-query.findEventsBySeriesSlug') async findEventsBySeriesSlug( seriesSlug: string, - options?: { page: number; limit: number }, + options?: { page: number; limit: number; publicOnly?: boolean }, ): Promise<[EventEntity[], number]> { try { await this.initializeRepository(); @@ -1723,6 +1729,21 @@ export class EventQueryService { .skip((page - 1) * limit) .take(limit); + // This is the same rule VisibilityGuard applies to GET /events/:slug. + // It runs in SQL rather than after the fetch so paging stays correct. + if (options?.publicOnly) { + queryBuilder + .andWhere('event.status IN (:...visibleStatuses)', { + visibleStatuses: [EventStatus.Published, EventStatus.Cancelled], + }) + .andWhere('event.visibility IN (:...publicVisibilities)', { + publicVisibilities: [ + EventVisibility.Public, + EventVisibility.Unlisted, + ], + }); + } + try { const [events, total] = await queryBuilder.getManyAndCount(); this.logger.debug( From dcf1e742b03a84ea7985bd1578afe037de064f79 Mon Sep 17 00:00:00 2001 From: Tom Scanlan Date: Thu, 20 Aug 2026 11:26:05 -0400 Subject: [PATCH 2/2] test(events): cover the series visibility predicate at the query layer The controller tests mock findEventsBySeriesSlug, so they check that the controller asks for a restricted query but not that the query applies the restriction. Deleting the two predicates left every test passing while the anonymous leak came back. These tests assert the status and visibility predicates directly, including that draft, pending and private are all excluded, and that an unset publicOnly leaves the query unrestricted so series materialization still sees every occurrence. --- .../services/event-query.service.spec.ts | 76 +++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/src/event/services/event-query.service.spec.ts b/src/event/services/event-query.service.spec.ts index 4d86c826..b7bc94dd 100644 --- a/src/event/services/event-query.service.spec.ts +++ b/src/event/services/event-query.service.spec.ts @@ -1001,4 +1001,80 @@ describe('EventQueryService', () => { expect(result).toHaveLength(0); }); }); + describe('findEventsBySeriesSlug visibility predicate', () => { + const buildQueryBuilderMock = () => ({ + where: jest.fn().mockReturnThis(), + andWhere: jest.fn().mockReturnThis(), + leftJoinAndSelect: jest.fn().mockReturnThis(), + orderBy: jest.fn().mockReturnThis(), + skip: jest.fn().mockReturnThis(), + take: jest.fn().mockReturnThis(), + getManyAndCount: jest.fn().mockResolvedValue([[], 0]), + }); + + const runWith = async (options: { + page: number; + limit: number; + publicOnly?: boolean; + }) => { + const qb = buildQueryBuilderMock(); + jest + .spyOn(service['tenantConnectionService'], 'getTenantConnection') + .mockResolvedValue({ + getRepository: jest.fn().mockReturnValue({ + createQueryBuilder: jest.fn().mockReturnValue(qb), + }), + } as any); + + await service.findEventsBySeriesSlug('test-series', options); + return qb; + }; + + it('should restrict status and visibility when publicOnly is set', async () => { + const qb = await runWith({ page: 1, limit: 10, publicOnly: true }); + + expect(qb.andWhere).toHaveBeenCalledWith( + 'event.status IN (:...visibleStatuses)', + { + visibleStatuses: [EventStatus.Published, EventStatus.Cancelled], + }, + ); + expect(qb.andWhere).toHaveBeenCalledWith( + 'event.visibility IN (:...publicVisibilities)', + { + publicVisibilities: [ + EventVisibility.Public, + EventVisibility.Unlisted, + ], + }, + ); + }); + + it('should never expose draft, pending or private occurrences when publicOnly is set', async () => { + const qb = await runWith({ page: 1, limit: 10, publicOnly: true }); + + const statusCall = qb.andWhere.mock.calls.find( + (call) => call[0] === 'event.status IN (:...visibleStatuses)', + ); + const visibilityCall = qb.andWhere.mock.calls.find( + (call) => call[0] === 'event.visibility IN (:...publicVisibilities)', + ); + + expect(statusCall).toBeDefined(); + expect(visibilityCall).toBeDefined(); + + const allowedStatuses = statusCall![1].visibleStatuses; + const allowedVisibilities = visibilityCall![1].publicVisibilities; + + expect(allowedStatuses).not.toContain(EventStatus.Draft); + expect(allowedStatuses).not.toContain(EventStatus.Pending); + expect(allowedVisibilities).not.toContain(EventVisibility.Private); + }); + + it('should not restrict the query when publicOnly is unset', async () => { + const qb = await runWith({ page: 1, limit: 10 }); + + expect(qb.andWhere).not.toHaveBeenCalled(); + }); + }); });