fix(federation): friend-add returns graceful 503 instead of 500 on peer lookup failure (BUG-3)
lookupRemoteUser now maps peer HTTP failures (403/5xx, malformed body) to a
structured {ok:false,reason:'unreachable'} instead of throwing, and the
federated friend-add wraps the call in try/catch as defense-in-depth. A
desynced/unreachable peer no longer surfaces as a raw 500 on a user action.
README.md left unstaged.
This commit is contained in:
@@ -274,6 +274,19 @@ describe('POST /api/social/requests — federated branch (lookup failures)', ()
|
|||||||
expect(JSON.parse(res.body).error).toBe('peer_unreachable');
|
expect(JSON.parse(res.body).error).toBe('peer_unreachable');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('returns 503 (not 500) when lookup throws unexpectedly — defense-in-depth (BUG-3)', async () => {
|
||||||
|
lookupRemoteUserMock.mockRejectedValue(new Error('boom: peer returned HTTP 403'));
|
||||||
|
const app = await buildApp();
|
||||||
|
const res = await app.inject({
|
||||||
|
method: 'POST',
|
||||||
|
url: '/api/social/requests',
|
||||||
|
payload: { username: 'alice@orbit.test' },
|
||||||
|
});
|
||||||
|
expect(res.statusCode).toBe(503);
|
||||||
|
expect(JSON.parse(res.body).error).toBe('peer_unreachable');
|
||||||
|
expect(testDb.select().from(schema.friendRequests).all()).toHaveLength(0);
|
||||||
|
});
|
||||||
|
|
||||||
it('returns 429 lookup_rate_limited with Retry-After header when lookup returns rate_limited', async () => {
|
it('returns 429 lookup_rate_limited with Retry-After header when lookup returns rate_limited', async () => {
|
||||||
lookupRemoteUserMock.mockResolvedValue({ ok: false, reason: 'rate_limited', retryAfter: 30 });
|
lookupRemoteUserMock.mockResolvedValue({ ok: false, reason: 'rate_limited', retryAfter: 30 });
|
||||||
const app = await buildApp();
|
const app = await buildApp();
|
||||||
|
|||||||
@@ -241,8 +241,19 @@ async function handleFederatedFriendRequest(
|
|||||||
}
|
}
|
||||||
// peering.status === 'active' — continue
|
// peering.status === 'active' — continue
|
||||||
|
|
||||||
// 3. Lookup
|
// 3. Lookup — a peer's HTTP/transport failure must never surface as a raw 500
|
||||||
const lookup = await lookupRemoteUser(peerOrigin, baseName);
|
// on a user action. lookupRemoteUser already maps peer HTTP failures (403/5xx,
|
||||||
|
// malformed body) to a structured `unreachable`; this try/catch is
|
||||||
|
// defense-in-depth so that any *unexpected* throw (e.g. a missing peer row) is
|
||||||
|
// still returned to the user as a graceful 503 rather than an Internal Server
|
||||||
|
// Error. (BUG-3, 2026-07-02: a desynced peer returned 403 → unhandled throw → 500.)
|
||||||
|
let lookup: Awaited<ReturnType<typeof lookupRemoteUser>>;
|
||||||
|
try {
|
||||||
|
lookup = await lookupRemoteUser(peerOrigin, baseName);
|
||||||
|
} catch (err) {
|
||||||
|
console.error(`[social] federated friend-add lookup failed for ${peerOrigin}:`, err);
|
||||||
|
return reply.code(503).send({ error: 'peer_unreachable', statusCode: 503, domain: targetDomain });
|
||||||
|
}
|
||||||
if (!lookup.ok) {
|
if (!lookup.ok) {
|
||||||
if (lookup.reason === 'not_found') {
|
if (lookup.reason === 'not_found') {
|
||||||
return reply.code(404).send({ error: 'user_not_found', statusCode: 404, domain: targetDomain, handle: baseName });
|
return reply.code(404).send({ error: 'user_not_found', statusCode: 404, domain: targetDomain, handle: baseName });
|
||||||
|
|||||||
@@ -202,7 +202,33 @@ describe('lookupRemoteUser', () => {
|
|||||||
expect(headers['X-Federation-Timestamp']).toMatch(/^\d+$/);
|
expect(headers['X-Federation-Timestamp']).toMatch(/^\d+$/);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('8. throws on malformed 200 body', async () => {
|
it('9. returns ok:false reason:unreachable on HTTP 403 (peer rejects our auth / desync) — must NOT throw', async () => {
|
||||||
|
vi.stubGlobal('fetch', vi.fn().mockResolvedValue({
|
||||||
|
ok: false,
|
||||||
|
status: 403,
|
||||||
|
headers: { get: () => null },
|
||||||
|
}));
|
||||||
|
|
||||||
|
const { lookupRemoteUser } = await import('./federationLookup.js');
|
||||||
|
const result = await lookupRemoteUser(PEER_ORIGIN, 'bob');
|
||||||
|
|
||||||
|
expect(result).toEqual({ ok: false, reason: 'unreachable' });
|
||||||
|
});
|
||||||
|
|
||||||
|
it('10. returns ok:false reason:unreachable on HTTP 500 (peer error) — must NOT throw', async () => {
|
||||||
|
vi.stubGlobal('fetch', vi.fn().mockResolvedValue({
|
||||||
|
ok: false,
|
||||||
|
status: 500,
|
||||||
|
headers: { get: () => null },
|
||||||
|
}));
|
||||||
|
|
||||||
|
const { lookupRemoteUser } = await import('./federationLookup.js');
|
||||||
|
const result = await lookupRemoteUser(PEER_ORIGIN, 'bob');
|
||||||
|
|
||||||
|
expect(result).toEqual({ ok: false, reason: 'unreachable' });
|
||||||
|
});
|
||||||
|
|
||||||
|
it('8. returns ok:false reason:unreachable on malformed 200 body — must NOT throw', async () => {
|
||||||
vi.stubGlobal('fetch', vi.fn().mockResolvedValue({
|
vi.stubGlobal('fetch', vi.fn().mockResolvedValue({
|
||||||
ok: true,
|
ok: true,
|
||||||
status: 200,
|
status: 200,
|
||||||
@@ -211,8 +237,7 @@ describe('lookupRemoteUser', () => {
|
|||||||
}));
|
}));
|
||||||
|
|
||||||
const { lookupRemoteUser } = await import('./federationLookup.js');
|
const { lookupRemoteUser } = await import('./federationLookup.js');
|
||||||
await expect(lookupRemoteUser(PEER_ORIGIN, 'bob')).rejects.toThrow(
|
const result = await lookupRemoteUser(PEER_ORIGIN, 'bob');
|
||||||
`lookupRemoteUser: peer ${PEER_ORIGIN} returned malformed body`,
|
expect(result).toEqual({ ok: false, reason: 'unreachable' });
|
||||||
);
|
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -60,12 +60,22 @@ export async function lookupRemoteUser(peerOrigin: string, username: string): Pr
|
|||||||
}
|
}
|
||||||
|
|
||||||
if (!response.ok) {
|
if (!response.ok) {
|
||||||
throw new Error(`lookupRemoteUser: peer ${peerOrigin} returned HTTP ${response.status}`);
|
// Any non-2xx that isn't 404 (not_found) or 429 (rate_limited) — e.g. 403
|
||||||
|
// (peer rejects our HMAC: revoked, not-yet-active, or a post-reset secret
|
||||||
|
// desync) or 5xx (peer error) — is treated as `unreachable`, NOT thrown.
|
||||||
|
// A peer's auth/transport failure must never surface as an unhandled 500 on
|
||||||
|
// a user action (e.g. a federated friend-add); callers already map
|
||||||
|
// `unreachable` to a graceful 503. Logged for operators.
|
||||||
|
console.warn(`[federation] lookupRemoteUser: peer ${peerOrigin} returned HTTP ${response.status} — treating as unreachable`);
|
||||||
|
return { ok: false, reason: 'unreachable' };
|
||||||
}
|
}
|
||||||
|
|
||||||
const json = (await response.json()) as FederationUserLookupResponse;
|
const json = (await response.json().catch(() => null)) as FederationUserLookupResponse | null;
|
||||||
if (!json || json.found !== true || !json.user || typeof json.user.homeUserId !== 'string') {
|
if (!json || json.found !== true || !json.user || typeof json.user.homeUserId !== 'string') {
|
||||||
throw new Error(`lookupRemoteUser: peer ${peerOrigin} returned malformed body`);
|
// A malformed / non-JSON 200 body is peer misbehavior — surface as
|
||||||
|
// unreachable rather than throwing (same reasoning as above).
|
||||||
|
console.warn(`[federation] lookupRemoteUser: peer ${peerOrigin} returned malformed body — treating as unreachable`);
|
||||||
|
return { ok: false, reason: 'unreachable' };
|
||||||
}
|
}
|
||||||
|
|
||||||
return {
|
return {
|
||||||
|
|||||||
Reference in New Issue
Block a user