diff --git a/docs/systems/federation.md b/docs/systems/federation.md index eb87d518..836b884e 100644 --- a/docs/systems/federation.md +++ b/docs/systems/federation.md @@ -327,9 +327,10 @@ In a single transaction, `markPeerReset`: **Idempotent / double-reset:** if an *unresolved* `federation_reset_events` row already exists for the origin (the peer reset again before an admin resolved the first), the original `dead_epoch` and `detected_at` are **preserved** (that is the incarnation whose users are already snapshotted) — only the summary counts are refreshed. `dead_epoch` is never overwritten on an unresolved row. A prior *resolved* reset starts a fresh journal entry. -**Detection sources (both wired in this feature):** +**Detection sources (all three wired in this feature):** - **Inbound handshake** — `/peer/accept` landing on an `active`/`needs_attention` row (`routes/federation.ts`): before the idempotent-200 return, `if (reqInstanceId && existing.peerInstanceId && reqInstanceId !== existing.peerInstanceId) markPeerReset(...)`. The guard still returns 200 and **still does not rekey** — the anti-hijack property is preserved verbatim; detection is layered on top. -- **Reachability probe** — `probePeerReachable()` (`utils/federationRecovery.ts`) now parses `instanceId` from the `/api/instance/info` response (returning `{ reachable, instanceId }`; a missing/unparseable epoch is `null`, never an error). The shared decision helper `recoverOrDetectReset(peer, result)` — used by both the background recovery tick (`processRecoveryTick`) and the manual recheck endpoint — routes to `markPeerReset` (returning `'reset_detected'`) when the peer has a non-null `peer_instance_id` and the probed epoch differs, and **does NOT call `markPeerRecovered`**. Rationale: a genuinely reset peer's HMAC secret is desynced, so flipping it back to `active` via a reachability probe would resume relay against a dead secret. Only when the probed epoch matches the baseline (or the baseline is null / epoch unknown) does the normal recovery-to-active path run. The manual recheck endpoint returns `{ recovered: false, status: 'needs_attention' }` on `'reset_detected'`. +- **Reachability probe (`unreachable` peers)** — `probePeerReachable()` (`utils/federationRecovery.ts`) now parses `instanceId` from the `/api/instance/info` response (returning `{ reachable, instanceId }`; a missing/unparseable epoch is `null`, never an error). The shared decision helper `recoverOrDetectReset(peer, result)` — used by both the background recovery tick (`processRecoveryTick`) and the manual recheck endpoint — routes to `markPeerReset` (returning `'reset_detected'`) when the peer has a non-null `peer_instance_id` and the probed epoch differs, and **does NOT call `markPeerRecovered`**. Rationale: a genuinely reset peer's HMAC secret is desynced, so flipping it back to `active` via a reachability probe would resume relay against a dead secret. Only when the probed epoch matches the baseline (or the baseline is null / epoch unknown) does the normal recovery-to-active path run. The manual recheck endpoint returns `{ recovered: false, status: 'needs_attention' }` on `'reset_detected'`. +- **Health-tick probe (`needs_attention` peers)** — `detectResetOnNeedsAttentionPeers()` (`utils/federationRecovery.ts`), called from `processHealthCheckTick` on the 15-minute tick, closes design §4.1's remaining sub-case. A reset peer can reach `needs_attention` via the **auth-failure path** — its HTTP is up but returns 401/403 because the new incarnation has no peer row for us, so `consecutive_auth_failures` crosses `AUTH_FAILURE_THRESHOLD` — **without ever transitioning through `unreachable`**. The `unreachable`-only recovery probe therefore never observes its epoch change, so no journal is ever created; a later manual Re-peer would then run `healResetIncarnation` with no journal row → no heal → the split-brain persists. This pass selects peers with `status='needs_attention'` AND `peer_instance_id IS NOT NULL` AND `needs_attention_reason` not already `peer_reset_detected` (those already carry a journal), probes each (`probePeerReachable`, one `/instance/info` GET per qualifying peer per tick), and calls `markPeerReset` on an observed epoch mismatch. **Detection only:** unlike `recoverOrDetectReset`, it NEVER flips a `needs_attention` peer to `active` (a match / unknown / unreachable result is a pure no-op) — that peer's secret is desynced and only an admin-authenticated re-peer restores trust. It touches neither `peer_instance_id` nor `hmac_secret`. Legacy peers advertise no epoch (`instanceId` null), so detection requires a non-null observed epoch differing from a non-null stored baseline — legacy peers never trigger it, and the existing `auth_failures → needs_attention → manual Reset` path continues unchanged for them. diff --git a/packages/server/src/utils/federationRecovery.test.ts b/packages/server/src/utils/federationRecovery.test.ts index db91d1e1..1cea5e18 100644 --- a/packages/server/src/utils/federationRecovery.test.ts +++ b/packages/server/src/utils/federationRecovery.test.ts @@ -150,4 +150,88 @@ describe('federationRecovery primitives', () => { // A reset peer must NOT be recovered to active. expect(onPeerActivated).not.toHaveBeenCalled(); }); + + // ── detectResetOnNeedsAttentionPeers (design §4.1) ───────────────────────── + // Closes the auth-failure sub-case: a reset peer whose HTTP is up (returning + // 401/403 because the new incarnation has no peer row for us) crosses + // AUTH_FAILURE_THRESHOLD and lands in `needs_attention` WITHOUT ever passing + // through `unreachable`, so the unreachable-only recovery probe never observes + // its epoch change. This pass probes those peers too — detection ONLY, never a + // recover-to-active. + function seedNeedsAttention(id: string, reason: string | null, peerInstanceId: string | null): void { + testDb.insert(schema.federationPeers).values({ + id, origin: 'https://peer.example', hmacSecret: 'secret', + status: 'needs_attention', needsAttentionReason: reason, + peerInstanceId, consecutiveFailures: 0, + lastSyncedAt: Date.now(), createdAt: Date.now(), + }).run(); + } + + it('detectResetOnNeedsAttentionPeers flags an auth-failure peer whose epoch changed (detection only)', async () => { + seedNeedsAttention('peer-na', 'auth_failures', 'E0'); + // A pure replicated stub belonging to the dead incarnation (bare-domain home). + testDb.insert(schema.users).values({ + id: 'stub-1', username: 'carol', displayName: 'carol', + passwordHash: '!federation-replicated', homeInstance: 'peer.example', + createdAt: Date.now(), + }).run(); + vi.spyOn(globalThis, 'fetch').mockResolvedValue(new Response('{"instanceId":"E1"}', { status: 200 })); + + const { detectResetOnNeedsAttentionPeers } = await import('./federationRecovery.js'); + await detectResetOnNeedsAttentionPeers(); + + const row = testDb.select().from(schema.federationPeers) + .where(eq(schema.federationPeers.id, 'peer-na')).get()!; + expect(row.status).toBe('needs_attention'); // NOT flipped to active + expect(row.needsAttentionReason).toBe('peer_reset_detected'); + expect(row.observedPeerInstanceId).toBe('E1'); // observed epoch recorded + expect(row.peerInstanceId).toBe('E0'); // trusted baseline untouched + expect(row.hmacSecret).toBe('secret'); // secret untouched + + const journal = testDb.select().from(schema.federationResetEvents) + .where(eq(schema.federationResetEvents.origin, 'https://peer.example')).get()!; + expect(journal.deadEpoch).toBe('E0'); + expect(journal.resolvedAt).toBeNull(); + + const stub = testDb.select().from(schema.users) + .where(eq(schema.users.id, 'stub-1')).get()!; + expect(stub.federationHealPending).toBe(1); // dead incarnation snapshotted + + expect(onPeerActivated).not.toHaveBeenCalled(); // detection only + }); + + it('detectResetOnNeedsAttentionPeers is a no-op when the probed epoch matches the baseline', async () => { + seedNeedsAttention('peer-same', 'auth_failures', 'E0'); + vi.spyOn(globalThis, 'fetch').mockResolvedValue(new Response('{"instanceId":"E0"}', { status: 200 })); + + const { detectResetOnNeedsAttentionPeers } = await import('./federationRecovery.js'); + await detectResetOnNeedsAttentionPeers(); + + const row = testDb.select().from(schema.federationPeers) + .where(eq(schema.federationPeers.id, 'peer-same')).get()!; + expect(row.status).toBe('needs_attention'); + expect(row.needsAttentionReason).toBe('auth_failures'); // unchanged + expect(testDb.select().from(schema.federationResetEvents).all()).toHaveLength(0); + expect(onPeerActivated).not.toHaveBeenCalled(); + }); + + it('detectResetOnNeedsAttentionPeers skips peers already flagged peer_reset_detected (no probe)', async () => { + seedNeedsAttention('peer-done', 'peer_reset_detected', 'E0'); + const spy = vi.spyOn(globalThis, 'fetch'); + + const { detectResetOnNeedsAttentionPeers } = await import('./federationRecovery.js'); + await detectResetOnNeedsAttentionPeers(); + + expect(spy).not.toHaveBeenCalled(); // already journaled — not re-probed + }); + + it('detectResetOnNeedsAttentionPeers skips peers with a null baseline (nothing to compare)', async () => { + seedNeedsAttention('peer-nobase', 'auth_failures', null); + const spy = vi.spyOn(globalThis, 'fetch'); + + const { detectResetOnNeedsAttentionPeers } = await import('./federationRecovery.js'); + await detectResetOnNeedsAttentionPeers(); + + expect(spy).not.toHaveBeenCalled(); // no trusted baseline → cannot detect a change + }); }); diff --git a/packages/server/src/utils/federationRecovery.ts b/packages/server/src/utils/federationRecovery.ts index c9734173..aec3eace 100644 --- a/packages/server/src/utils/federationRecovery.ts +++ b/packages/server/src/utils/federationRecovery.ts @@ -1,6 +1,6 @@ import { getDb } from '../db/index.js'; import * as schema from '../db/schema.js'; -import { eq } from 'drizzle-orm'; +import { and, eq, isNotNull, isNull, ne, or } from 'drizzle-orm'; import { onPeerActivated } from './federationPeerActivation.js'; import { markPeerReset } from './federationReset.js'; @@ -103,3 +103,62 @@ export async function recoverOrDetectReset( await markPeerRecovered(peer.id); return 'recovered'; } + +/** + * Detection-only epoch probe for peers already parked in `needs_attention` + * (design §4.1). Runs on the 15-minute health-check tick. + * + * The gap this closes: a reset peer can reach `needs_attention` via the + * AUTH-FAILURE path — its HTTP is up and returning 401/403 because the new + * incarnation has no peer row for us, so `consecutive_auth_failures` crosses + * `AUTH_FAILURE_THRESHOLD` — WITHOUT ever transitioning through `unreachable`. + * The `unreachable`-only recovery probe (`processRecoveryTick`) therefore never + * sees such a peer, so its epoch change is never observed and no + * `federation_reset_events` journal is ever created. A later manual admin + * Re-peer would then run `healResetIncarnation` with no journal row → no heal → + * the stale-friendship / split-DM split-brain persists for this sub-case. This + * pass probes those peers so the journal is created at detection time. + * + * **Detection only — never a recover-to-active.** Unlike `recoverOrDetectReset`, + * this NEVER flips a peer to `active`: a `needs_attention` peer's HMAC secret is + * desynced, so a matching or unknown epoch means "still broken, still needs an + * admin," not "recovered." On an observed epoch mismatch it calls + * `markPeerReset` (snapshot + journal + admin notify) and nothing else; the + * trusted baseline (`peer_instance_id`) and `hmac_secret` are left untouched. + * On a match / unknown / unreachable result it does nothing at all. + * + * Candidate set (deliberately small — one `/instance/info` GET per peer per + * tick): `status='needs_attention'` AND `peer_instance_id IS NOT NULL` (a null + * baseline has nothing to compare against) AND the reason is not already + * `peer_reset_detected` (those peers already carry a journal — re-probing would + * be wasted work). Peers whose reason is `auth_failures` or NULL qualify. + */ +export async function detectResetOnNeedsAttentionPeers(signal?: AbortSignal): Promise { + const db = getDb(); + const peers = db + .select({ + id: schema.federationPeers.id, + origin: schema.federationPeers.origin, + peerInstanceId: schema.federationPeers.peerInstanceId, + }) + .from(schema.federationPeers) + .where(and( + eq(schema.federationPeers.status, 'needs_attention'), + isNotNull(schema.federationPeers.peerInstanceId), + or( + isNull(schema.federationPeers.needsAttentionReason), + ne(schema.federationPeers.needsAttentionReason, 'peer_reset_detected'), + ), + )) + .all(); + + for (const peer of peers) { + const result = await probePeerReachable(peer.origin, signal); + // Detection fires ONLY on a reachable peer advertising a non-null epoch that + // differs from the trusted baseline. Everything else (unreachable, unknown + // epoch, or a matching epoch) is a no-op — no recover-to-active from here. + if (result.reachable && result.instanceId && peer.peerInstanceId && result.instanceId !== peer.peerInstanceId) { + markPeerReset(peer.id, peer.origin, peer.peerInstanceId, result.instanceId); + } + } +} diff --git a/packages/server/src/utils/federationWorker.ts b/packages/server/src/utils/federationWorker.ts index ecfbd02b..90164a26 100644 --- a/packages/server/src/utils/federationWorker.ts +++ b/packages/server/src/utils/federationWorker.ts @@ -12,7 +12,7 @@ import { connectionManager } from '../ws/handler.js'; import { generateThumbnail } from './thumbnail.js'; import type { FederationRelayRequest, FederationRelayResponse, FederationRelayEvent } from '@backspace/shared'; import { startupBootstrapSync, onPeerDeactivated } from './federationPeerActivation.js'; -import { probePeerReachable, recoverOrDetectReset } from './federationRecovery.js'; +import { probePeerReachable, recoverOrDetectReset, detectResetOnNeedsAttentionPeers } from './federationRecovery.js'; import { backfillReplicatedProfileAssets } from '../routes/federation.js'; import { invokePermanentFailureCallback } from './federationRollback.js'; import { refreshPeerEpochs, getInstanceId } from './federationEpoch.js'; @@ -1180,6 +1180,17 @@ async function processHealthCheckTick(): Promise { // relay/user activity. Best-effort — a failed fetch is a benign no-op retried // next tick, so it never disturbs the rest of the health-check work. await refreshPeerEpochs().catch(() => {}); + + // ── Reset detection for needs_attention peers (design §4.1) ───────────────── + // A reset peer can land in `needs_attention` via the auth-failure path (HTTP + // up, 401/403 from a new incarnation) WITHOUT ever passing through + // `unreachable`, so the unreachable-only recovery probe never observes its + // epoch change. Probe those peers here so a reset journal is created at + // detection time (otherwise a later manual Re-peer heals nothing). Detection + // ONLY — never flips a needs_attention peer to active. Best-effort: a failure + // is a benign no-op retried next tick and must not disturb the rest of the tick. + // No shared abort signal — probePeerReachable carries its own 10s timeout. + await detectResetOnNeedsAttentionPeers().catch(() => {}); } // ─── Federated Call Health Sweep ────────────────────────────────────────────