fix(sync): websocket events are per-user — chokepoint the fan-out

Owner-flagged after the dev-docs audit: BroadcastProgressUpdate,
BroadcastAnnotationUpdate, and BroadcastConflictNotification fanned out
through the untargeted Broadcast, delivering one user's book progress,
conflict notices, and — worst — annotation payloads (which can carry
note/highlight text) to EVERY connected client on the instance. A
family-lens violation of exactly the kind the RLS move eliminated for
database reads: user B's client received events for user A's books,
including books in libraries hidden from B.

Fix is the RLS philosophy applied to the fan-out layer: enforcement at
one choke point. The untargeted Broadcast method and its channel/loop
machinery are deleted — BroadcastToUser is now the only door, and the
compiler forces every future emitter to name its audience instead of
relying on review. The three helpers take the acting user's ID; the
four sync call sites (progress.go x2, annotations.go x2) already had
the acting user in scope. Scan events were already per-triggering-
identity and collections already per-user — unchanged. Logging is
untouched: the only log line in the diff is the deleted method's own,
so docker logs remains the complete administrator audit trail.

TestBroadcastIsPerUser pins the invariant (alice's two devices receive
her three events; bob's client receives nothing — any message there is
a cross-user leak). Both websocket docs updated in the same commit:
the wire doc's fan-out section now states per-user delivery and the
chokepoint, and the gated endpoint doc's intro matches. Full make test
green.
This commit is contained in:
John O'Keefe committed 2026-11-02 18:06:45 -05:00
1 parent e957d385ec
commit a431dfa10b
6 files changed
+122 -62

No files matched your search

+2 -1
View File
@@ -1,6 +1,7 @@
# WebSocket Protocol
Real-time sync events broadcast to connected clients.
Real-time sync events, delivered only to the acting user's connected
devices.
## Connect to WebSocket
+14 -14
View File
@@ -62,10 +62,9 @@ All ten types defined by `internal/sync/websocket.go`:
| `heartbeat` | *Defined, not currently emitted* | — |
| `sync_complete` | *Defined, not currently emitted* | — |
(`collection_updated` is the one per-user-targeted event — it's emitted
with the connecting user's id directly. It's also currently the only
message type defined as a raw string in the collections handler rather
than in the sync package's constants block.)
(`collection_updated` is currently the only message type defined as a raw
string in the collections handler rather than in the sync package's
constants block.)
## Keepalive
@@ -76,14 +75,15 @@ nothing to implement. A connection that stops answering is removed.
## Fan-out and backpressure — read this before building a client
- Sync and scan events fan out **instance-wide**: every connected client
receives progress, annotation, conflict, and scan events. The
exception is `collection_updated`, which is targeted to one user's
connections. (A per-user targeting helper exists and is used for
collections only — the sync/scan emitters do not use it.)
- **Every event is delivered only to the acting user's connections.**
Progress, annotation, and conflict events go to the user whose sync
activity produced them; scan events go to the identity that triggered
the job; collection events to the collection's owner. The
ConnectionManager has no untargeted broadcast — the compiler enforces
an audience on every emitter (2026-11-02, after cross-user delivery
was found and fixed).
- Every connection has a bounded send queue. **If a client falls behind,
messages for that connection are skipped, not queued** (and the global
broadcast ring drops messages when saturated). Clients must therefore
treat the socket as a *hint* — on reconnect (or any gap), refetch real
state over REST. The fresh `initial_state` on reconnect is designed to
be that gap-healer.
messages for that connection are skipped, not queued.** Clients must
therefore treat the socket as a *hint* — on reconnect (or any gap),
refetch real state over REST. The fresh `initial_state` on reconnect is
designed to be that gap-healer.