fix(desktop): land live presence updates for not-yet-cached pubkeys#947
Conversation
The WebSocket presence handler dropped any kind:20001 update for a pubkey not already present in the cached lookup. Because get_presence omits offline/unknown pubkeys entirely, a user's offline->online transition always targeted an absent key and was discarded, leaving the UI stale until the 60s backstop refetch re-seeded the cache (minutes of lag). Scope the cache write with a predicate matching the pubkey against the query key (which encodes the requested set) and merge the pubkey in even when absent. Same fix applied to the self-presence mutation's onSuccess. Merge/scope logic extracted to pure helpers in lib/presence.ts with tests. Co-authored-by: Brain <21994759fc7a6fa6b965551d35cfd7897d262f2495467f2d78694ddcfa6a5c7e@sprout-oss.stage.blox.sqprod.co> Signed-off-by: Wes <wesbillman@users.noreply.github.com>
|
@codex please review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a9ab982c7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| presenceQueryWantsPubkey(query.queryKey, pubkey), | ||
| }, | ||
| (old) => mergePresenceUpdate(old, pubkey, status), |
There was a problem hiding this comment.
When any client publishes a live kind:20001 event with a p tag naming a victim, the relay stores presence under the authenticated/event pubkey but fans out the original event unchanged (crates/sprout-relay/src/handlers/event.rs 446-452). With this predicate/merge change, if that victim is requested but absent from the cached lookup (the offline/unknown case this patch now inserts), the attacker-controlled p tag passes the query-key check and mergePresenceUpdate marks the victim online/away/offline until the next refetch. The WS subscription should key live updates by event.pubkey, or only honor p tags on trusted relay-synthesized query results.
Useful? React with 👍 / 👎.
|
|
||
| import { mergePresenceUpdate, presenceQueryWantsPubkey } from "./presence.ts"; | ||
|
|
||
| const WILL = "8e39cba681211b3782d0e4483e9343719b9b7be66515252da5491f26421896b1"; |
A reviewer flagged that the new absent-pubkey merge widened a spoof surface. The relay fans out client-authored kind:20001 events unchanged, and the WS handler read the event's p tag as the subject. Legitimate live events (sendPresence) carry no p tag — the subject is the author — but a malicious client could self-sign an event naming a victim in a p tag and flip that victim's cached presence until the next refetch. Key live updates off event.pubkey only. The p-tag subject is trusted just on the relay-signed REST/seed path (get_presence), which is unchanged. Logic extracted to parseLivePresenceEvent with spoof-prevention tests. Co-authored-by: Brain <21994759fc7a6fa6b965551d35cfd7897d262f2495467f2d78694ddcfa6a5c7e@sprout-oss.stage.blox.sqprod.co> Signed-off-by: Wes <wesbillman@users.noreply.github.com>
…947) Signed-off-by: Wes <wesbillman@users.noreply.github.com> Co-authored-by: Brain <21994759fc7a6fa6b965551d35cfd7897d262f2495467f2d78694ddcfa6a5c7e@sprout-oss.stage.blox.sqprod.co>
2 participants
Problem
Will showed offline for minutes in the desktop app even though the relay had him online the whole time (verified via
sprout users presence). It then self-healed without any user action.Root cause is in the WebSocket presence handler (
desktop/src/features/presence/hooks.ts). The live-update merge dropped any presence event for a pubkey not already in the cached lookup:The backend
get_presence(profile.rs) returns only pubkeys with a presence record — offline/unknown users are omitted entirely, and the UI reads a missing pubkey as offline. So the offline→online transition always targets an absent key:!(pubkey in old)→ dropped.The one transition presence exists to show is the one this guard always missed.
Fix
Scope the cache write with a
predicatethat matches the pubkey against the query key (the key already encodes the requested set:["presence", ...sortedPubkeys]), then merge the pubkey in even when absent. Live online events now land instantly; the 60s poll stays as the backstop it was meant to be.presence/lib/presence.ts— pure helperspresenceQueryWantsPubkey(key, pubkey)andmergePresenceUpdate(old, pubkey, status).presence/hooks.ts— bothsetQueriesDatasites (WS subscription and self-presence mutation, which had the identical latent guard) use the predicate + helper.presence/lib/presence.test.mjs— 8 unit tests.Verification
tsc --noEmitclean, biome lint clean.A unit test is the right proof here: the bug is a deterministic data-merge decision. A live-relay test would ride on timing and the very 60s backstop that masks the bug.
Out of scope (noted, not addressed here)
sprout users presenceCLI mislabels the subject pubkey with the relay signing key (reads event author instead of theptag).SPROUT_RELAY_PRIVATE_KEYunset).