From 54ab660204c94dee4ed49d2f37a2f6b6d6351599 Mon Sep 17 00:00:00 2001 From: Jannis Braun <151788261+TheZwiss@users.noreply.github.com> Date: Thu, 2 Jul 2026 20:10:35 +0200 Subject: [PATCH] =?UTF-8?q?feat(federation):=20near-instant=20reset=20dete?= =?UTF-8?q?ction=20=E2=80=94=20probe=20epoch=20at=20the=20auth-failure=20t?= =?UTF-8?q?ransition=20+=20on=20worker=20startup?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A reset peer reaches needs_attention via the auth-failure path (HMAC desynced by the new incarnation) without passing through unreachable, so the 5s recovery probe never saw it — detection waited up to a full 15-min health-check cycle before 'Re-peer & heal' surfaced. Extract detectResetForPeer() and fire it event-driven at the transition, plus a startup sweep for already-stuck peers. 15-min tick remains the backstop. --- docs/systems/federation.md | 6 +- .../src/utils/federationRecovery.test.ts | 62 +++++++++++++++++++ .../server/src/utils/federationRecovery.ts | 46 +++++++++++--- packages/server/src/utils/federationWorker.ts | 34 +++++++++- 4 files changed, 138 insertions(+), 10 deletions(-) diff --git a/docs/systems/federation.md b/docs/systems/federation.md index 87f9feea..1e882194 100644 --- a/docs/systems/federation.md +++ b/docs/systems/federation.md @@ -332,7 +332,11 @@ In a single transaction, `markPeerReset`: **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 (`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`. +- **`needs_attention`-peer reset probe** — 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 5-second recovery probe therefore never observes its epoch change, so no journal is created; a later manual Re-peer would then run `healResetIncarnation` with no journal row → no heal → the split-brain persists. The shared per-peer unit is **`detectResetForPeer(peer)`** (`utils/federationRecovery.ts`): for a peer with a non-null baseline it runs one `probePeerReachable` (`/instance/info` GET) and calls `markPeerReset` on an observed epoch mismatch. It is invoked from **three** places, so detection latency is near-zero rather than up to a full health-check cycle: + 1. **Event-driven, at the transition** — the instant the outbox worker moves a peer to `needs_attention` on the auth-failure threshold (`federationWorker.ts`), it fires `detectResetForPeer` for that peer (fire-and-forget). This is the common live case: the moment the connection is declared broken, the epoch is checked and "Re-peer & heal" surfaces immediately. + 2. **Worker-startup sweep** — `startFederationWorkers()` runs `detectResetOnNeedsAttentionPeers()` once on boot, catching any peer already parked in `needs_attention` (reset while this instance was down, or transitioned before this probe shipped). + 3. **15-minute health-tick backstop** — `detectResetOnNeedsAttentionPeers()` also still runs at the end of `processHealthCheckTick` as the periodic safety net. It 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) and calls `detectResetForPeer` on each. + **Detection only:** unlike `recoverOrDetectReset`, none of these ever 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. Neither `peer_instance_id` nor `hmac_secret` is touched. 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 1cea5e18..fdd7b178 100644 --- a/packages/server/src/utils/federationRecovery.test.ts +++ b/packages/server/src/utils/federationRecovery.test.ts @@ -234,4 +234,66 @@ describe('federationRecovery primitives', () => { expect(spy).not.toHaveBeenCalled(); // no trusted baseline → cannot detect a change }); + + // ── detectResetForPeer (per-peer unit) ───────────────────────────────────── + // The shared single-peer probe fired the instant a peer crosses into + // needs_attention via the auth-failure path (event-driven, in federationWorker) + // and by the worker-startup sweep — collapsing reset-detection latency from a + // 15-minute health-check cycle to one /instance/info GET. + + it('detectResetForPeer returns true and journals the reset when the probed epoch differs', async () => { + seedNeedsAttention('peer-evt', 'auth_failures', 'E0'); + vi.spyOn(globalThis, 'fetch').mockResolvedValue(new Response('{"instanceId":"E1"}', { status: 200 })); + + const { detectResetForPeer } = await import('./federationRecovery.js'); + const detected = await detectResetForPeer({ id: 'peer-evt', origin: 'https://peer.example', peerInstanceId: 'E0' }); + + expect(detected).toBe(true); + const row = testDb.select().from(schema.federationPeers) + .where(eq(schema.federationPeers.id, 'peer-evt')).get()!; + expect(row.status).toBe('needs_attention'); // detection only — never flipped to active + expect(row.needsAttentionReason).toBe('peer_reset_detected'); + expect(row.observedPeerInstanceId).toBe('E1'); + expect(row.peerInstanceId).toBe('E0'); // trusted baseline untouched + expect(row.hmacSecret).toBe('secret'); // never rekeyed + expect(testDb.select().from(schema.federationResetEvents) + .where(eq(schema.federationResetEvents.origin, 'https://peer.example')).get()!.resolvedAt).toBeNull(); + expect(onPeerActivated).not.toHaveBeenCalled(); + }); + + it('detectResetForPeer returns false and is a no-op when the probed epoch matches the baseline', async () => { + seedNeedsAttention('peer-evt-same', 'auth_failures', 'E0'); + vi.spyOn(globalThis, 'fetch').mockResolvedValue(new Response('{"instanceId":"E0"}', { status: 200 })); + + const { detectResetForPeer } = await import('./federationRecovery.js'); + const detected = await detectResetForPeer({ id: 'peer-evt-same', origin: 'https://peer.example', peerInstanceId: 'E0' }); + + expect(detected).toBe(false); + const row = testDb.select().from(schema.federationPeers) + .where(eq(schema.federationPeers.id, 'peer-evt-same')).get()!; + expect(row.needsAttentionReason).toBe('auth_failures'); // unchanged + expect(testDb.select().from(schema.federationResetEvents).all()).toHaveLength(0); + }); + + it('detectResetForPeer returns false WITHOUT probing when the baseline is null', async () => { + const spy = vi.spyOn(globalThis, 'fetch'); + const { detectResetForPeer } = await import('./federationRecovery.js'); + const detected = await detectResetForPeer({ id: 'peer-evt-nobase', origin: 'https://peer.example', peerInstanceId: null }); + + expect(detected).toBe(false); + expect(spy).not.toHaveBeenCalled(); // no baseline → cannot detect a change, no wasted GET + }); + + it('detectResetForPeer returns false when the peer is unreachable (no false reset)', async () => { + seedNeedsAttention('peer-evt-down', 'auth_failures', 'E0'); + vi.spyOn(globalThis, 'fetch').mockRejectedValue(new Error('ENOTFOUND')); + + const { detectResetForPeer } = await import('./federationRecovery.js'); + const detected = await detectResetForPeer({ id: 'peer-evt-down', origin: 'https://peer.example', peerInstanceId: 'E0' }); + + expect(detected).toBe(false); + expect(testDb.select().from(schema.federationResetEvents).all()).toHaveLength(0); + expect(testDb.select().from(schema.federationPeers) + .where(eq(schema.federationPeers.id, 'peer-evt-down')).get()!.needsAttentionReason).toBe('auth_failures'); + }); }); diff --git a/packages/server/src/utils/federationRecovery.ts b/packages/server/src/utils/federationRecovery.ts index aec3eace..2ae64d04 100644 --- a/packages/server/src/utils/federationRecovery.ts +++ b/packages/server/src/utils/federationRecovery.ts @@ -153,12 +153,44 @@ export async function detectResetOnNeedsAttentionPeers(signal?: AbortSignal): Pr .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); - } + await detectResetForPeer(peer, signal); } } + +/** + * Detection-only epoch probe for a SINGLE `needs_attention` peer — the per-peer + * unit shared by the health-tick sweep (`detectResetOnNeedsAttentionPeers`), the + * worker-startup sweep, and the event-driven probe fired the instant a peer + * crosses into `needs_attention` via the auth-failure path (`federationWorker`). + * + * The auth-failure transition is the case this exists for: a reset peer whose + * HTTP is up but whose HMAC is desynced accrues 401/403s and lands in + * `needs_attention` WITHOUT ever passing through `unreachable`, so the 5-second + * unreachable-only recovery probe never sees it. Before this probe fired at the + * transition, such a peer waited up to a full 15-minute health-check cycle before + * its reset was detected and the admin surface offered "Re-peer & heal". Probing + * at the transition collapses that latency to a single `/instance/info` GET. + * + * Detection fires ONLY on a reachable peer advertising a non-null epoch that + * differs from the trusted baseline (`peer_instance_id`). Everything else — + * unreachable, unknown/absent epoch, a matching epoch, or a null baseline (which + * has nothing to compare against) — is a no-op. NEVER flips a peer to `active`: a + * `needs_attention` peer's HMAC secret is desynced, so a match/unknown means + * "still broken, still needs an admin," not "recovered." On a confirmed epoch + * mismatch it calls `markPeerReset` (snapshot + journal + admin notify) and + * nothing else — the baseline and `hmac_secret` are left untouched. + * + * @returns `true` if a reset was detected and `markPeerReset` was called. + */ +export async function detectResetForPeer( + peer: { id: string; origin: string; peerInstanceId: string | null }, + signal?: AbortSignal, +): Promise { + if (!peer.peerInstanceId) return false; // no baseline → nothing to compare against + const result = await probePeerReachable(peer.origin, signal); + if (result.reachable && result.instanceId && result.instanceId !== peer.peerInstanceId) { + markPeerReset(peer.id, peer.origin, peer.peerInstanceId, result.instanceId); + return true; + } + return false; +} diff --git a/packages/server/src/utils/federationWorker.ts b/packages/server/src/utils/federationWorker.ts index 90164a26..8d821557 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, detectResetOnNeedsAttentionPeers } from './federationRecovery.js'; +import { probePeerReachable, recoverOrDetectReset, detectResetOnNeedsAttentionPeers, detectResetForPeer } from './federationRecovery.js'; import { backfillReplicatedProfileAssets } from '../routes/federation.js'; import { invokePermanentFailureCallback } from './federationRollback.js'; import { refreshPeerEpochs, getInstanceId } from './federationEpoch.js'; @@ -347,7 +347,10 @@ export async function processOutboxTick(): Promise { // needs_attention; bounded retry (AUTH_FAILURE_THRESHOLD) rides out // transient clock skew and rotation-grace edge races. const currentRow = db - .select({ consecutiveAuthFailures: schema.federationPeers.consecutiveAuthFailures }) + .select({ + consecutiveAuthFailures: schema.federationPeers.consecutiveAuthFailures, + peerInstanceId: schema.federationPeers.peerInstanceId, + }) .from(schema.federationPeers) .where(eq(schema.federationPeers.id, peerId)) .get(); @@ -369,6 +372,23 @@ export async function processOutboxTick(): Promise { `[federation-worker] Peer ${peerOrigin} transitioned to needs_attention after ${decision.newAuthFailures} consecutive ${response.status} responses`, ); + // Event-driven reset detection: a genuinely reset peer reaches + // needs_attention via THIS auth-failure path (HMAC desynced by the new + // incarnation) without ever passing through `unreachable`, so the + // 5-second unreachable-only recovery probe never sees it. Probe its + // epoch NOW — the instant the connection is declared broken — instead of + // waiting up to a full 15-minute health-check cycle for the backstop + // sweep. Detection-only (markPeerReset); never flips back to active. + // Fire-and-forget: a probe failure is a benign no-op the 15-min tick + // retries, and it must not stall the outbox loop. + detectResetForPeer({ + id: peerId, + origin: peerOrigin, + peerInstanceId: currentRow?.peerInstanceId ?? null, + }).catch(err => + console.error('[federation-worker] reset probe on auth-threshold transition failed:', err) + ); + const contextMap = buildContextMapForPeer(db, peerId); if (contextMap.size > 0) { pushPeerRejectedEvent( @@ -1275,6 +1295,16 @@ export function startFederationWorkers(): void { console.error('[federation-worker] Startup bootstrap sync error:', err); }); + // Startup reset-detection sweep: probe every peer already parked in + // `needs_attention` for an epoch change. This catches a peer that was reset + // while this instance was down (so no live transition fired) AND any peer that + // crossed into needs_attention before this build shipped the event-driven + // probe — surfacing "Re-peer & heal" immediately on boot instead of on the + // next 15-minute health-check cycle. Best-effort, detection-only. + detectResetOnNeedsAttentionPeers().catch((err) => { + console.error('[federation-worker] Startup reset-detection sweep error:', err); + }); + // Backfill any replicated user avatars/banners still stored as absolute URLs // (legacy data from before file replication, or rows whose home was offline // on a previous attempt). Best-effort and idempotent — safe to re-run.