Two correctness/defense fixes plus regression tests in the existing
in-memory drizzle test file.
1. Reverse-direction idempotency. The sender-side path in social.ts
checks BOTH directions of friend_requests and returns 409
incoming_request_exists when an opposite-direction row exists. The
receiver only matched from->to, so cross-fire (alice@A and bob@B both
click "add friend" near-simultaneously) produced two opposite
pending rows on each instance. The receiver now silent-accepts when
either direction matches a pending row, mirroring the sender's
both-direction check.
2. Self-target guard (defense-in-depth). Reject events whose
from-identity equals to-identity (after normalizeOriginForCompare)
with a new receiver-acknowledged 4xx code self_target_invalid.
Sender's local cannot_friend_self should catch this, but the
receiver does not trust upstream validation. Added to
TERMINAL_REJECTION_REASONS so the standard rollback fires
(mapped client-side to peer_rejected). Logged at console.warn.
Spec updates: social.md inbound contract now documents both-direction
idempotency and the self-target guard; federation.md and the
s2s-friend-add design spec list the new terminal rejection reason.
Live verification caught a stale-UI bug: when admin denial / approval
fanout / janitor expiry cascaded subscriber rows away, only the
peering_notification_received WS event fired (which only refetches
the notification list). The user's pending-subscriptions section
stayed stale until manual refresh.
Each terminal-state path now also fires peering_subscription_changed
to affected users so their pending list refreshes alongside the new
notification. Fixed in:
- onPeerActivated.fanoutOutboundSubscribers (approval path)
- handleOutboundDeny (admin denial)
- cleanupExpiredApprovalRequests (janitor expiry; also adds the
notification-received broadcast that was previously deferred to
next-page-load only)
db.select() returned every column including userId; spec §4.9
defined the response row WITHOUT userId. The leak is harmless today
(user queries their own rows) but expands the public API surface
beyond the spec, and would become part of the contract once Task 10
generates client types. Switching to explicit column projection.
- GET /api/federation/peering-notifications (?unread=1 filter)
- POST /api/federation/peering-notifications/:id/read (per-row mark-read)
- POST /api/federation/peering-notifications/read-all (bulk mark-read,
preserves already-read readAt; returns affected count)
- janitor: outbound expired rows fan out kind='expired' notifications
to each subscriber before cascade-deleting parent (replaces Task 1's
scaffolding 'continue' guard); inbound expired rows are deleted outright
(both sides expire independently — no cross-instance network call)
- janitor: 30-day cleanup pass for read notifications
(cleanupReadPeeringNotifications); unread rows persist indefinitely
- cleanupExpiredApprovalRequests is now sync (no async network IO)
Notes:
- No WS broadcast on janitor expiry — offline users see notifications on
next GET, matching the persistence guarantee of the notifications table.
- The previous /peer/denied network call on inbound expiry is removed;
symmetric per-side expiry now handles termination on both sides.
Tests: 12 new (3 + 4 endpoint cases on 3 endpoints; 2 outbound/inbound
janitor expiry cases + 1 not-yet-expired guard + 1 retention sweep);
247 server tests passing total.
User-facing endpoints for the pending-peering-approvals UI surface.
GET lists the user's own pending subscriber rows joined to parent
peering requests. DELETE cancels a single subscription; if it was
the last subscriber, cascade-deletes the parent (admin queue updates
via federation_peers_changed). Authorization: subscriber.userId must
match request.userId.
- social.ts friend-add: user_action, with 409 peer_pending_local_admin
when gate fires
- /peer/ensure: user_action, surfaces peeringStatus: 'admin_required'
- sendCallRelay (typing warm-up + call relay): system intent
- federationWorker resolvePendingPeers: system intent (defensive — gate
is unreachable from here since pending rows already exist)
- CallRelayFailureReason: peer_admin_required added (mapped to
peer_transient_failure on the user-facing event surface, since system
intent should never legitimately surface admin_required)
- Test files: thread intent arg through racePeering and ensurePeered
calls (positional shift from racePeering signature change)
- outboundGate.test.ts: tighten noUncheckedIndexedAccess access via
non-null assertions after toHaveLength()
- docs/systems/social.md: peer_pending_local_admin error code documented
Three changes to keep the outbound /peer/accept call sites consistent
with the new approval-token mechanism:
1. /approve outbound body now forwards approvalToken from the queued
peer_approval_requests row when present. Receiver's awaiting_approval
branch verifies it and promotes mutual approval. Legacy null-token
rows omit the field; receiver falls through autoAccept gate. Spec §3.7.
2. /approve and /peer/initiate 202 paths now capture the approvalToken
returned by the remote and store it on the local federation_peers row.
Without this, the symmetric autoAccept=0 mutual-approval flow could
not verify on the eventual return /peer/accept. Spec §3.7.
3. /approve and /peer/initiate 200 paths now include approvalToken=null
in the activation UPDATE — single-use lifecycle hygiene per §3.2.
Test coverage: +6 tests (4 in approveOutbound, 2 in peerInitiateOutbound).
Total: 301 → 307. Web tsc clean.
Receiver-side defense — closes the trust-bypass class the cheap fix
(4533e36) cannot cover. The /peer/accept handler's awaiting_approval
branch now requires an approvalToken matching the one stored on the
local peer row before promoting to active. Without a match:
- autoAccept=0 falls through to queueApprovalRequest (no bypass; new
approval-request queued, existing awaiting_approval row untouched).
- autoAccept=1 falls back to permissive promotion (no regression vs
prior behavior, since autoAccept=1 would accept any inbound regardless).
Successful match also deletes any stale approval-request row for the
origin to prevent debris accumulation from prior bypass attempts.
Refactor: queueing path extracted to a top-level queueApprovalRequest()
helper so both the no-existing-peer case and the awaiting_approval
mismatch fallback share one implementation. Helper generates a fresh
single-use token on every call.
Adds 'accept_awaiting_approval_fallback' variant to PeerActivationReason
to distinguish the autoAccept=1 fallback path from the verified path
in onPeerActivated audit logs.
Spec §3.5, §3.6.
Test counts: 289 → 301 (+12).
Refactors the POST handler into handleLocalFriendRequest + handleFederatedFriendRequest helpers. The federated branch resolves the target domain, ensures peering, looks up the remote user, creates/hydrates a replicated stub, and writes a transactional (friend_requests + mutation_log + outbox) event with relayMessageId set. Also exports hydrateReplicatedUserProfile from federation.ts and adds the T11 happy-path test.
Third initiator path that calls remote /peer/accept. Mirrors performHandshake (auto-peer) and /peer/initiate (admin-initiate) — same try/catch parse, same null-or-non-empty-string guard. Caught in final review of #33; same root cause as Bug #1, bundled rather than fragmented to a new backlog item.
Mirrors the previous performHandshake fix for the admin-initiated path. /peer/initiate now parses the remote's instanceName from the /peer/accept response body and writes it alongside status='active'.
Bidirectional handshake exchange. Today the responder learns the initiator's instance name from request body but the initiator never learns the responder's. Adding {instanceName} to the response body lets the initiator persist it on its side (next commit). Field is optional so older peers omitting it cause no ill effect.
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).
processDmCallStartEvent Path A now skips offline local members (matching
Path B's pre-existing per-member check) and pushes undeliverable when no
member could be rung, instead of creating a stranded FederatedCallEntry.
TDD — two new tests cover zero-online and mixed-online cases.
processDmCallStartEvent Path B no longer silently accepts when no local
participant is reachable. Pushes {messageId, reason: 'no_recipient'} to
the undeliverable ack bucket so the caller can surface fast-fail.
TDD — test asserts undeliverable push + no FederatedCallEntry.
Additive plumbing. No behavior change — every existing event-type path
continues to push to accepted/rejected only. Response serializes the new
bucket only when non-empty (byte-identical responses in the normal case).
Tasks 3-4 add actual undeliverable pushes for dm_call_start paths.
Admin-revoke endpoint (DELETE /api/federation/peers/:id): fires
onPeerDeactivated(id, 'admin_revoked') after the status write to
'revoked', evicting any in-flight federated calls for the now-revoked
peer.
Admin-reset endpoint (POST /api/federation/peers/:id/reset): hook
SKIPPED. The reset endpoint is guarded to only run when status is
already 'needs_attention' (active peers are rejected at the boundary
with a 400). Because the peer was already deactivated before reset is
called, onPeerDeactivated was already fired at the active→needs_attention
transition. The reset deletes the row entirely rather than writing a new
status; it does not represent a transition OUT OF active, so wiring it
here would be a semantic error — double-evicting an already-deactivated
peer.
Adds contextType='profile' query branch. Extends the DM-pass
mutation_type IN-clause to include dm_close, dm_reopen,
read_state_update, file_rejected (events with no associated
dm_messages row). Builds channelFederatedIdMap for O(1)
federatedId resolution. Adds serializer branches for all five
new event types (dm_close/dm_reopen, read_state_update,
file_rejected, profile_update) that emit FederationRelayEvent
objects compatible with the existing inbound processors.
Inbound processors (processDmCloseEvent, processDmReopenEvent,
processReadStateUpdateEvent, processProfileUpdateEvent,
processFileRejectedEvent) already exist; sync replay feeds
events through processRelayEvents without any new receive-side
code.
Every code location that sets federation_peers.status='active'
now invokes onPeerActivated(peerId, reason). HTTP handler sites
use fire-and-forget (.catch(log)) so the response isn't blocked
by sync-pull pagination. The worker-internal health-check site
awaits the handler since the tick is already async.
Sites: /peer/initiate, /peer/accept (4 branches), /approval-
requests/:id/approve, health check recovery, ensurePeered/
performHandshake.
/peer/initiate checked `response.ok` to decide whether to activate the
local peer. `response.ok` is true for the full 2xx range, so a remote
that returned 202 (queued for admin approval — autoAcceptPeering off
on their side) caused the local peer to flip to `active` while the
remote had us `awaiting_approval`. The split only self-healed when
the remote admin approved and pushed us an `awaiting_approval → active`
override via the peer_approval_requests inbound path.
The auto-peer flow in federationPeering.ts:performHandshake already
had the correct 202 branch: set local status to awaiting_approval,
broadcast federation_peers_changed, surface a pending outcome. Mirror
it here:
- Check response.status === 202 BEFORE the !response.ok branch so the
fall-through can't reach the activation code.
- Transition local peer to awaiting_approval (not active).
- Broadcast federation_peers_changed so other admin tabs refresh.
- Return 202 with the sanitized peer so the client observes the
queued state distinctly from both success and failure.
Also added the missing federation_peers_changed broadcast on the
activation (200) path for parity with every other peer-state-change
site in the codebase — it was a pre-existing drift that would leave
sibling admin tabs stale after an initiate. Pattern-aligned with
federationPeering.ts:160 and the rest of routes/federation.ts.
Docs: expanded Phase 1 bullets in docs/systems/federation.md to cover
the 200 / 202 / other non-2xx / network-error branches explicitly and
reference the mirrored auto-peer branch.
Verified: pnpm -r typecheck clean (shared + server), vitest 70/70
pass.
Closes#21 from S2S DM unification backlog.
The column was `integer DEFAULT 0` (nullable) since the initial schema.
Counters should not be nullable — the semantics are a count, not an
optional measurement. `consecutive_auth_failures` (added later) was
correctly declared NOT NULL; tightening `consecutive_failures` to match
removes the drift and eliminates the "|null" burden everywhere the value
is read.
SQLite does not support in-place ALTER … SET NOT NULL, so drizzle-kit
cannot auto-generate this. The manual migration uses the standard
SQLite recreate pattern (new table + INSERT SELECT + DROP + RENAME +
recreate index) under `PRAGMA defer_foreign_keys = ON` so the existing
federation_outbox → federation_peers FK survives the swap. The COPY
step coalesces any hypothetical NULL to 0 defensively; live probes on
both test instances (nova, orbit) showed zero NULL rows so no
actual backfill is required.
Verified by applying the full migration chain against a copy of the VM's
live DB: column ends as `notnull=1 dflt=0`, the peer row is preserved,
the unique index on origin is recreated, NULL inserts are rejected, and
`PRAGMA foreign_key_check` reports no violations.
Server `SanitizedPeer.consecutiveFailures` tightened to `number` to
match the new drizzle inference and the shared `FederationPeer` shape.
Follow-up #22 from S2S DM unification backlog.
Admin-only endpoint for recovering from needs_attention. Deletes the
local peer row; FK cascade removes queued outbox entries. Gated to
peers in needs_attention to prevent accidental resets of healthy
peerings (use /peers/:id for revoke on active peers).
Also extends the Task 8.5 test mock of '../db/index.js' to re-export
`schema`. federation.ts imports `schema` from the re-export alongside
`getDb`; the previous mock only exposed `getDb`, causing the route
handler to blow up with 500s before reaching any assertion. This is
a scaffolding fix — no test assertions were changed.
Unauthenticated /peer/accept must not overwrite hmac_secret for peers in
needs_attention, same as active. needs_attention means 'auth trust broke
and we don't know why' — letting an unauthenticated request flip it back
would reintroduce a path for silent HMAC rotation via the outbox-401 loop
the rest of #19 closes. Legitimate recovery is the admin 'Reset peering'
action (next task).
processMemberAddEvent, processMemberRemoveEvent, and processOwnershipTransferEvent inserted system messages unconditionally. Outbox retries and initial-sync replays (triggered whenever an admin re-approves a peering request, which recreates the peer row with lastSyncedAt=0) duplicated the system message on every delivery. Each new snowflake ID exceeded the user's last_read_message_id, flipping the channel back to unread after every deploy.
Processors now short-circuit on a matching (source_instance, source_message_id) row, and persist those fields when inserting. processMemberAddEvent emits the tagged system message in both bootstrap and incremental paths so bootstrap replays don't fall through and insert a second one; the bootstrap's dm_channel_created broadcast carries that message as lastMessage so sidebar previews and unread anchors agree across instances.
Added federation_peers_changed (no-payload signal) broadcast from every
peer state mutation, and federation_approval_request_received when a new
approval request is queued. Client subscribes via onFederationPeersChanged
callback registry. FederationPanel and PendingApprovals debounce-refetch
on any event. sendToAdmins helper broadcasts only to admin users.
When both instances have autoAcceptPeering off, the approval flow
ping-ponged indefinitely. Admin A approves → handshakes to B → B
queues (202) → A's peer becomes awaiting_approval. Admin B approves →
handshakes to A → but A's gate only matched 'pending', not
'awaiting_approval', so it re-queued instead of accepting.
Now the gate matches both 'pending' and 'awaiting_approval'. When the
second admin approves and handshakes back, the first instance recognizes
its admin already approved and accepts — completing the peering.
1. queueOutboxEvent no longer creates pending peer placeholders when
autoAcceptPeering is disabled — prevents bypassing the admin's
peering control
2. Approval endpoint checks for 202 before response.ok — when the
remote also has autoAcceptPeering off, sets peer to awaiting_approval
instead of incorrectly activating it
3. awaiting_approval status added to Federation panel UI — status label,
colors, filter options so these peers are visible and manageable
- C1: Include 'unreachable' peers in queueOutboxEvent query to prevent
UNIQUE constraint violation when creating placeholders
- I1: Add 'rejected' to StatusFilter in FederationPanel so admins can
see and manage rejected peers with delete/re-initiate actions
- I2: Map ensurePeered 'failed' to 'pending' in peer/ensure response
to match spec and client expectations
When a profile_update relay arrives with avatar/banner URLs, download
the files to local storage instead of storing remote absolute URLs.
Falls back to absolute URL on any download failure. Cleans up old
local files when replaced.
Four fixes addressing the full state management problem:
1. Passive ready handler: no longer auto-connects to LiveKit on
page refresh. Prevents identity conflicts when the same user
has multiple sessions fighting for one LiveKit identity slot.
The user must re-accept to join; state is shown but not acted on.
2. SoundController sync guard: incomingCallLoading/outgoingCallLoading
refs prevent multiple playSound calls during async audio load.
If call is cancelled while sound loads, stops it immediately on
completion. Eliminates the "5 ringtones at once" bug.
3. Host dm_call_accepted broadcasts now include federatedCallId so
all clients (including remote instances) can match the event.
4. Removed all diagnostic console.log statements.
Three fixes for multi-instance call state consistency:
1. Client dm_call_accepted handler only auto-connects to LiveKit if
the user was the caller (outgoingCall was set). Other instances of
the same user just clear ringing state without connecting.
2. Server processDmCallAcceptEvent remote path skips duplicate
broadcast when FederatedCallEntry is already active (prevents
state conflicts from host fan-out arriving after local accept).
3. Ready payload handler clears stuck incomingCall when restoring
an already-active call after page refresh.
When findOrCreateDmChannel creates or finds a local channel for a
federatedId that has an active FederatedCallEntry with null dmChannelId,
update the entry. Prevents stale null references for Path B calls.
Jannis Braun