mirror of
https://github.com/J3vb/OwnCord.git
synced 2026-09-03 03:50:00 +03:00
fix(ws): filter channel metadata broadcasts by READ_MESSAGES (F9)
channel_create and channel_update were handed to BroadcastToAll and enqueued with channelID 0, so the full channel payload -- name, topic and category of a channel that channel_overrides hides from the recipient's role -- went to every connected client and was replayed unconditionally from the ring buffer. Both now resolve an audience through the same READ_MESSAGES helper the voice path uses and enqueue under the real channel id, which filters live delivery and both replay tiers by one mechanism. channel_delete stays unfiltered by design: the row is already gone, so a check there would strand the channel in the sidebar of users who saw it via a positive override. Verified by a panel of agents; a base-revert control fails on both the live leak and the replay leak, while the pre-existing broadcast tests pass unmodified. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
+33
-8
@@ -544,28 +544,36 @@ func (h *Hub) BroadcastToAll(msg []byte) {
|
||||
// The audience is resolved here, on the caller's goroutine, so the hub's
|
||||
// dispatch loop never blocks on permission lookups.
|
||||
func (h *Hub) broadcastVoiceEvent(ctx context.Context, channelID int64, msg []byte) {
|
||||
h.broadcastChannelScoped(ctx, channelID, msg, "voice event")
|
||||
}
|
||||
|
||||
// broadcastChannelScoped enqueues msg for exactly the connected clients whose
|
||||
// current role may READ channelID, tagged with that channel id so reconnect
|
||||
// replay filters it too (EventsSinceFiltered replays a channelID of 0
|
||||
// unconditionally). kind only labels the drop warning.
|
||||
func (h *Hub) broadcastChannelScoped(ctx context.Context, channelID int64, msg []byte, kind string) {
|
||||
bm := broadcastMsg{
|
||||
channelID: channelID,
|
||||
msg: msg,
|
||||
recipients: h.voiceEventAudience(ctx, channelID),
|
||||
recipients: h.channelReadAudience(ctx, channelID),
|
||||
}
|
||||
select {
|
||||
case h.broadcast <- bm:
|
||||
default:
|
||||
h.broadcastDrops.Add(1)
|
||||
slog.Warn("hub: broadcast channel full, dropping voice event",
|
||||
slog.Warn("hub: broadcast channel full, dropping "+kind,
|
||||
"channel_id", channelID, "msg_len", len(msg))
|
||||
}
|
||||
}
|
||||
|
||||
// voiceEventAudience returns the connected user IDs whose current role may READ
|
||||
// channelReadAudience returns the connected user IDs whose current role may READ
|
||||
// channelID. Always non-nil, so an empty result means "deliver to nobody"
|
||||
// rather than "no filter". Roles are resolved per client (an admin may have
|
||||
// reassigned one mid-session) and the channel verdict is memoised per role, so
|
||||
// the cost is one role lookup per connected client plus one override lookup per
|
||||
// distinct role. Fails closed: a client whose role cannot be resolved is left
|
||||
// out. Mirrors RefreshChannelVisibility, which resolves visibility the same way.
|
||||
func (h *Hub) voiceEventAudience(ctx context.Context, channelID int64) []int64 {
|
||||
func (h *Hub) channelReadAudience(ctx context.Context, channelID int64) []int64 {
|
||||
h.mu.RLock()
|
||||
userIDs := make([]int64, 0, len(h.clients))
|
||||
for uid := range h.clients {
|
||||
@@ -602,17 +610,34 @@ func (h *Hub) BroadcastServerRestart(reason string, delaySeconds int) {
|
||||
h.BroadcastToAll(buildServerRestartMsg(reason, delaySeconds))
|
||||
}
|
||||
|
||||
// BroadcastChannelCreate sends a channel_create message to all connected clients.
|
||||
// BroadcastChannelCreate sends a channel_create message to the connected
|
||||
// clients whose current role may READ ch. It used to go out via BroadcastToAll,
|
||||
// which handed every authenticated client the name, category and topic of a
|
||||
// channel that channel_overrides hides from their role — metadata the ready
|
||||
// payload (buildReady/VisibleChannelIDs) deliberately withholds.
|
||||
//
|
||||
// The admin HubBroadcaster interface carries no context, so — like
|
||||
// RefreshChannelVisibility — the audience is resolved against Background: the
|
||||
// fan-out must complete regardless of the triggering request.
|
||||
func (h *Hub) BroadcastChannelCreate(ch *db.Channel) {
|
||||
h.BroadcastToAll(buildChannelCreate(ch))
|
||||
h.broadcastChannelScoped(context.Background(), ch.ID, buildChannelCreate(ch), "channel_create")
|
||||
}
|
||||
|
||||
// BroadcastChannelUpdate sends a channel_update message to all connected clients.
|
||||
// BroadcastChannelUpdate sends a channel_update message to the connected
|
||||
// clients whose current role may READ ch. Same disclosure as
|
||||
// BroadcastChannelCreate; same filtered fan-out.
|
||||
func (h *Hub) BroadcastChannelUpdate(ch *db.Channel) {
|
||||
h.BroadcastToAll(buildChannelUpdate(ch))
|
||||
h.broadcastChannelScoped(context.Background(), ch.ID, buildChannelUpdate(ch), "channel_update")
|
||||
}
|
||||
|
||||
// BroadcastChannelDelete sends a channel_delete message to all connected clients.
|
||||
//
|
||||
// Deliberately unfiltered: the payload is the bare channel id, with none of the
|
||||
// metadata create/update carry, and by the time the admin handler calls this the
|
||||
// channel row — and with it the ON DELETE CASCADE'd channel_overrides — is
|
||||
// already gone, so a permission check here would answer from base role perms
|
||||
// and could drop the delete for exactly the users who saw the channel via a
|
||||
// positive override, stranding it in their sidebar.
|
||||
func (h *Hub) BroadcastChannelDelete(channelID int64) {
|
||||
h.BroadcastToAll(buildChannelDelete(channelID))
|
||||
}
|
||||
|
||||
@@ -0,0 +1,112 @@
|
||||
package ws_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/owncord/server/db"
|
||||
"github.com/owncord/server/permissions"
|
||||
"github.com/owncord/server/ws"
|
||||
)
|
||||
|
||||
// countChannelMetaFor counts channel_create / channel_update events for a
|
||||
// specific channel id in a batch of raw WS frames.
|
||||
func countChannelMetaFor(msgs [][]byte, channelID int64) int {
|
||||
n := 0
|
||||
for _, m := range msgs {
|
||||
var env struct {
|
||||
Type string `json:"type"`
|
||||
Payload struct {
|
||||
ID int64 `json:"id"`
|
||||
} `json:"payload"`
|
||||
}
|
||||
if json.Unmarshal(m, &env) != nil {
|
||||
continue
|
||||
}
|
||||
if env.Payload.ID != channelID {
|
||||
continue
|
||||
}
|
||||
if env.Type == "channel_create" || env.Type == "channel_update" {
|
||||
n++
|
||||
}
|
||||
}
|
||||
return n
|
||||
}
|
||||
|
||||
// TestChannelMetadata_NotDeliveredToRolesDeniedRead locks the visibility
|
||||
// invariant for channel metadata: channel_create / channel_update used to go
|
||||
// out via BroadcastToAll, so every authenticated client learned the name,
|
||||
// category and topic of a channel that channel_overrides hides from their role —
|
||||
// live, and again on reconnect, since an event stored under channelID 0 is
|
||||
// replayed unconditionally. A role that may READ the channel must still receive
|
||||
// both, live and on replay.
|
||||
func TestChannelMetadata_NotDeliveredToRolesDeniedRead(t *testing.T) {
|
||||
hub, database := newHandlerHub(t)
|
||||
|
||||
pubID := seedTestChannel(t, database, "chmeta-general")
|
||||
privID := seedTestChannel(t, database, "chmeta-leadership")
|
||||
|
||||
insider := seedMemberUser(t, database, "chmeta-insider") // role 4: base READ_MESSAGES
|
||||
outsider := seedModUser(t, database, "chmeta-outsider") // role 3: denied READ below
|
||||
if err := database.UpsertChannelOverride(
|
||||
context.Background(), privID, outsider.RoleID, 0, permissions.ReadMessages,
|
||||
); err != nil {
|
||||
t.Fatalf("UpsertChannelOverride: %v", err)
|
||||
}
|
||||
|
||||
insiderSend := make(chan []byte, 64)
|
||||
outsiderSend := make(chan []byte, 64)
|
||||
hub.Register(ws.NewTestClientWithUser(hub, insider, 0, insiderSend))
|
||||
hub.Register(ws.NewTestClientWithUser(hub, outsider, 0, outsiderSend))
|
||||
time.Sleep(30 * time.Millisecond)
|
||||
|
||||
pub := &db.Channel{ID: pubID, Name: "chmeta-general", Type: "text", Category: "Text"}
|
||||
priv := &db.Channel{
|
||||
ID: privID, Name: "chmeta-leadership", Type: "text",
|
||||
Category: "Staff", Topic: "acquisition talks",
|
||||
}
|
||||
hub.BroadcastChannelCreate(pub)
|
||||
hub.BroadcastChannelUpdate(pub)
|
||||
hub.BroadcastChannelCreate(priv)
|
||||
hub.BroadcastChannelUpdate(priv)
|
||||
time.Sleep(150 * time.Millisecond)
|
||||
|
||||
// ── live delivery ─────────────────────────────────────────────────────────
|
||||
insiderLive := drainChanTimeout(insiderSend, 200*time.Millisecond)
|
||||
outsiderLive := drainChanTimeout(outsiderSend, 200*time.Millisecond)
|
||||
|
||||
if got := countChannelMetaFor(insiderLive, privID); got != 2 {
|
||||
t.Errorf("insider received %d channel_create/channel_update for the private channel, want 2", got)
|
||||
}
|
||||
// Positive control: the outsider is connected and receiving, so a zero count
|
||||
// on the private channel is filtering and not a broken delivery path.
|
||||
if got := countChannelMetaFor(outsiderLive, pubID); got != 2 {
|
||||
t.Errorf("outsider received %d channel_create/channel_update for the readable channel, want 2", got)
|
||||
}
|
||||
if got := countChannelMetaFor(outsiderLive, privID); got != 0 {
|
||||
t.Errorf("a role denied READ received %d channel metadata events for the private channel, want 0", got)
|
||||
}
|
||||
|
||||
// ── reconnect replay ──────────────────────────────────────────────────────
|
||||
oldest := hub.ReplayBuffer().OldestSeq()
|
||||
if oldest == 0 {
|
||||
t.Fatal("replay buffer recorded no channel events (oldest seq is 0)")
|
||||
}
|
||||
replayFor := func(u *db.User) [][]byte {
|
||||
t.Helper()
|
||||
allowed, err := hub.ComputeAllowedChannelsForTest(database, u)
|
||||
if err != nil {
|
||||
t.Fatalf("ComputeAllowedChannelsForTest: %v", err)
|
||||
}
|
||||
return hub.ReplayBuffer().EventsSinceFiltered(oldest+1, allowed)
|
||||
}
|
||||
|
||||
if got := countChannelMetaFor(replayFor(insider), privID); got != 2 {
|
||||
t.Errorf("insider replay contained %d channel metadata events for the private channel, want 2", got)
|
||||
}
|
||||
if got := countChannelMetaFor(replayFor(outsider), privID); got != 0 {
|
||||
t.Errorf("replay leaked %d channel metadata events for the private channel to a role denied READ, want 0", got)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user