From 5299ee578c23269507f7335a93625fe9e01f2809 Mon Sep 17 00:00:00 2001 From: Ilya Dorman Date: Sat, 25 Apr 2026 18:29:06 +0100 Subject: [PATCH 1/2] Stabilize Vitest integration harness --- src/db.integration.test.ts | 8 ++++---- src/ghost/ghost-post.service.integration.test.ts | 2 +- src/http/api/views/blocks.view.integration.test.ts | 2 +- src/http/api/views/explore.view.integration.test.ts | 2 +- .../api/views/recommendations.view.integration.test.ts | 2 +- src/http/api/views/topic.view.integration.test.ts | 2 +- src/post/post.service.integration.test.ts | 2 +- vitest.config.ts | 5 ++++- 8 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/db.integration.test.ts b/src/db.integration.test.ts index 6caffe45a..541779e5a 100644 --- a/src/db.integration.test.ts +++ b/src/db.integration.test.ts @@ -18,11 +18,11 @@ describe('Timestamp Test', () => { .timestamp('created_at', { precision: 6 }) .defaultTo(client.fn.now(6)); }); + }); - afterAll(async () => { - await client.schema.dropTableIfExists(TEST_TABLE); - await client.destroy(); - }); + afterAll(async () => { + await client?.schema.dropTableIfExists(TEST_TABLE); + await client?.destroy(); }); it('should insert and return UTC timestamp', async () => { diff --git a/src/ghost/ghost-post.service.integration.test.ts b/src/ghost/ghost-post.service.integration.test.ts index facfb1646..624aeae0d 100644 --- a/src/ghost/ghost-post.service.integration.test.ts +++ b/src/ghost/ghost-post.service.integration.test.ts @@ -102,7 +102,7 @@ describe('GhostPostService', () => { afterEach(async () => { // Clean up database connections - await db.destroy(); + await db?.destroy(); }); describe('updateArticleFromGhostPost', () => { diff --git a/src/http/api/views/blocks.view.integration.test.ts b/src/http/api/views/blocks.view.integration.test.ts index 4b6ba3fdf..91fd30b0d 100644 --- a/src/http/api/views/blocks.view.integration.test.ts +++ b/src/http/api/views/blocks.view.integration.test.ts @@ -21,7 +21,7 @@ describe('BlocksView', () => { }); afterAll(async () => { - await db.destroy(); + await db?.destroy(); }); describe('getBlockedAccounts', () => { diff --git a/src/http/api/views/explore.view.integration.test.ts b/src/http/api/views/explore.view.integration.test.ts index af9f87b76..979ebbb22 100644 --- a/src/http/api/views/explore.view.integration.test.ts +++ b/src/http/api/views/explore.view.integration.test.ts @@ -23,7 +23,7 @@ describe('ExploreView', () => { }); afterAll(async () => { - await db.destroy(); + await db?.destroy(); }); describe('getAccountsInTopic', () => { diff --git a/src/http/api/views/recommendations.view.integration.test.ts b/src/http/api/views/recommendations.view.integration.test.ts index eca6fc947..f2e2e0632 100644 --- a/src/http/api/views/recommendations.view.integration.test.ts +++ b/src/http/api/views/recommendations.view.integration.test.ts @@ -22,7 +22,7 @@ describe('RecommendationsView', () => { }); afterAll(async () => { - await db.destroy(); + await db?.destroy(); }); describe('getRecommendations', () => { diff --git a/src/http/api/views/topic.view.integration.test.ts b/src/http/api/views/topic.view.integration.test.ts index deb299ab6..f7a642b43 100644 --- a/src/http/api/views/topic.view.integration.test.ts +++ b/src/http/api/views/topic.view.integration.test.ts @@ -22,7 +22,7 @@ describe('TopicView', () => { }); afterAll(async () => { - await db.destroy(); + await db?.destroy(); }); describe('getTopics', () => { diff --git a/src/post/post.service.integration.test.ts b/src/post/post.service.integration.test.ts index 28028d912..dc282a79e 100644 --- a/src/post/post.service.integration.test.ts +++ b/src/post/post.service.integration.test.ts @@ -142,7 +142,7 @@ describe('PostService', () => { afterEach(async () => { // Clean up database connections - await db.destroy(); + await db?.destroy(); }); describe('createNote', () => { diff --git a/vitest.config.ts b/vitest.config.ts index 980605ad9..ff9e1afe4 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -4,6 +4,9 @@ import { defineConfig } from 'vitest/config'; export default defineConfig({ plugins: [tsconfigPaths()], test: { - testTimeout: 1000 * 10, + testTimeout: 1000 * 30, + hookTimeout: 1000 * 60, + minWorkers: 1, + maxWorkers: 4, }, }); From e9494d8d9559e762ff0dd4debdff07b30befd3db Mon Sep 17 00:00:00 2001 From: Ilya Dorman Date: Sat, 25 Apr 2026 18:29:15 +0100 Subject: [PATCH 2/2] Validate actor consistency for inbound Undo activities --- src/dispatchers.ts | 21 ++++ src/dispatchers.unit.test.ts | 182 ++++++++++++++++++++++++++++++++++- 2 files changed, 201 insertions(+), 2 deletions(-) diff --git a/src/dispatchers.ts b/src/dispatchers.ts index a7c14aad0..ca1955773 100644 --- a/src/dispatchers.ts +++ b/src/dispatchers.ts @@ -502,6 +502,16 @@ export const createUndoHandler = ( return; } + if ( + !undo.actorId || + undo.actorId.href !== follow.actorId.href + ) { + ctx.data.logger.warn( + 'Undo actor does not match Follow actor - exiting', + ); + return; + } + const [unfollower, unfollowing] = await Promise.all([ accountService.getAccountByApId(follow.actorId.href), accountService.getAccountByApId(follow.objectId.href), @@ -519,6 +529,17 @@ export const createUndoHandler = ( await accountService.recordAccountUnfollow(unfollowing, unfollower); } else if (object instanceof Announce) { + if ( + !undo.actorId || + !object.actorId || + undo.actorId.href !== object.actorId.href + ) { + ctx.data.logger.warn( + 'Undo actor does not match Announce actor - exiting', + ); + return; + } + const sender = await object.getActor(ctx); if (sender === null || sender.id === null) { ctx.data.logger.debug( diff --git a/src/dispatchers.unit.test.ts b/src/dispatchers.unit.test.ts index 84cfe4aee..3d45b86c8 100644 --- a/src/dispatchers.unit.test.ts +++ b/src/dispatchers.unit.test.ts @@ -1,7 +1,13 @@ +import { + type Actor, + Announce, + exportJwk, + Follow, + type RequestContext, + Undo, +} from '@fedify/fedify'; import { beforeEach, describe, expect, it, vi } from 'vitest'; -import { exportJwk, type RequestContext } from '@fedify/fedify'; - import type { Account, AccountEntity } from '@/account/account.entity'; import type { AccountService } from '@/account/account.service'; import type { Account as AccountType } from '@/account/types'; @@ -20,12 +26,14 @@ import { createFollowingDispatcher, createOutboxCounter, createOutboxDispatcher, + createUndoHandler, keypairDispatcher, likedDispatcher, nodeInfoDispatcher, } from '@/dispatchers'; import type { HostDataContextLoader } from '@/http/host-data-context-loader'; import { OutboxType, Post, PostType } from '@/post/post.entity'; +import type { KnexPostRepository } from '@/post/post.repository.knex'; import type { PostService } from '@/post/post.service'; import type { Site } from '@/site/site.service'; import { generateTestCryptoKeyPair } from '@/test/crypto-key-pair'; @@ -70,6 +78,176 @@ describe('dispatchers', () => { vi.clearAllMocks(); }); + describe('createUndoHandler', () => { + const attackerApId = new URL('https://evil.example/users/attacker'); + const aliceApId = new URL('https://example.com/users/alice'); + const ghostApId = new URL( + 'https://ghost.example/.ghost/activitypub/users/index', + ); + const postApId = new URL('https://ghost.example/post/123'); + + let accountService: AccountService; + let postRepository: KnexPostRepository; + let postService: PostService; + let undoCtx: FedifyContext; + let handler: ReturnType; + let aliceAccount: Account; + let ghostAccount: Account; + + beforeEach(() => { + aliceAccount = { + id: 1, + apId: aliceApId, + } as Account; + + ghostAccount = { + id: 2, + apId: ghostApId, + } as Account; + + accountService = { + getAccountByApId: vi.fn(async (apId: string) => { + if (apId === aliceApId.href) { + return aliceAccount; + } + if (apId === ghostApId.href) { + return ghostAccount; + } + return null; + }), + getByApId: vi.fn(async (apId: URL) => { + if (apId.href === aliceApId.href) { + return aliceAccount; + } + return null; + }), + recordAccountUnfollow: vi.fn(), + } as unknown as AccountService; + + postRepository = { + save: vi.fn(), + } as unknown as KnexPostRepository; + + postService = { + getByApId: vi.fn(), + } as unknown as PostService; + + undoCtx = { + data: { + logger: { + debug: vi.fn(), + warn: vi.fn(), + }, + globaldb: { + set: vi.fn(), + }, + }, + } as unknown as FedifyContext; + + handler = createUndoHandler( + accountService, + postRepository, + postService, + ); + }); + + it('ignores Undo(Follow) when the outer actor does not match the Follow actor', async () => { + const follow = new Follow({ + id: new URL('https://example.com/activities/follow'), + actor: aliceApId, + object: ghostApId, + }); + const undo = new Undo({ + id: new URL('https://evil.example/activities/undo'), + actor: attackerApId, + object: follow, + }); + + await handler(undoCtx, undo); + + expect(accountService.getAccountByApId).not.toHaveBeenCalled(); + expect(accountService.recordAccountUnfollow).not.toHaveBeenCalled(); + expect(undoCtx.data.globaldb.set).not.toHaveBeenCalled(); + }); + + it('processes Undo(Follow) when the outer actor matches the Follow actor', async () => { + const follow = new Follow({ + id: new URL('https://example.com/activities/follow'), + actor: aliceApId, + object: ghostApId, + }); + const undo = new Undo({ + id: new URL('https://example.com/activities/undo'), + actor: aliceApId, + object: follow, + }); + + await handler(undoCtx, undo); + + expect(accountService.getAccountByApId).toHaveBeenCalledWith( + aliceApId.href, + ); + expect(accountService.getAccountByApId).toHaveBeenCalledWith( + ghostApId.href, + ); + expect(undoCtx.data.globaldb.set).toHaveBeenCalledWith( + [undo.id?.href], + await undo.toJsonLd(), + ); + expect(accountService.recordAccountUnfollow).toHaveBeenCalledWith( + ghostAccount, + aliceAccount, + ); + }); + + it('ignores Undo(Announce) when the outer actor does not match the Announce actor', async () => { + const announce = new Announce({ + id: new URL('https://example.com/activities/announce'), + actor: aliceApId, + object: postApId, + }); + const undo = new Undo({ + id: new URL('https://evil.example/activities/undo'), + actor: attackerApId, + object: announce, + }); + + await handler(undoCtx, undo); + + expect(accountService.getByApId).not.toHaveBeenCalled(); + expect(postService.getByApId).not.toHaveBeenCalled(); + expect(postRepository.save).not.toHaveBeenCalled(); + }); + + it('processes Undo(Announce) when the outer actor matches the Announce actor', async () => { + const announce = new Announce({ + id: new URL('https://example.com/activities/announce'), + actor: aliceApId, + object: postApId, + }); + vi.spyOn(announce, 'getActor').mockResolvedValue({ + id: aliceApId, + } as Actor); + const undo = new Undo({ + id: new URL('https://example.com/activities/undo'), + actor: aliceApId, + object: announce, + }); + const post = { + removeRepost: vi.fn(), + } as unknown as Post; + + vi.mocked(postService.getByApId).mockResolvedValue(ok(post)); + + await handler(undoCtx, undo); + + expect(accountService.getByApId).toHaveBeenCalledWith(aliceApId); + expect(postService.getByApId).toHaveBeenCalledWith(postApId); + expect(post.removeRepost).toHaveBeenCalledWith(aliceAccount); + expect(postRepository.save).toHaveBeenCalledWith(post); + }); + }); + describe('likedDispatcher', () => { it('returns an empty array', async () => { const ctx = {