From 5db73f86dfaf5ef2127b0b9c2a35bbb12f54e2c6 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Wed, 9 Jul 2025 18:21:52 -0400 Subject: [PATCH] fix logic errors in collection fetch and featured posts validation --- .../src/core/activitypub/models/ApPersonService.ts | 11 +++++++++-- packages/backend/test/unit/activitypub.ts | 13 ++++++++++--- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/packages/backend/src/core/activitypub/models/ApPersonService.ts b/packages/backend/src/core/activitypub/models/ApPersonService.ts index 5f31d529ad..2b87e94314 100644 --- a/packages/backend/src/core/activitypub/models/ApPersonService.ts +++ b/packages/backend/src/core/activitypub/models/ApPersonService.ts @@ -943,10 +943,17 @@ export class ApPersonService implements OnModuleInit { const itemId = getNullableApId(item); if (itemId && isPost(item)) { try { - return await this.apNoteService.resolveNote(item, { + const note = await this.apNoteService.resolveNote(item, { resolver: resolver, - sentFrom: user.uri, + sentFrom: itemId, // resolveCollectionItems has already verified this, so we can re-use it to avoid double fetch }); + + if (note && note.userId !== userId) { + this.logger.warn(`Ignoring cross-note pin: user ${userId} tried to pin note ${note.id} belonging to user ${note.userId}`); + return null; + } + + return note; } catch (err) { this.logger.warn(`Couldn't fetch pinned note ${itemId} for user ${user.id} (@${user.username}@${user.host}): ${renderInlineError(err)}`); } diff --git a/packages/backend/test/unit/activitypub.ts b/packages/backend/test/unit/activitypub.ts index eccad42633..1e2cc02f2f 100644 --- a/packages/backend/test/unit/activitypub.ts +++ b/packages/backend/test/unit/activitypub.ts @@ -26,10 +26,11 @@ import { LoggerService } from '@/core/LoggerService.js'; import { CacheManagementService } from '@/global/CacheManagementService.js'; import { ApResolverService } from '@/core/activitypub/ApResolverService.js'; import type { IActor, IApDocument, ICollection, IObject, IPost } from '@/core/activitypub/type.js'; -import { MiMeta, MiNote, MiUser, MiUserKeypair, UserProfilesRepository, UserPublickeysRepository, UserKeypairsRepository, UsersRepository, NotesRepository } from '@/models/_.js'; +import { MiMeta, MiNote, MiUser, MiUserKeypair, UserNotePiningsRepository, UserProfilesRepository, UserPublickeysRepository, UserKeypairsRepository, UsersRepository, NotesRepository } from '@/models/_.js'; import { DI } from '@/di-symbols.js'; import { secureRndstr } from '@/misc/secure-rndstr.js'; import { DownloadService } from '@/core/DownloadService.js'; +import { ApUtilityService } from '@/core/activitypub/ApUtilityService.js'; import { genAidx } from '@/misc/id/aidx.js'; import { IdService } from '@/core/IdService.js'; import { MockResolver } from '../misc/mock-resolver.js'; @@ -75,7 +76,7 @@ function createRandomFeaturedCollection(actor: NonTransientIActor, length: numbe return { '@context': 'https://www.w3.org/ns/activitystreams', type: 'Collection', - id: actor.outbox as string, + id: actor.featured as string | null ?? `${actor.id}/featured`, totalItems: items.length, items, }; @@ -108,6 +109,7 @@ describe('ActivityPub', () => { let cacheManagementService: CacheManagementService; let mockConsole: MockConsole; let notesRepository: NotesRepository; + let userNotePiningsRepository: UserNotePiningsRepository; const metaInitial = { id: 'x', @@ -185,6 +187,7 @@ describe('ActivityPub', () => { mockConsole = app.get(DI.console); notesRepository = app.get(DI.notesRepository); }); + userNotePiningsRepository = app.get(DI.userNotePiningsRepository); afterAll(async () => { await app.close(); @@ -380,7 +383,7 @@ describe('ActivityPub', () => { resolver.register(actor2.id, actor2); resolver.register(actor2Note.id, actor2Note); - await personService.createPerson(actor1.id, resolver); + const created = await personService.createPerson(actor1.id, resolver); // actor2Note is from a different server and needs to be fetched again assert.deepStrictEqual( @@ -394,6 +397,10 @@ describe('ActivityPub', () => { // Reflects the original content instead of the fraud assert.strictEqual(note.text, 'test test foo'); assert.strictEqual(note.uri, actor2Note.id); + + // Cross-user pin should be rejected + const pinExists = await userNotePiningsRepository.existsBy({ userId: created.id, noteId: note.id }); + expect(pinExists).toBe(false); }); test('Fetch a note that is a featured note of the attributed actor', async () => {