From 11c5a2bf06a5088ec2ad204fa5e8032f34b5183b Mon Sep 17 00:00:00 2001 From: Jannis Braun <151788261+TheZwiss@users.noreply.github.com> Date: Sun, 26 Apr 2026 00:12:42 +0200 Subject: [PATCH] fix(federation): refuse outbound handshake when inbound approval pending MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the auto-reconnect trust-bypass: any code path calling ensurePeered(remote) on an instance with autoAcceptPeering=0 could previously bypass the admin gate by initiating a fresh handshake to the remote, which the remote then accepted against its existing awaiting_approval row. The trigger surfaced was stores/instanceStore.ts:1010 — the silent .catch(() => {}) auto-reconnect that fires for any user with the remote in their replicatedInstances (commonly: any admin). Anyone with that profile reloading their session activated peering on both sides without any admin approval action. Surgical fix: ensurePeered now returns rejected when an unresolved inbound peer_approval_requests row exists for the target origin. The legitimate admin-approve flow (routes/federation.ts:1089) does not call ensurePeered; it deletes the approval-request and does its own direct fetch to /peer/accept, so this check does not block legitimate approvals. The receiver-side trust assumption at routes/federation.ts:619-645 (awaiting_approval branch in /peer/accept) still has the same flaw — an adversarial peer that knows the timing could re-handshake at the right moment to flip the receiver to active. That deeper trust-model rework is plan-grade work tracked at internal notes 2026-04-26-peer-handshake-trust-model.md. --- docs/systems/federation.md | 2 + .../federationPeering.trustGuard.test.ts | 136 ++++++++++++++++++ .../server/src/utils/federationPeering.ts | 22 +++ 3 files changed, 160 insertions(+) create mode 100644 packages/server/src/utils/federationPeering.trustGuard.test.ts diff --git a/docs/systems/federation.md b/docs/systems/federation.md index 9ccfdc49..f8deb935 100644 --- a/docs/systems/federation.md +++ b/docs/systems/federation.md @@ -157,6 +157,8 @@ When `autoAcceptPeering` is `false` and an instance calls `POST /api/federation/ **Expiry** — The janitor (`federationJanitor.ts`) runs on its scheduled interval and deletes rows where `expires_at < now`. Expired requests do NOT create a `rejected` peer — the requesting instance can re-submit. Admin denial, by contrast, does create a `rejected` peer record, blocking re-requests until an admin clears it. +**Pre-handshake guard (`ensurePeered`)** — Before any outbound handshake, `ensurePeered(origin)` in `federationPeering.ts` refuses with `{ status: 'rejected', error: 'Local admin must resolve…' }` if a `peer_approval_requests` row exists for that origin. This blocks the auto-reconnect bypass: without the guard, any code path calling `ensurePeered` (e.g., the silent reconnect in `stores/instanceStore.ts`) could initiate a fresh outbound handshake to a peer that has a pending inbound approval request, and the receiver's `/peer/accept` `awaiting_approval` branch would activate the relationship without admin involvement. The legitimate approve flow (`POST /api/federation/approval-requests/:id/approve`) does NOT call `ensurePeered` — it deletes the approval-request row and does its own direct `fetch` to `/peer/accept` — so the guard does not block legitimate approvals. Note: the guard closes the trigger only; the receiver-side trust assumption in the `/peer/accept` `awaiting_approval` branch is still permissive and is tracked separately at `docs/superpowers/problems/2026-04-26-peer-handshake-trust-model.md`. + ### Admin Endpoints | Endpoint | Method | Auth | Purpose | diff --git a/packages/server/src/utils/federationPeering.trustGuard.test.ts b/packages/server/src/utils/federationPeering.trustGuard.test.ts new file mode 100644 index 00000000..56b5cfe9 --- /dev/null +++ b/packages/server/src/utils/federationPeering.trustGuard.test.ts @@ -0,0 +1,136 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import Database from 'better-sqlite3'; +import { drizzle } from 'drizzle-orm/better-sqlite3'; +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 './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, + schema, +})); + +vi.mock('./federationAuth.js', async () => { + const actual = await vi.importActual('./federationAuth.js'); + return { + ...actual, + getOurOrigin: () => 'https://local.example', + generateHmacSecret: () => 'mock-hmac-secret', + }; +}); + +vi.mock('../routes/federation.js', () => ({ + validateOrigin: (raw: string) => { + try { + const url = new URL(raw); + return url.origin; + } catch { + return null; + } + }, +})); + +vi.mock('../ws/handler.js', () => ({ + connectionManager: { + sendToAdmins: vi.fn(), + sendToUser: vi.fn(), + getAllOnlineUserIds: () => [], + }, +})); + +vi.mock('./federationPeerActivation.js', () => ({ + onPeerActivated: vi.fn(async () => undefined), + onPeerDeactivated: 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(): void { + testDb.insert(schema.instanceSettings).values({ + id: 1, + instanceName: 'Local Backspace', + autoAcceptPeering: 1, + registrationOpen: 1, + updatedAt: Date.now(), + }).run(); +} + +describe('ensurePeered — refuses when unresolved inbound approval-request exists', () => { + beforeEach(() => { + sqlite = new Database(':memory:'); + testDb = drizzle(sqlite, { schema }); + applyMigrations(sqlite); + seedInstanceSettings(); + }); + + afterEach(() => { + vi.restoreAllMocks(); + sqlite.close(); + }); + + it('returns rejected when a peer_approval_requests row exists for the target origin, even if no peer row exists', async () => { + const now = Date.now(); + testDb.insert(schema.peerApprovalRequests).values({ + id: 'approval-1', + origin: 'https://orbit.test', + instanceName: 'Orbit', + hmacSecret: 'a'.repeat(64), + requestedAt: now, + expiresAt: now + 30 * 24 * 60 * 60 * 1000, + }).run(); + + const fetchSpy = vi.spyOn(globalThis, 'fetch'); + + const { ensurePeered, _clearInFlightPeering } = await import('./federationPeering.js'); + _clearInFlightPeering(); + const result = await ensurePeered('https://orbit.test'); + + expect(result.status).toBe('rejected'); + if (result.status === 'rejected') { + expect(result.error).toMatch(/admin must resolve/i); + } + + // Refusal is BEFORE performHandshake — no peer row created, no outbound POST. + const peers = testDb.select().from(schema.federationPeers).all(); + expect(peers).toHaveLength(0); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + it('does not block when there is no inbound approval-request', async () => { + // No approval-request row, no existing peer. Should proceed to performHandshake. + // Stub fetch with a network failure so the handshake resolves as 'failed' + // (transient) rather than reaching any 4xx/5xx branches. + vi.stubGlobal('fetch', vi.fn(async () => { + throw new TypeError('fetch failed'); + })); + + const { ensurePeered, _clearInFlightPeering } = await import('./federationPeering.js'); + _clearInFlightPeering(); + const result = await ensurePeered('https://nopeer.test'); + + // Reached performHandshake — failure mode is 'failed' (network), NOT + // the pre-handshake 'rejected' from the new guard. + expect(result.status).not.toBe('rejected'); + expect(result.status).toBe('failed'); + }); +}); diff --git a/packages/server/src/utils/federationPeering.ts b/packages/server/src/utils/federationPeering.ts index 4e8ea932..2fbda0a5 100644 --- a/packages/server/src/utils/federationPeering.ts +++ b/packages/server/src/utils/federationPeering.ts @@ -84,6 +84,28 @@ export async function ensurePeered(origin: string): Promise } } + // Pre-handshake gate: refuse if we have an unresolved inbound approval-request + // for this origin. The local admin must approve or deny it first. Without this + // check, any code path calling ensurePeered (e.g., the silent auto-reconnect + // in stores/instanceStore.ts) could bypass autoAcceptPeering=0 by initiating a + // fresh handshake to the remote, which the remote then accepts against its + // existing awaiting_approval row (routes/federation.ts /peer/accept branch). + // The legitimate approval flow (routes/federation.ts /approval-requests/:id/ + // approve) does NOT call ensurePeered — it deletes the approval-request first + // and does its own fetch — so this guard does not block legitimate approvals. + const pendingInbound = db + .select({ id: schema.peerApprovalRequests.id }) + .from(schema.peerApprovalRequests) + .where(eq(schema.peerApprovalRequests.origin, normalized)) + .get(); + + if (pendingInbound) { + return { + status: 'rejected', + error: 'Local admin must resolve pending peering approval before initiating', + }; + } + // Deduplicate: if a handshake is already in flight, share the promise const inflight = inFlightPeering.get(normalized); if (inflight) {