- New 'admin_required' EnsurePeeredResult variant.
- Required intent argument (no optional default) prevents future
user-initiated callers from silently getting system behavior.
- Gate runs only when no peer row exists; toggling autoAccept later
does not retroactively gate established peer rows.
- queueOutboundApproval helper upserts parent + subscriber rows and
broadcasts to admins + user.
- Schema enum-narrows direction ('inbound' | 'outbound') and
notifications.kind ('approved' | 'denied' | 'expired') at the column
level; drizzle generate confirmed no migration delta.
- Existing call sites now fail typecheck — fixed in Task 5.
drizzle's snapshot does not encode SQL-level CHECK constraints. Without
this comment, a future migration that recreates the table for unrelated
reasons would silently drop the (direction='inbound' → hmac_secret
NOT NULL) invariant.
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).
- 202 response: parse approvalToken from body and store on the local
federation_peers row alongside status='awaiting_approval'. Legacy
receivers that omit the field result in null stored token, handled
gracefully by the verification logic landing in subsequent commits.
- 200 response: clear approvalToken in the same UPDATE that sets
status='active' (single-use consumption per spec §3.2).
Test coverage: 4 tests in federationPeering.approvalToken.test.ts covering
JSON token, missing token (legacy), non-JSON body, and 200 clear after
prior token storage.
Nullable text columns on federation_peers and peer_approval_requests.
Existing rows degrade gracefully (NULL token) per spec §5; the verification
logic landing in subsequent commits routes legacy null-token state through
the existing autoAccept gate.
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.
The friend contextId must be byte-identical on both peers for a given
canonical pair regardless of argument order. Initial-sync backfill keys
on contextId; if the two sides computed different values for the same
friendship, missed events would never reconcile.
The local branch was reusing friendRequestPayload (built for
friend_request_received with user=sender) for the sender-side
friend_request_sent broadcast. Tab B on the sender would render
the sender's own avatar where the target's should appear.
Match the federated branch — sent broadcast carries target.
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.
Nullable indexed column tracking the entityId of the originating relay
event for federated friend requests. Used by the rollback hook in
federationRollback.ts to locate and delete rows when a federated
friend_request_create is permanently rejected (spec §5).
Per code-review suggestion: add a brief comment explaining why
module-level sqlite/testDb/app reassignment works (mock getter
closes over the current binding) and what would break it
(top-level it, .concurrent describe). Prevents a future foot-gun.
Registration canonicalizes usernames to lowercase (auth.ts:32),
but the friend-request endpoint compared with strict eq() against
raw user input, so 'Bob' returned 404 even when 'bob' existed.
Trim and lowercase before lookup, matching the rest of the auth
boundary. Empty-after-trim now returns 400 (was: 404).
Tests appended to social.test.ts as a second describe block
sharing the harness from Task 1.
Drop the file-path reference per code-review suggestion (paths in
comments rot); keep the load-bearing why — substring of the domain
matches every stub because stubs are stored as <homeUserId>@<domain>.
Tombstoned users, replicated federated stubs, and users with
discoverable=0 were all surfacing in Add Friend search results.
Add the three WHERE filters that /api/social/discover already
applies. Federated users continue to be surfaced via the
client-side cross-instance fan-out in socialStore.searchUsers.
New tests: social.test.ts covers all five filter cases plus
existing self-exclusion and displayName-match behaviours.
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.
Fresh-create returned {id, ownerId, federatedId, createdAt, members, lastMessage}; the existing-DM path returned the same shape minus federatedId. Inconsistency was a footgun for any future feature reading federatedId from this response — fresh-create tests would pass while idempotent path would break. One-line addition to the result builder.
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'.
Initiator side of the bidirectional handshake exchange. /peer/accept now returns instanceName in the response body (prior commit); performHandshake parses it and persists alongside status='active'. Tolerates missing field (older peers) and non-JSON bodies.
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).
baselineExistingInstall, healInitialSchemaDrift, and healRenamedColumns
existed only because of pre-squash intermediate states: dev DBs drifted
against drizzle-kit's assumed 0000 baseline, or carried pre-rename
columns from the old manual migration system. Under a single squashed
baseline there are no intermediate states to drift against, and these
functions are unreachable.
initDatabase() is now: pragmas -> drizzle() -> migrate() -> ensureDefaults().
ensureDefaults stays (idempotent startup seeding for settings row, worker
ID, first-admin promotion).
Refs backlog #31 Phase 2.
Follow-up from Task 2 code review. Keeps the test mock aligned with
the widened return type even though vi.mock doesn't structurally
typecheck the factory.
sendFederatedCallStart now treats a 200-with-undeliverable-messageId as
a peer-failure instead of unconditional success. Feeds the existing
failures[] array and terminal-determination machinery from #16.
New sendFederatedCallStartForTest export mirrors the existing
handleDm*ForTest pattern. TDD — three tests cover single-peer terminal
no_recipient, group-DM mixed delivered+undeliverable non-terminal, and
the happy-path (empty undeliverable → no event).
Also hardens sendCallRelay's response parse: validates undeliverable
is an Array and entries are well-shaped, logs protocol drift at warn/debug
rather than silently falling back to old-peer semantics.
CallRelayResult success arm gains undeliverable: string[]. sendCallRelay
parses FederationRelayResponse.undeliverable (when present) and returns
the messageIds so sendFederatedCallStart can reclassify per-peer results.
Old peers that omit the field → empty array → today's behavior.
TDD — three tests cover old-peer, new-peer-with-undeliverable, and 5xx paths.
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.
The admin reset endpoint (DELETE-pattern gated on peer.status !== 'needs_attention')
doesn't transition status — it deletes the row of an already-deactivated peer.
onPeerDeactivated already fired at the earlier needs_attention transition, so
the reset site correctly has no hook. The enum value was defensive-unused; per
project principles (no backwards-compat shims, no placeholders) drop it.
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.
Code-review catch: the Path-2 accept-rollback previously emitted
dm_call_undeliverable { terminal: true } via sendToFederatedCallUsers,
which broadcasts to every ringedUserIds entry. In a group DM this
would prematurely tear down non-accepting ringees whose own accept /
reject / timeout paths should govern their state. Switch to
sendToUser(acceptorId) so only the acting user gets the terminal
signal. Reorder the clearFederatedCall to happen before the emit so a
concurrent end-handler sees a cleared entry (clearFederatedCall is
idempotent). Spec updated, test extended to assert the scoping with a
two-ringee group-DM fixture.
Jannis Braun