fix(ws): require DM participation to join a DM voice room (F11)

The voice gate authorized a client-supplied channel_id with role bits only, and DM channels carry no channel_overrides rows, so any member's base CONNECT_VOICE bit minted a LiveKit RoomJoin and CanSubscribe token for any DM. Both voice entry points now go through a gate that re-runs the old role predicate and additionally requires DM participation, delegating that rule to the existing permissions.Checker.RequireChannelAccess rather than adding a second implementation of it. Verified by a panel of agents; a negative control of the base tree plus only the new test file fails both non-participant tests with a LiveKit room token issued for a DM the user is not a participant of, while both participant tests pass on base and patched alike. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-03 03:50:00 +03:00 · 2026-07-30 15:18:08 +02:00
parent 35acc09121
commit d6cc57af56
4 changed files with 218 additions and 7 deletions
@@ -129,6 +129,59 @@ func hasPerm(ctx context.Context, database *db.DB, perms *permissions.Checker, u
 	return perms.HasChannelPerm(ctx, role.Permissions, role.ID, channelID, perm)
 }

+// hasChannelAccess is the gate to use when the channel id comes from the client:
+// it is hasPerm plus the channel-type branch that role bits cannot express.
+//
+// A DM channel carries no channel_overrides rows, so a default Member's base
+// bits satisfy hasPerm for ANY dm channel id — including a conversation the
+// caller is not part of. permissions.Checker.RequireChannelAccess is the shared
+// definition of channel access (service.PermissionService.RequireChannelAccess
+// mirrors it for the REST/service paths) and supplies the IsDMParticipant
+// branch, so the DM membership rule keeps exactly one implementation. Group DMs
+// need no special case: dm_participants holds one row per participant and
+// IsDMParticipant is a lookup on (user_id, channel_id).
+//
+// The role bit is still required on top, which RequireChannelAccess waives for
+// DMs. Voice has always demanded CONNECT_VOICE and sweepStaleVoiceStates keeps
+// re-checking it per role for every live participant, so keeping it here means
+// this check can only ever narrow access — never hand someone a grant the old
+// role-only check refused, and never let the sweeper evict a client the join
+// gate admitted.
+//
+// Blocking is deliberately not consulted here: it is the message paths' rule
+// (service.requireDMNotBlocked), it is two-party only, and a blocked user is
+// still a participant, so it is orthogonal to the non-participant hole this
+// closes.
+func hasChannelAccess(ctx context.Context, database *db.DB, perms *permissions.Checker, userID, channelID, perm int64) bool {
+	if database == nil || perms == nil {
+		return false
+	}
+	role, err := database.GetRoleForUser(ctx, userID)
+	if err != nil || role == nil {
+		return false
+	}
+	if !perms.HasChannelPerm(ctx, role.Permissions, role.ID, channelID, perm) {
+		return false
+	}
+	ch, err := database.GetChannel(ctx, channelID)
+	if err != nil {
+		// Fail closed: an unknown type would silently take the non-DM path.
+		slog.Error("ws: hasChannelAccess GetChannel failed, denying",
+			"user_id", userID, "channel_id", channelID, "err", err)
+		return false
+	}
+	channelType := ""
+	if ch != nil {
+		channelType = ch.Type
+	}
+	// A missing channel row leaves channelType empty, i.e. the role verdict
+	// above stands: there is no DM there to join, and callers keep reporting a
+	// deleted channel the way they always have. For every non-DM type this call
+	// just re-runs the role check above; the repeated lookup is the price of one
+	// shared definition of the rule, on a per-user rate-limited path.
+	return perms.RequireChannelAccess(ctx, userID, role.Permissions, role.ID, channelType, channelID, perm) == nil
+}
+
 // ── V2 handler type ─────────────────────────────────────────────────────────

 // HandlerV2 is the function signature for new-style (pure-ish) handlers.
@@ -218,11 +218,16 @@ func (h *Hub) hasChannelPerm(ctx context.Context, c *Client, channelID int64, pe
 	return h.permChecker.HasChannelPerm(ctx, role.Permissions, role.ID, channelID, perm)
 }

