From 7fe9476e5714af3dbe4292f8a6b0beb6b7a726e3 Mon Sep 17 00:00:00 2001 From: Jannis Braun <151788261+TheZwiss@users.noreply.github.com> Date: Sat, 25 Apr 2026 00:31:34 +0200 Subject: [PATCH] fix(federation): persist peer instance_name on /peer/accept activation paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously /peer/accept read body.instanceName only when queueing for admin approval. The four paths that mutate federation_peers (rejected→active override, awaiting_approval→active, pending→active, new-peer create) all wrote status='active' without persisting instance_name. Result: every peer established via direct handshake had instance_name = NULL forever. Anywhere peerLabel was rendered fell back to origin hostname. Active/needs_attention idempotent early-return path deliberately left alone — same security posture that already refuses to overwrite hmac_secret from unauthenticated requests on already-active peers. Existing live NULL rows are repaired post-deploy via manual UPDATE statements (see plan). --- .../src/routes/federation.peerAccept.test.ts | 229 ++++++++++++++++++ packages/server/src/routes/federation.ts | 9 +- 2 files changed, 235 insertions(+), 3 deletions(-) create mode 100644 packages/server/src/routes/federation.peerAccept.test.ts diff --git a/packages/server/src/routes/federation.peerAccept.test.ts b/packages/server/src/routes/federation.peerAccept.test.ts new file mode 100644 index 00000000..e3c1929e --- /dev/null +++ b/packages/server/src/routes/federation.peerAccept.test.ts @@ -0,0 +1,229 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import Fastify, { type FastifyInstance } from 'fastify'; +import Database from 'better-sqlite3'; +import { drizzle } from 'drizzle-orm/better-sqlite3'; +import { eq } from 'drizzle-orm'; +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import * as schema from '../db/schema.js'; +import { setWorkerId } from '../utils/snowflake.js'; + +setWorkerId(1); + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); + +type TestDb = ReturnType>; +let sqlite: Database.Database; +let testDb: TestDb; + +vi.mock('../db/index.js', () => ({ + getDb: () => testDb, + getRawDb: () => sqlite, + schema, +})); + +vi.mock('../utils/auth.js', () => ({ + authenticate: async (req: { userId?: string }) => { + req.userId = 'admin-user'; + }, + requireAdmin: async () => { + // no-op (peer/accept endpoint is unauthenticated anyway) + }, +})); + +vi.mock('../ws/handler.js', () => ({ + connectionManager: { + sendToAdmins: vi.fn(), + getAllOnlineUserIds: () => [], + sendToUser: vi.fn(), + sendToDmMembers: vi.fn(), + }, +})); + +vi.mock('../utils/federationPeerActivation.js', () => ({ + onPeerActivated: vi.fn(async () => undefined), +})); + +function applyMigrations(db: Database.Database): void { + const migrationsDir = path.resolve(__dirname, '../../drizzle'); + const files = fs.readdirSync(migrationsDir).filter(f => f.endsWith('.sql')).sort(); + for (const f of files) { + const sqlText = fs.readFileSync(path.join(migrationsDir, f), 'utf8'); + const statements = sqlText.split(/-->\s*statement-breakpoint/); + for (const stmt of statements) { + const clean = stmt.trim(); + if (clean) db.exec(clean); + } + } +} + +function seedInstanceSettings(name: string): void { + testDb.insert(schema.instanceSettings).values({ + id: 1, + instanceName: name, + autoAcceptPeering: 1, + registrationOpen: 1, + updatedAt: Date.now(), + }).run(); +} + +async function buildApp(): Promise { + const app = Fastify({ logger: false }); + const { federationRoutes } = await import('./federation.js'); + await app.register(federationRoutes); + await app.ready(); + return app; +} + +describe('POST /api/federation/peer/accept — instance_name persistence', () => { + let app: FastifyInstance; + + beforeEach(async () => { + sqlite = new Database(':memory:'); + testDb = drizzle(sqlite, { schema }); + applyMigrations(sqlite); + seedInstanceSettings('Local Backspace'); + app = await buildApp(); + }); + + it('writes instance_name when creating a new peer (autoAccept=1, no existing row)', async () => { + const response = await app.inject({ + method: 'POST', + url: '/api/federation/peer/accept', + payload: { + sourceOrigin: 'https://remote.example', + hmacSecret: 'remote-secret', + instanceName: 'Remote Backspace', + }, + }); + + expect(response.statusCode).toBe(200); + const row = testDb.select().from(schema.federationPeers) + .where(eq(schema.federationPeers.origin, 'https://remote.example')).get(); + expect(row).toBeTruthy(); + expect(row?.instanceName).toBe('Remote Backspace'); + expect(row?.status).toBe('active'); + }); + + it('writes instance_name when activating an existing pending peer', async () => { + testDb.insert(schema.federationPeers).values({ + id: 'peer-pending', + origin: 'https://remote.example', + hmacSecret: 'old-secret', + status: 'pending', + createdAt: Date.now(), + }).run(); + + const response = await app.inject({ + method: 'POST', + url: '/api/federation/peer/accept', + payload: { + sourceOrigin: 'https://remote.example', + hmacSecret: 'new-secret', + instanceName: 'Remote Backspace', + }, + }); + + expect(response.statusCode).toBe(200); + const row = testDb.select().from(schema.federationPeers) + .where(eq(schema.federationPeers.id, 'peer-pending')).get(); + expect(row?.instanceName).toBe('Remote Backspace'); + expect(row?.status).toBe('active'); + }); + + it('writes instance_name when activating an existing awaiting_approval peer', async () => { + testDb.insert(schema.federationPeers).values({ + id: 'peer-await', + origin: 'https://remote.example', + hmacSecret: 'old-secret', + status: 'awaiting_approval', + createdAt: Date.now(), + }).run(); + + const response = await app.inject({ + method: 'POST', + url: '/api/federation/peer/accept', + payload: { + sourceOrigin: 'https://remote.example', + hmacSecret: 'new-secret', + instanceName: 'Remote Backspace', + }, + }); + + expect(response.statusCode).toBe(200); + const row = testDb.select().from(schema.federationPeers) + .where(eq(schema.federationPeers.id, 'peer-await')).get(); + expect(row?.instanceName).toBe('Remote Backspace'); + expect(row?.status).toBe('active'); + }); + + it('writes instance_name when overriding rejected → active', async () => { + testDb.insert(schema.federationPeers).values({ + id: 'peer-rejected', + origin: 'https://remote.example', + hmacSecret: 'old-secret', + status: 'rejected', + createdAt: Date.now(), + }).run(); + + const response = await app.inject({ + method: 'POST', + url: '/api/federation/peer/accept', + payload: { + sourceOrigin: 'https://remote.example', + hmacSecret: 'new-secret', + instanceName: 'Remote Backspace', + }, + }); + + expect(response.statusCode).toBe(200); + const row = testDb.select().from(schema.federationPeers) + .where(eq(schema.federationPeers.id, 'peer-rejected')).get(); + expect(row?.instanceName).toBe('Remote Backspace'); + expect(row?.status).toBe('active'); + }); + + it('does NOT overwrite instance_name on the active idempotent early-return path', async () => { + testDb.insert(schema.federationPeers).values({ + id: 'peer-active', + origin: 'https://remote.example', + hmacSecret: 'existing-secret', + status: 'active', + instanceName: 'Original Name', + createdAt: Date.now(), + }).run(); + + const response = await app.inject({ + method: 'POST', + url: '/api/federation/peer/accept', + payload: { + sourceOrigin: 'https://remote.example', + hmacSecret: 'attacker-secret', + instanceName: 'Attacker Name', + }, + }); + + expect(response.statusCode).toBe(200); + const row = testDb.select().from(schema.federationPeers) + .where(eq(schema.federationPeers.id, 'peer-active')).get(); + expect(row?.instanceName).toBe('Original Name'); + expect(row?.hmacSecret).toBe('existing-secret'); + }); + + it('writes null instance_name when body omits the field', async () => { + const response = await app.inject({ + method: 'POST', + url: '/api/federation/peer/accept', + payload: { + sourceOrigin: 'https://remote.example', + hmacSecret: 'remote-secret', + }, + }); + + expect(response.statusCode).toBe(200); + const row = testDb.select().from(schema.federationPeers) + .where(eq(schema.federationPeers.origin, 'https://remote.example')).get(); + expect(row?.instanceName).toBeNull(); + }); +}); diff --git a/packages/server/src/routes/federation.ts b/packages/server/src/routes/federation.ts index b14a9121..f01d7291 100644 --- a/packages/server/src/routes/federation.ts +++ b/packages/server/src/routes/federation.ts @@ -394,7 +394,7 @@ export async function federationRoutes(app: FastifyInstance): Promise { // ─── POST /api/federation/peer/accept ────────────────────────────────────── // Server-to-server: accept a peering request from a remote instance. // No JWT auth — this is first contact. Rate-limited by IP. - app.post<{ Body: { sourceOrigin: string; challenge?: string; hmacSecret: string } }>( + app.post<{ Body: { sourceOrigin: string; challenge?: string; hmacSecret: string; instanceName?: string } }>( '/api/federation/peer/accept', async (request, reply) => { const clientIp = request.ip; @@ -405,7 +405,7 @@ export async function federationRoutes(app: FastifyInstance): Promise { }); } - const { sourceOrigin: rawOrigin, hmacSecret } = request.body ?? {}; + const { sourceOrigin: rawOrigin, hmacSecret, instanceName: reqInstanceName } = request.body ?? {}; if (!rawOrigin || typeof rawOrigin !== 'string') { return reply.code(400).send({ error: 'sourceOrigin is required', statusCode: 400 }); @@ -471,7 +471,6 @@ export async function federationRoutes(app: FastifyInstance): Promise { } // Queue for admin approval — upsert into peer_approval_requests - const { instanceName: reqInstanceName } = request.body as { instanceName?: string }; const now = Date.now(); const THIRTY_DAYS_MS = 30 * 24 * 60 * 60 * 1000; @@ -551,6 +550,7 @@ export async function federationRoutes(app: FastifyInstance): Promise { db.update(schema.federationPeers) .set({ hmacSecret, + instanceName: reqInstanceName ?? null, status: 'active', lastSeenAt: Date.now(), }) @@ -577,6 +577,7 @@ export async function federationRoutes(app: FastifyInstance): Promise { db.update(schema.federationPeers) .set({ hmacSecret, + instanceName: reqInstanceName ?? null, status: 'active', lastSeenAt: Date.now(), }) @@ -602,6 +603,7 @@ export async function federationRoutes(app: FastifyInstance): Promise { db.update(schema.federationPeers) .set({ hmacSecret, + instanceName: reqInstanceName ?? null, status: 'active', lastSeenAt: Date.now(), }) @@ -622,6 +624,7 @@ export async function federationRoutes(app: FastifyInstance): Promise { id: peerId, origin: sourceOrigin, hmacSecret, + instanceName: reqInstanceName ?? null, status: 'active', lastSeenAt: Date.now(), createdAt: Date.now(),