-// requireChannelPerm checks whether the client has the given permission on the
-// channel. If not, it sends a FORBIDDEN error to the client and returns false.
-// The permLabel should be the human-readable permission name (e.g. "SEND_MESSAGES").
-func (h *Hub) requireChannelPerm(ctx context.Context, c *Client, channelID int64, perm int64, permLabel string) bool {
-	if h.hasChannelPerm(ctx, c, channelID, perm) {
+// requireChannelAccess checks whether the client may act on the channel with the
+// given permission. If not, it sends a FORBIDDEN error to the client and returns
+// false. The permLabel should be the human-readable permission name (e.g.
+// "SEND_MESSAGES").
+//
+// Unlike hasChannelPerm it is channel-type aware (see hasChannelAccess), which
+// is what a channel id taken straight from a client frame requires: role bits
+// alone let any member through to a DM they are not a participant of.
+func (h *Hub) requireChannelAccess(ctx context.Context, c *Client, channelID int64, perm int64, permLabel string) bool {
+	if hasChannelAccess(ctx, h.db, h.permChecker, c.userID, channelID, perm) {
 		return true
 	}
 	slog.Warn("ws permission denied", "user_id", c.userID, "channel_id", channelID, "perm", permLabel)
@@ -0,0 +1,147 @@
+package ws_test
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/owncord/server/ws"
+)
+
+// F11: voice_join and voice_token_refresh authorized the client-supplied channel
+// id with a role-only permission check. DM channels carry no channel_overrides,
+// so a default Member's base CONNECT_VOICE bit satisfied that check for ANY dm
+// channel id and the server minted a LiveKit RoomJoin+CanSubscribe token for a
+// conversation the caller is not part of (and then fed them the other
+// participants' voice_e2ee_announce keys). Both entry points must consult DM
+// membership; both must still work for a genuine participant.
+
+// assertNoVoiceToken asserts that no LiveKit token reached the client and that a
+// FORBIDDEN error did.
+func assertNoVoiceToken(t *testing.T, msgs [][]byte) {
+	t.Helper()
+	for _, m := range msgs {
+		if extractType(t, m) == "voice_token" {
+			t.Fatal("a LiveKit room token was issued for a DM the user is not a participant of")
+		}
+	}
+	found := false
+	for _, m := range msgs {
+		if extractCode(t, m) == "FORBIDDEN" {
+			found = true
+			break
+		}
+	}
+	if !found {
+		t.Error("expected a FORBIDDEN error for the non-participant")
+	}
+}
+
+func hasVoiceToken(t *testing.T, msgs [][]byte) bool {
+	t.Helper()
+	for _, m := range msgs {
+		if extractType(t, m) == "voice_token" {
+			return true
+		}
+	}
+	return false
+}
+
+func TestVoiceJoin_DMNonParticipant_GetsNoTokenAndNoVoiceState(t *testing.T) {
+	hub, database := newVoiceHub(t)
+	alice := seedMemberUser(t, database, "dmvoice-alice")
+	bob := seedMemberUser(t, database, "dmvoice-bob")
+	mallory := seedMemberUser(t, database, "dmvoice-mallory")
+	dmID := seedDMChannel(t, database, alice.ID, bob.ID)
+
+	send := make(chan []byte, 32)
+	c := ws.NewTestClientWithUser(hub, mallory, 0, send)
+	hub.Register(c)
+	time.Sleep(20 * time.Millisecond)
+
+	hub.HandleMessageForTest(c, voiceJoinMsg(dmID))
+	time.Sleep(50 * time.Millisecond)
+
+	assertNoVoiceToken(t, drainChanTimeout(send, 200*time.Millisecond))
+
+	state, err := database.GetVoiceState(context.Background(), mallory.ID)
+	if err != nil {
+		t.Fatalf("GetVoiceState: %v", err)
+	}
+	if state != nil {
+		t.Fatalf("non-participant was persisted into the DM's voice channel (%d)", state.ChannelID)
+	}
+}
+
+func TestVoiceJoin_DMParticipant_StillJoins(t *testing.T) {
+	hub, database := newVoiceHub(t)
+	alice := seedMemberUser(t, database, "dmvoice-ok-alice")
+	bob := seedMemberUser(t, database, "dmvoice-ok-bob")
+	dmID := seedDMChannel(t, database, alice.ID, bob.ID)
+
+	send := make(chan []byte, 32)
+	c := ws.NewTestClientWithUser(hub, alice, 0, send)
+	hub.Register(c)
+	time.Sleep(20 * time.Millisecond)
+
+	hub.HandleMessageForTest(c, voiceJoinMsg(dmID))
+	time.Sleep(50 * time.Millisecond)
+
+	if !hasVoiceToken(t, drainChanTimeout(send, 200*time.Millisecond)) {
+		t.Error("a DM participant must still receive a voice token for their own DM")
+	}
+
+	state, err := database.GetVoiceState(context.Background(), alice.ID)
+	if err != nil {
+		t.Fatalf("GetVoiceState: %v", err)
+	}
+	if state == nil || state.ChannelID != dmID {
+		t.Fatalf("participant voice state = %+v, want channel %d", state, dmID)
+	}
+}
+
+func TestVoiceTokenRefresh_DMNonParticipant_Refused(t *testing.T) {
+	hub, database := newVoiceHub(t)
+	alice := seedMemberUser(t, database, "dmrefresh-alice")
+	bob := seedMemberUser(t, database, "dmrefresh-bob")
+	mallory := seedMemberUser(t, database, "dmrefresh-mallory")
+	dmID := seedDMChannel(t, database, alice.ID, bob.ID)
+
+	send := make(chan []byte, 32)
+	c := ws.NewTestClientWithUser(hub, mallory, 0, send)
+	hub.Register(c)
+	time.Sleep(20 * time.Millisecond)
+
+	// Second entry point: the refresh mints a token from the session's own voice
+	// channel id, so it must re-run the same membership check rather than trust
+	// that a join once passed.
+	ws.SetVoiceChIDForTest(c, dmID)
+
+	hub.HandleMessageForTest(c, voiceTokenRefreshMsg())
+	time.Sleep(50 * time.Millisecond)
+
+	assertNoVoiceToken(t, drainChanTimeout(send, 200*time.Millisecond))
+}
+
+func TestVoiceTokenRefresh_DMParticipant_StillRefreshes(t *testing.T) {
+	hub, database := newVoiceHub(t)
+	alice := seedMemberUser(t, database, "dmrefresh-ok-alice")
+	bob := seedMemberUser(t, database, "dmrefresh-ok-bob")
+	dmID := seedDMChannel(t, database, alice.ID, bob.ID)
+
+	send := make(chan []byte, 32)
+	c := ws.NewTestClientWithUser(hub, alice, 0, send)
+	hub.Register(c)
+	time.Sleep(20 * time.Millisecond)
+
+	hub.HandleMessageForTest(c, voiceJoinMsg(dmID))
+	time.Sleep(50 * time.Millisecond)
+	drainChanBuf(send)
+
+	hub.HandleMessageForTest(c, voiceTokenRefreshMsg())
+	time.Sleep(50 * time.Millisecond)
+
+	if !hasVoiceToken(t, drainChanTimeout(send, 200*time.Millisecond)) {
+		t.Error("a DM participant must still be able to refresh their voice token")
+	}
+}
@@ -58,7 +58,10 @@ func (h *Hub) handleVoiceJoin(ctx context.Context, c *Client, payload json.RawMe
 		return
 	}

-	if !h.requireChannelPerm(ctx, c, channelID, permissions.ConnectVoice, "CONNECT_VOICE") {
+	// channel_id is attacker-controlled, so the gate must be channel-TYPE aware:
+	// a role-only check passes for any DM channel id (DMs have no overrides), and
+	// the token minted below carries RoomJoin+CanSubscribe for that DM's room.
+	if !h.requireChannelAccess(ctx, c, channelID, permissions.ConnectVoice, "CONNECT_VOICE") {
 		return
 	}

@@ -277,7 +280,10 @@ func handleVoiceTokenRefreshV2(ctx context.Context, cmd Command, info ClientInfo
 	// alone would leave the live session in place, so the refusal also evicts:
 	// LeaveVoice runs handleVoiceLeave, which clears the client's voice state,
 	// deletes the voice_states row and removes the LiveKit participant.
-	if !hasPerm(ctx, d.DB, d.Permissions, userID, channelID, permissions.ConnectVoice) {
+	// Channel-type aware, like the voice_join gate: this mints the same
+	// RoomJoin+CanSubscribe credential, so a role-only check here would keep
+	// re-issuing one for a DM the user is not a participant of.
+	if !hasChannelAccess(ctx, d.DB, d.Permissions, userID, channelID, permissions.ConnectVoice) {
 		return Result{
 			Error:      ClientError{Code: ErrCodeForbidden, Message: "missing CONNECT_VOICE permission"},
 			LeaveVoice: true,