diff --git a/Server/permissions/checker.go b/Server/permissions/checker.go index 75195bbe..f6e17428 100644 --- a/Server/permissions/checker.go +++ b/Server/permissions/checker.go @@ -67,22 +67,38 @@ func NewChecker(db DB) *Checker { // userID may be 0 for a check that is genuinely role-only (no member in hand); // the per-user layer is then skipped rather than queried for a nonexistent id. func (ck *Checker) HasChannelPerm(ctx context.Context, rolePerms int64, roleID, userID, channelID, perm int64) bool { - if HasAdmin(rolePerms) { - return true - } - allow, deny, err := ck.db.GetChannelPermissions(ctx, channelID, roleID) + s, err := ck.Subject(ctx, rolePerms, roleID, userID, channelID) if err != nil { return false } - o := ChannelOverride{Allow: allow, Deny: deny} + return s.Has(perm) +} + +// Subject resolves both override layers for the member in channelID, live +// from the database, and returns them with the role bits as a Subject for the +// value-taking predicates (CanViewChannel and friends). The channel's flags +// and any DM state are the caller's to fill in. Administrator skips the fetch +// — overrides never change its verdict. A lookup failure is returned, not +// collapsed, so error-aware callers (the voice sweep) can tell a transient +// read failure from a denial; HasChannelPerm collapses it to false. +func (ck *Checker) Subject(ctx context.Context, rolePerms int64, roleID, userID, channelID int64) (Subject, error) { + s := Subject{RolePerms: rolePerms} + if HasAdmin(rolePerms) { + return s, nil + } + allow, deny, err := ck.db.GetChannelPermissions(ctx, channelID, roleID) + if err != nil { + return Subject{}, err + } + s.Override = ChannelOverride{Allow: allow, Deny: deny} if userID != 0 { uAllow, uDeny, uErr := ck.db.GetUserChannelPermissions(ctx, channelID, userID) if uErr != nil { - return false + return Subject{}, uErr } - o.UserAllow, o.UserDeny = uAllow, uDeny + s.Override.UserAllow, s.Override.UserDeny = uAllow, uDeny } - return EffectiveChannelPerms(rolePerms, o)&perm == perm + return s, nil } // HasChannelPermBatch reports whether the member has the given permission on @@ -91,11 +107,7 @@ func (ck *Checker) HasChannelPerm(ctx context.Context, rolePerms int64, roleID, // channels in bulk. The zero-value ChannelOverride (no entry in map) is correct // -- it means no override exists at either layer. func (ck *Checker) HasChannelPermBatch(rolePerms int64, overrides map[int64]ChannelOverride, channelID, perm int64) bool { - if HasAdmin(rolePerms) { - return true - } - o := overrides[channelID] // zero value when no override exists - return EffectiveChannelPerms(rolePerms, o)&perm == perm + return Subject{RolePerms: rolePerms, Override: overrides[channelID]}.Has(perm) } // VisibleChannelIDs returns the set of non-DM channel IDs the member @@ -113,13 +125,10 @@ func (ck *Checker) VisibleChannelIDs(rolePerms int64, channels []ChannelRef, ove if ch.Type == "dm" { continue } - // Archived channels are hidden from every client surface (admins - // included) — they stay manageable from the admin panel, which lists - // channels without this predicate. - if ch.Archived { - continue - } - if ck.HasChannelPermBatch(rolePerms, overrides, ch.ID, ReadMessages) { + // CanViewChannel hides archived channels from every client surface + // (admins included) — they stay manageable from the admin panel, + // which lists channels without this predicate. + if CanViewChannel(Subject{RolePerms: rolePerms, Override: overrides[ch.ID], Channel: ch}) == nil { visible[ch.ID] = true } } diff --git a/Server/permissions/predicates.go b/Server/permissions/predicates.go new file mode 100644 index 00000000..41f28dd1 --- /dev/null +++ b/Server/permissions/predicates.go @@ -0,0 +1,166 @@ +package permissions + +import ( + "errors" + "fmt" +) + +// One predicate per security property (B2-5). Each is a pure function over a +// Subject the caller has already resolved — no context, no store — so a call +// site cannot drift by re-deriving half the rule: ready's can_send, the +// composer refresh, typing, the send path and the plugin gate all ask +// CanSendMessage the same question. A nil result is "allowed"; a non-nil one +// is a sentinel from this package (or wraps ErrPermissionDenied with the +// missing bit's name) so each call site keeps mapping denials to its own +// status codes and messages. + +// ErrArchived is returned for a write to, or a client surface for, an archived +// channel. History stays readable, so read paths do not consult it. +var ErrArchived = errors.New("channel is archived") + +// ErrBlocked is returned when either DM party has blocked the other. +var ErrBlocked = errors.New("user is blocked") + +// ErrNotVoiceChannel is returned by CanJoinVoice for a channel type that has +// no voice room (text, announcement). +var ErrNotVoiceChannel = errors.New("not a voice channel") + +// Subject is everything a channel predicate consults: the actor's role bits, +// both override layers for the one channel in question, the channel's flags, +// and — for a DM — the membership and block state the caller looked up. The +// zero value is a subject with no role and no membership, which every +// predicate refuses. +type Subject struct { + RolePerms int64 + Override ChannelOverride + Channel ChannelRef // Type and Archived; ID is not consulted + // DMParticipant and DMBlocked are consulted only when Channel.Type is + // "dm". Group DMs pass DMBlocked=false — blocks are enforced at group + // creation, not per message (service.requireDMNotBlocked). + DMParticipant bool + DMBlocked bool +} + +// Has reports whether the subject's effective permission in the channel holds +// every bit of perm. Administrator bypasses both override layers; a zero perm +// is never held. This is the single value-taking bit predicate — Checker and +// PermissionService resolve a Subject and ask it. +func (s Subject) Has(perm int64) bool { + return HasAdmin(s.RolePerms) || HasPerm(EffectiveChannelPerms(s.RolePerms, s.Override), perm) +} + +// missing wraps ErrPermissionDenied with the bit a caller would name in its +// own FORBIDDEN message. For a multi-bit perm the named bit is the last one +// the caller listed as the "reason" (SEND for READ|SEND). +func missing(named int64) error { + return fmt.Errorf("%w: missing %s", ErrPermissionDenied, Name(named)) +} + +// dmMember refuses a non-participant. Every DM rule starts here and nothing +// about the role bypasses it — an administrator is not in someone else's DM. +func dmMember(s Subject) error { + if !s.DMParticipant { + return ErrNotDMParticipant + } + return nil +} + +// CanViewChannel is visibility: what the sidebar lists, the ready payload +// carries, reconnect replay filters to, and channel-scoped fan-out reaches. +// Permission is checked before the archive flag so an unauthorized caller +// learns nothing about the channel from the error; archived channels are then +// hidden from everyone, admins included (they stay manageable from the admin +// panel, which does not use this predicate). +func CanViewChannel(s Subject) error { + if s.Channel.Type == "dm" { + return dmMember(s) + } + if !s.Has(ReadMessages) { + return missing(ReadMessages) + } + if s.Channel.Archived { + return ErrArchived + } + return nil +} + +// CanAdmitSession decides whether a live socket may attach to a channel's +// stream (channel_focus, the post-Subscribe revalidation). It is +// CanViewChannel by definition: a session sees exactly what the user sees. +func CanAdmitSession(s Subject) error { return CanViewChannel(s) } + +// CanSendMessage is the post policy: READ and SEND in the channel, MANAGE on +// top for announcement channels, never into an archive; for a DM, membership +// and no block in either direction. SendMessage, EditMessage, CanPost, the +// ready payload's can_send, the composer refresh and typing all delegate here. +func CanSendMessage(s Subject) error { + if s.Channel.Type == "dm" { + if err := dmMember(s); err != nil { + return err + } + if s.DMBlocked { + return ErrBlocked + } + return nil + } + if !s.Has(ReadMessages | SendMessages) { + return missing(SendMessages) + } + if s.Channel.Type == "announcement" && !s.Has(ManageMessages) { + return missing(ManageMessages) + } + if s.Channel.Archived { + return ErrArchived + } + return nil +} + +// CanType is CanSendMessage (S-01): a typing indicator announces a post, so a +// member who cannot post cannot announce one. +func CanType(s Subject) error { return CanSendMessage(s) } + +// CanJoinVoice gates the LiveKit credential: CONNECT_VOICE in the channel +// (required for DM calls too — the role bit was always demanded on top of +// membership, so this can only ever narrow), a channel that has a room, for +// a DM membership plus no block, and no archive for either kind — the admin +// PATCH accepts `archived` for any channel type, and an evicted participant +// must not rejoin the archived room. Applies at join, at token refresh, to +// the target of a moderator move, and in the stale-voice sweep. +func CanJoinVoice(s Subject) error { + if !s.Has(ConnectVoice) { + return missing(ConnectVoice) + } + switch s.Channel.Type { + case "dm": + if err := dmMember(s); err != nil { + return err + } + if s.DMBlocked { + return ErrBlocked + } + case "voice": + default: + return ErrNotVoiceChannel + } + if s.Channel.Archived { + return ErrArchived + } + return nil +} + +// CanModerateVoice is the actor's authority in the TARGET's channel: +// effective MUTE_MEMBERS there (so a per-channel deny holds — SEC-02), and +// READ_MESSAGES so a moderator can only act in a room they can see; a DM call +// additionally requires the actor to be a participant. Rank is not a +// permission and stays with the caller (the strict-outranks check). +func CanModerateVoice(s Subject) error { + if s.Channel.Type == "dm" { + if err := dmMember(s); err != nil { + return err + } + } + if !s.Has(ReadMessages | MuteMembers) { + return missing(MuteMembers) + } + return nil +} diff --git a/Server/permissions/predicates_test.go b/Server/permissions/predicates_test.go new file mode 100644 index 00000000..bfe5799b --- /dev/null +++ b/Server/permissions/predicates_test.go @@ -0,0 +1,149 @@ +package permissions + +import ( + "errors" + "testing" +) + +// Each predicate is one security property. The tables below are the property +// stated as cases; every call site that used to hand-roll a copy of the rule +// now delegates here and carries a parity test against these verdicts. + +const ( + memberBits = ReadMessages | SendMessages | ConnectVoice + modBits = memberBits | ManageMessages | MuteMembers +) + +func text(archived bool) ChannelRef { return ChannelRef{Type: "text", Archived: archived} } +func announcement() ChannelRef { return ChannelRef{Type: "announcement"} } +func voice(archived bool) ChannelRef { return ChannelRef{Type: "voice", Archived: archived} } +func dm() ChannelRef { return ChannelRef{Type: "dm"} } +func deny(bits int64) ChannelOverride { return ChannelOverride{Deny: bits} } +func userDeny(bits int64) ChannelOverride { return ChannelOverride{UserDeny: bits} } +func allow(bits int64) ChannelOverride { return ChannelOverride{Allow: bits} } +func userAllow(bits int64) ChannelOverride { return ChannelOverride{UserAllow: bits} } + +type predicateCase struct { + name string + s Subject + want error // nil = allowed; otherwise errors.Is(got, want) +} + +func runPredicate(t *testing.T, name string, fn func(Subject) error, cases []predicateCase) { + t.Helper() + for _, c := range cases { + got := fn(c.s) + if c.want == nil && got != nil { + t.Errorf("%s/%s: want allowed, got %v", name, c.name, got) + } + if c.want != nil && !errors.Is(got, c.want) { + t.Errorf("%s/%s: want %v, got %v", name, c.name, c.want, got) + } + } +} + +func TestCanViewChannel(t *testing.T) { + runPredicate(t, "CanViewChannel", CanViewChannel, []predicateCase{ + {"member reads text", Subject{RolePerms: memberBits, Channel: text(false)}, nil}, + {"no role fails closed", Subject{Channel: text(false)}, ErrPermissionDenied}, + {"role deny READ hides", Subject{RolePerms: memberBits, Override: deny(ReadMessages), Channel: text(false)}, ErrPermissionDenied}, + {"user deny READ hides", Subject{RolePerms: memberBits, Override: userDeny(ReadMessages), Channel: text(false)}, ErrPermissionDenied}, + {"user allow beats role deny", Subject{RolePerms: memberBits, Override: ChannelOverride{Deny: ReadMessages, UserAllow: ReadMessages}, Channel: text(false)}, nil}, + {"admin bypasses deny", Subject{RolePerms: Administrator, Override: deny(ReadMessages), Channel: text(false)}, nil}, + {"archived hidden from members", Subject{RolePerms: memberBits, Channel: text(true)}, ErrArchived}, + {"archived hidden from admins", Subject{RolePerms: Administrator, Channel: text(true)}, ErrArchived}, + {"unauthorized never learns archived", Subject{Channel: text(true)}, ErrPermissionDenied}, + {"dm participant sees", Subject{Channel: dm(), DMParticipant: true}, nil}, + {"dm non-participant blind, even admin", Subject{RolePerms: Administrator, Channel: dm()}, ErrNotDMParticipant}, + {"dm ignores block", Subject{Channel: dm(), DMParticipant: true, DMBlocked: true}, nil}, + }) +} + +func TestCanSendMessage(t *testing.T) { + cases := []predicateCase{ + {"member posts in text", Subject{RolePerms: memberBits, Channel: text(false)}, nil}, + {"reader without SEND refused", Subject{RolePerms: ReadMessages, Channel: text(false)}, ErrPermissionDenied}, + {"SEND without READ refused", Subject{RolePerms: SendMessages, Channel: text(false)}, ErrPermissionDenied}, + {"role deny SEND refused", Subject{RolePerms: memberBits, Override: deny(SendMessages), Channel: text(false)}, ErrPermissionDenied}, + {"user deny SEND refused", Subject{RolePerms: memberBits, Override: userDeny(SendMessages), Channel: text(false)}, ErrPermissionDenied}, + {"announcement needs MANAGE", Subject{RolePerms: memberBits, Channel: announcement()}, ErrPermissionDenied}, + {"moderator posts in announcement", Subject{RolePerms: modBits, Channel: announcement()}, nil}, + {"override allow MANAGE enables announcement", Subject{RolePerms: memberBits, Override: allow(ManageMessages), Channel: announcement()}, nil}, + {"user allow MANAGE enables announcement", Subject{RolePerms: memberBits, Override: userAllow(ManageMessages), Channel: announcement()}, nil}, + {"admin bypasses on announcement", Subject{RolePerms: Administrator, Channel: announcement()}, nil}, + {"archived refuses members", Subject{RolePerms: memberBits, Channel: text(true)}, ErrArchived}, + {"archived refuses admins", Subject{RolePerms: Administrator, Channel: text(true)}, ErrArchived}, + {"unauthorized never learns archived", Subject{RolePerms: ReadMessages, Channel: text(true)}, ErrPermissionDenied}, + {"dm participant posts without role bits", Subject{Channel: dm(), DMParticipant: true}, nil}, + {"dm non-participant refused", Subject{RolePerms: Administrator, Channel: dm()}, ErrNotDMParticipant}, + {"dm blocked refused", Subject{Channel: dm(), DMParticipant: true, DMBlocked: true}, ErrBlocked}, + } + runPredicate(t, "CanSendMessage", CanSendMessage, cases) + // CanType is CanSendMessage by definition (S-01): a typing indicator is + // the first half of a post, so it answers to the same rule. + runPredicate(t, "CanType", CanType, cases) +} + +func TestCanAdmitSession(t *testing.T) { + // Session admission (channel_focus / topic subscribe) is visibility. + cases := []predicateCase{ + {"member admitted", Subject{RolePerms: memberBits, Channel: text(false)}, nil}, + {"archived refused", Subject{RolePerms: memberBits, Channel: text(true)}, ErrArchived}, + {"dm participant admitted", Subject{Channel: dm(), DMParticipant: true}, nil}, + {"dm non-participant refused", Subject{Channel: dm()}, ErrNotDMParticipant}, + } + runPredicate(t, "CanAdmitSession", CanAdmitSession, cases) + runPredicate(t, "CanViewChannel(parity)", CanViewChannel, cases) +} + +func TestCanJoinVoice(t *testing.T) { + runPredicate(t, "CanJoinVoice", CanJoinVoice, []predicateCase{ + {"member joins voice", Subject{RolePerms: memberBits, Channel: voice(false)}, nil}, + {"no CONNECT refused", Subject{RolePerms: ReadMessages, Channel: voice(false)}, ErrPermissionDenied}, + {"role deny CONNECT refused", Subject{RolePerms: memberBits, Override: deny(ConnectVoice), Channel: voice(false)}, ErrPermissionDenied}, + {"user deny CONNECT refused", Subject{RolePerms: memberBits, Override: userDeny(ConnectVoice), Channel: voice(false)}, ErrPermissionDenied}, + {"admin bypasses deny", Subject{RolePerms: Administrator, Override: deny(ConnectVoice), Channel: voice(false)}, nil}, + {"text channel is not voice", Subject{RolePerms: memberBits, Channel: text(false)}, ErrNotVoiceChannel}, + {"archived voice refused", Subject{RolePerms: memberBits, Channel: voice(true)}, ErrArchived}, + {"unauthorized never learns archived", Subject{RolePerms: ReadMessages, Channel: voice(true)}, ErrPermissionDenied}, + {"dm call needs CONNECT bit too", Subject{Channel: dm(), DMParticipant: true}, ErrPermissionDenied}, + {"dm participant with CONNECT joins", Subject{RolePerms: ConnectVoice, Channel: dm(), DMParticipant: true}, nil}, + {"dm non-participant refused", Subject{RolePerms: Administrator, Channel: dm()}, ErrNotDMParticipant}, + {"dm blocked refused", Subject{RolePerms: ConnectVoice, Channel: dm(), DMParticipant: true, DMBlocked: true}, ErrBlocked}, + // The admin PATCH accepts archived for any channel type, and the old + // voice_join gate refused every archived channel — an evicted + // participant must not rejoin an archived DM call (Codex P2, #1440). + {"archived dm call refused", Subject{RolePerms: ConnectVoice, Channel: ChannelRef{Type: "dm", Archived: true}, DMParticipant: true}, ErrArchived}, + {"archived dm non-participant learns nothing", Subject{RolePerms: ConnectVoice, Channel: ChannelRef{Type: "dm", Archived: true}}, ErrNotDMParticipant}, + }) +} + +func TestCanModerateVoice(t *testing.T) { + runPredicate(t, "CanModerateVoice", CanModerateVoice, []predicateCase{ + {"moderator with MUTE in channel", Subject{RolePerms: modBits, Channel: voice(false)}, nil}, + {"no MUTE refused", Subject{RolePerms: memberBits, Channel: voice(false)}, ErrPermissionDenied}, + {"role deny MUTE in this channel refused", Subject{RolePerms: modBits, Override: deny(MuteMembers), Channel: voice(false)}, ErrPermissionDenied}, + {"user deny MUTE in this channel refused", Subject{RolePerms: modBits, Override: userDeny(MuteMembers), Channel: voice(false)}, ErrPermissionDenied}, + {"channel hidden from moderator refused", Subject{RolePerms: modBits, Override: deny(ReadMessages), Channel: voice(false)}, ErrPermissionDenied}, + {"admin bypasses deny", Subject{RolePerms: Administrator, Override: deny(MuteMembers), Channel: voice(false)}, nil}, + {"dm call needs actor membership", Subject{RolePerms: modBits, Channel: dm()}, ErrNotDMParticipant}, + {"dm participant moderator allowed", Subject{RolePerms: modBits, Channel: dm(), DMParticipant: true}, nil}, + {"dm participant without MUTE refused", Subject{RolePerms: memberBits, Channel: dm(), DMParticipant: true}, ErrPermissionDenied}, + }) +} + +// TestSubjectHas pins the one generic predicate every other one is built on: +// Administrator bypasses overrides, everything else is the resolved two-layer +// mask, and a zero perm is never held (matching HasPerm). +func TestSubjectHas(t *testing.T) { + s := Subject{RolePerms: memberBits, Override: ChannelOverride{Deny: SendMessages, UserAllow: ManageMessages}} + if !s.Has(ReadMessages) || s.Has(SendMessages) || !s.Has(ManageMessages) || s.Has(ReadMessages|SendMessages) { + t.Fatal("Has must apply both override layers and be ALL-of") + } + if s.Has(0) { + t.Fatal("zero perm is never held") + } + if !(Subject{RolePerms: Administrator, Override: deny(AllPerms)}).Has(ManageServer) { + t.Fatal("Administrator bypasses overrides") + } +} diff --git a/Server/service/channel.go b/Server/service/channel.go index 32c91f6d..1f0d4775 100644 --- a/Server/service/channel.go +++ b/Server/service/channel.go @@ -118,19 +118,14 @@ func (s *ChannelService) HandleTyping(ctx context.Context, userID, channelID int return nil, nil //nolint:nilerr // typing indicators are best-effort; errors silently dropped } - if ch.Type == "dm" { - ok, dmErr := s.st.IsDMParticipant(ctx, userID, channelID) - if dmErr != nil || !ok { - return nil, nil //nolint:nilerr // typing indicators are best-effort; errors silently dropped - } - // A blocked user must not be able to keep poking the blocker with - // typing indicators. Same gate as the other DM sinks; silently dropped - // here because typing is best-effort. - if blkErr := requireDMNotBlocked(ctx, s.st, userID, channelID); blkErr != nil { - return nil, nil //nolint:nilerr // best-effort: a blocked or unreadable DM emits nothing - } - } else if !s.perms.HasChannelPerm(ctx, userID, channelID, permissions.ReadMessages) { - return nil, nil // silent drop + // A typing indicator announces a post, so it answers to the post policy + // (permissions.CanType is CanSendMessage — S-01): a read-only member, an + // announcement reader without MANAGE_MESSAGES, an archived channel, a + // blocked or non-participant DM user all emit nothing. Silent, because + // typing is best-effort. + sub, subErr := channelSubject(ctx, s.st, s.perms, userID, ch, true) + if subErr != nil || permissions.CanType(sub) != nil { + return nil, nil //nolint:nilerr // best-effort: a denial or a DM lookup failure emits nothing } // Per-user-per-channel rate limit. Built only now that the channel is @@ -247,24 +242,18 @@ func (s *ChannelService) HandleChannelFocus(ctx context.Context, userID, channel return nil, fmt.Errorf("%w: channel not found", ErrNotFound) } - switch { - case ch.Type == "dm": - ok, err := s.st.IsDMParticipant(ctx, userID, channelID) - if err != nil || !ok { - return nil, fmt.Errorf("%w: access denied", ErrForbidden) - } - case !s.perms.HasChannelPerm(ctx, userID, channelID, permissions.ReadMessages): + // Session admission is permissions.CanAdmitSession — visibility, the same + // predicate behind ListVisibleChannels, the ready payload and reconnect + // replay — so a socket that still holds an id it can no longer see (or + // an archived channel, OC-0070) cannot resubscribe to the live topic or + // advance its read state. channel_focus and mark_read share this one + // service call, so the gate closes both at once. + sub, subErr := channelSubject(ctx, s.st, s.perms, userID, ch, false) + if subErr != nil { return nil, fmt.Errorf("%w: access denied", ErrForbidden) - case ch.Archived: - // Archived channels are hidden from every other client surface - // (ListVisibleChannels, ready payload, reconnect replay, voice join — - // see permissions.Checker.VisibleChannelIDs and ws/voice_join.go). - // HasChannelPerm alone doesn't know about the archive flag, so without - // this a socket that still held the id could resubscribe to the live - // topic and advance its own read state on a channel reconnect replay - // then filters back out. channel_focus and mark_read share this one - // service call, so the guard closes both at once (OC-0070). - return nil, fmt.Errorf("%w: channel is archived", ErrForbidden) + } + if err := permissions.CanAdmitSession(sub); err != nil { + return nil, fmt.Errorf("%w: %v", ErrForbidden, err) } // Mark channel as read. latestID == 0 (no undeleted messages) still diff --git a/Server/service/message_perms.go b/Server/service/message_perms.go index 307fe677..92c3efa1 100644 --- a/Server/service/message_perms.go +++ b/Server/service/message_perms.go @@ -2,6 +2,7 @@ package service import ( "context" + "errors" "fmt" "github.com/J3vb/OwnCord/Server/db" @@ -69,36 +70,63 @@ func (s *MessageService) CanPost(ctx context.Context, userID, channelID int64) e return s.checkSendPermission(ctx, userID, ch) } -// checkSendPermission validates send permission for ch. Announcement channels -// are readable by anyone with READ_MESSAGES but only postable by users with -// MANAGE_MESSAGES (posting is restricted to moderators/admins); all other -// non-DM channels require SEND_MESSAGES. Also enforces requireChannelWritable, -// so every caller — SendMessage, EditMessage, CanPost — refuses an archived -// channel without re-implementing that check itself. +// checkSendPermission is permissions.CanSendMessage over the resolved subject: +// READ|SEND in the channel, MANAGE_MESSAGES on top for announcement channels, +// never into an archive; DM membership and no block. Every caller — +// SendMessage, EditMessage, CanPost, and typing via ChannelService — asks that +// one predicate, so none can drift (S-01, S-12). func (s *MessageService) checkSendPermission(ctx context.Context, userID int64, ch *db.Channel) error { - isDM := ch.Type == "dm" - if isDM { - ok, err := s.st.IsDMParticipant(ctx, userID, ch.ID) - if err != nil { - return fmt.Errorf("%w: failed to check DM participation: %v", ErrInternal, err) - } - if !ok { - return fmt.Errorf("%w: not a participant in this DM", ErrForbidden) - } - return requireDMNotBlocked(ctx, s.st, userID, ch.ID) - } - if err := requireChannelWritable(ch); err != nil { + sub, err := channelSubject(ctx, s.st, s.perms, userID, ch, true) + if err != nil { return err } - if !s.perms.HasChannelPerm(ctx, userID, ch.ID, permissions.ReadMessages|permissions.SendMessages) { - return fmt.Errorf("%w: missing SEND_MESSAGES permission", ErrForbidden) + return denial(permissions.CanSendMessage(sub)) +} + +// channelSubject resolves what the channel predicates need for userID in ch: +// role bits and both override layers from the permission cache (a lookup +// failure or a missing role yields no bits — fail closed, as HasChannelPerm +// always has), the channel's flags, and for a DM its membership and, when +// withBlock is set, the two-party block state. The only error is a DM lookup +// failure, wrapped as ErrInternal; callers keep their own posture toward it +// (SendMessage reports it, typing drops silently). +func channelSubject(ctx context.Context, st Store, perms *PermissionService, userID int64, ch *db.Channel, withBlock bool) (permissions.Subject, error) { + sub, err := perms.Subject(ctx, userID, ch.ID) + if err != nil { + sub = permissions.Subject{} } - // Announcement channels: posting is restricted to users who can manage - // messages, even though everyone with READ_MESSAGES can view them. - if ch.Type == "announcement" && !s.perms.HasChannelPerm(ctx, userID, ch.ID, permissions.ManageMessages) { - return fmt.Errorf("%w: announcement channels require MANAGE_MESSAGES to post", ErrForbidden) + sub.Channel = permissions.ChannelRef{ID: ch.ID, Type: ch.Type, Archived: ch.Archived} + if ch.Type != "dm" { + return sub, nil + } + ok, dmErr := st.IsDMParticipant(ctx, userID, ch.ID) + if dmErr != nil { + return sub, fmt.Errorf("%w: failed to check DM participation: %v", ErrInternal, dmErr) + } + sub.DMParticipant = ok + if ok && withBlock { + switch blkErr := requireDMNotBlocked(ctx, st, userID, ch.ID); { + case errors.Is(blkErr, ErrBlocked): + sub.DMBlocked = true + case blkErr != nil: + return sub, blkErr + } + } + return sub, nil +} + +// denial maps a predicate verdict onto the service's error kinds: a block is +// ErrBlocked (its own client-visible code), every other refusal ErrForbidden +// carrying the predicate's reason. +func denial(err error) error { + switch { + case err == nil: + return nil + case errors.Is(err, permissions.ErrBlocked): + return fmt.Errorf("%w: user is blocked", ErrBlocked) + default: + return fmt.Errorf("%w: %v", ErrForbidden, err) } - return nil } // requireChannelWritable refuses a write against an archived non-DM channel. diff --git a/Server/service/permission.go b/Server/service/permission.go index e804bd41..5602d792 100644 --- a/Server/service/permission.go +++ b/Server/service/permission.go @@ -96,6 +96,24 @@ func (s *PermissionService) HasChannelPermChecked(ctx context.Context, userID, c return s.checker.HasChannelPermBatch(cp.rolePerms, cp.overrides, channelID, perm), nil } +// Subject resolves the user's role bits and both override layers for +// channelID from the per-user cache, as a permissions.Subject for the +// value-taking predicates (CanSendMessage and friends). Channel flags and DM +// state are the caller's to fill in. A missing role row yields the zero +// Subject (no bits — every predicate refuses it) with a nil error; a store +// failure is returned so callers choose between failing closed and +// reporting it. +func (s *PermissionService) Subject(ctx context.Context, userID, channelID int64) (permissions.Subject, error) { + cp, err := s.getOrPopulate(ctx, userID) + if err != nil { + return permissions.Subject{}, err + } + if cp == nil { + return permissions.Subject{}, nil + } + return permissions.Subject{RolePerms: cp.rolePerms, Override: cp.overrides[channelID]}, nil +} + // RequireChannelAccess checks whether the user can access the channel with // the given permission. For DM channels it verifies participant membership. // For regular channels it uses cached role-based permission checks. diff --git a/Server/service/predicate_parity_test.go b/Server/service/predicate_parity_test.go new file mode 100644 index 00000000..8771459c --- /dev/null +++ b/Server/service/predicate_parity_test.go @@ -0,0 +1,166 @@ +package service + +import ( + "context" + "errors" + "testing" + + "github.com/J3vb/OwnCord/Server/db" + "github.com/J3vb/OwnCord/Server/permissions" +) + +// B2-5 parity tables: every service call site that decides a security +// property is run against the canonical permissions predicate over the same +// fixture, so the two can never disagree. The fixture covers each input the +// predicates consult — role bits, both override layers, channel type and +// archive flag, DM membership and blocks. + +const ( + parityRoleMember = int64(20) // READ|SEND + parityRoleReader = int64(21) // READ only + parityRoleMod = int64(22) // READ|SEND|MANAGE_MESSAGES + parityRoleAdmin = int64(23) // ADMINISTRATOR only + + parityUserMember = int64(1) + parityUserReader = int64(2) + parityUserMod = int64(3) + parityUserAdmin = int64(4) + parityUserBob = int64(5) // member; DM partner + parityUserEve = int64(6) // member; blocked by Bob + + parityChanText = int64(10) + parityChanAnnouncement = int64(11) + parityChanArchived = int64(12) + parityChanRoleDeny = int64(13) // role override denies SEND to member role + parityChanUserDeny = int64(14) // per-user override denies READ to the member user + parityChanUserAllow = int64(15) // per-user override grants MANAGE_MESSAGES to the member user; announcement + parityChanDM = int64(50) // member <-> bob + parityChanDMBlocked = int64(51) // eve <-> bob, bob blocks eve + parityChanMissing = int64(999) +) + +var parityUsers = []int64{parityUserMember, parityUserReader, parityUserMod, parityUserAdmin, parityUserBob, parityUserEve} + +var parityChannels = []int64{ + parityChanText, parityChanAnnouncement, parityChanArchived, parityChanRoleDeny, + parityChanUserDeny, parityChanUserAllow, parityChanDM, parityChanDMBlocked, parityChanMissing, +} + +func newParityDB(t *testing.T) *db.DB { + t.Helper() + database := newTestDB(t) + seedRole(t, database, &db.Role{ID: parityRoleMember, Name: "p-member", Permissions: permissions.ReadMessages | permissions.SendMessages, Position: 1}) + seedRole(t, database, &db.Role{ID: parityRoleReader, Name: "p-reader", Permissions: permissions.ReadMessages, Position: 1}) + seedRole(t, database, &db.Role{ID: parityRoleMod, Name: "p-mod", Permissions: permissions.ReadMessages | permissions.SendMessages | permissions.ManageMessages, Position: 2}) + seedRole(t, database, &db.Role{ID: parityRoleAdmin, Name: "p-admin", Permissions: permissions.Administrator, Position: 3}) + for uid, rid := range map[int64]int64{ + parityUserMember: parityRoleMember, parityUserReader: parityRoleReader, parityUserMod: parityRoleMod, + parityUserAdmin: parityRoleAdmin, parityUserBob: parityRoleMember, parityUserEve: parityRoleMember, + } { + seedUser(t, database, &db.User{ID: uid, Username: seedUsername(uid)}) + seedUserRole(t, database, uid, rid) + } + seedChannel(t, database, &db.Channel{ID: parityChanText, Name: "text", Type: "text"}) + seedChannel(t, database, &db.Channel{ID: parityChanAnnouncement, Name: "news", Type: "announcement"}) + seedChannel(t, database, &db.Channel{ID: parityChanArchived, Name: "old", Type: "text"}) + if _, err := database.ExecContext(context.Background(), `UPDATE channels SET archived = 1 WHERE id = ?`, parityChanArchived); err != nil { + t.Fatalf("archive: %v", err) + } + seedChannel(t, database, &db.Channel{ID: parityChanRoleDeny, Name: "role-deny", Type: "text"}) + seedChannelOverride(t, database, parityRoleMember, parityChanRoleDeny, 0, permissions.SendMessages) + seedChannel(t, database, &db.Channel{ID: parityChanUserDeny, Name: "user-deny", Type: "text"}) + seedChannelUserOverride(t, database, parityUserMember, parityChanUserDeny, 0, permissions.ReadMessages) + seedChannel(t, database, &db.Channel{ID: parityChanUserAllow, Name: "user-allow", Type: "announcement"}) + seedChannelUserOverride(t, database, parityUserMember, parityChanUserAllow, permissions.ManageMessages, 0) + seedChannel(t, database, &db.Channel{ID: parityChanDM, Name: "dm", Type: "dm"}) + seedDMParticipant(t, database, parityChanDM, parityUserMember) + seedDMParticipant(t, database, parityChanDM, parityUserBob) + seedChannel(t, database, &db.Channel{ID: parityChanDMBlocked, Name: "dm-blocked", Type: "dm"}) + seedDMParticipant(t, database, parityChanDMBlocked, parityUserEve) + seedDMParticipant(t, database, parityChanDMBlocked, parityUserBob) + seedBlock(t, database, parityUserBob, parityUserEve) + return database +} + +// parityWant is the canonical verdict for (user, channel) from the predicate +// over a Subject the test resolves itself — independently of the call site. +func parityWant(t *testing.T, database *db.DB, perms *PermissionService, pred func(permissions.Subject) error, userID, channelID int64) (allowed bool, verdict error) { + t.Helper() + ch, err := database.GetChannel(context.Background(), channelID) + if err != nil { + t.Fatalf("GetChannel(%d): %v", channelID, err) + } + if ch == nil { + return false, ErrNotFound + } + sub, err := channelSubject(context.Background(), database, perms, userID, ch, true) + if err != nil { + t.Fatalf("channelSubject(%d,%d): %v", userID, channelID, err) + } + verdict = pred(sub) + return verdict == nil, verdict +} + +// TestSendPolicyParity: CanPost (the send path) and HandleTyping (S-01) agree +// with CanSendMessage for every (user, channel) in the fixture, including the +// kind of refusal. +func TestSendPolicyParity(t *testing.T) { + database := newParityDB(t) + perms := NewPermissionService(database, permissions.NewChecker(database)) + msgSvc := NewMessageService(database, perms, nil) + chSvc := NewChannelService(database, perms) + ctx := context.Background() + + for _, uid := range parityUsers { + for _, cid := range parityChannels { + wantOK, want := parityWant(t, database, perms, permissions.CanSendMessage, uid, cid) + + got := msgSvc.CanPost(ctx, uid, cid) + if (got == nil) != wantOK { + t.Errorf("CanPost(user=%d, chan=%d) = %v, predicate says %v", uid, cid, got, want) + } + switch { + case errors.Is(want, permissions.ErrBlocked) && !errors.Is(got, ErrBlocked): + t.Errorf("CanPost(user=%d, chan=%d) = %v, want ErrBlocked", uid, cid, got) + case errors.Is(want, ErrNotFound) && !errors.Is(got, ErrNotFound): + t.Errorf("CanPost(user=%d, chan=%d) = %v, want ErrNotFound", uid, cid, got) + case want != nil && !errors.Is(want, permissions.ErrBlocked) && !errors.Is(want, ErrNotFound) && !errors.Is(got, ErrForbidden): + t.Errorf("CanPost(user=%d, chan=%d) = %v, want ErrForbidden", uid, cid, got) + } + + ch, err := chSvc.HandleTyping(ctx, uid, cid, nil) + if err != nil { + t.Errorf("HandleTyping(user=%d, chan=%d) errored: %v", uid, cid, err) + } + if (ch != nil) != wantOK { + t.Errorf("HandleTyping(user=%d, chan=%d) emits=%v, but CanSendMessage says %v (S-01: typing must follow the send policy)", uid, cid, ch != nil, want) + } + } + } +} + +// TestViewPolicyParity: HandleChannelFocus (session admission: channel_focus +// and mark_read) agrees with CanAdmitSession for every (user, channel). +func TestViewPolicyParity(t *testing.T) { + database := newParityDB(t) + perms := NewPermissionService(database, permissions.NewChecker(database)) + chSvc := NewChannelService(database, perms) + ctx := context.Background() + + for _, uid := range parityUsers { + for _, cid := range parityChannels { + wantOK, want := parityWant(t, database, perms, permissions.CanAdmitSession, uid, cid) + + _, got := chSvc.HandleChannelFocus(ctx, uid, cid) + if (got == nil) != wantOK { + t.Errorf("HandleChannelFocus(user=%d, chan=%d) = %v, predicate says %v", uid, cid, got, want) + } + switch { + case errors.Is(want, ErrNotFound) && !errors.Is(got, ErrNotFound): + t.Errorf("HandleChannelFocus(user=%d, chan=%d) = %v, want ErrNotFound", uid, cid, got) + case want != nil && !errors.Is(want, ErrNotFound) && !errors.Is(got, ErrForbidden): + t.Errorf("HandleChannelFocus(user=%d, chan=%d) = %v, want ErrForbidden", uid, cid, got) + } + } + } +} diff --git a/Server/ws/can_send_test.go b/Server/ws/can_send_test.go index ece56f13..9447c65c 100644 --- a/Server/ws/can_send_test.go +++ b/Server/ws/can_send_test.go @@ -38,6 +38,17 @@ func TestChannelCanSend(t *testing.T) { if got := channelCanSend(c.role, c.o, c.ctype); got != c.want { t.Errorf("%s: channelCanSend = %v, want %v", c.name, got, c.want) } + // B2-5 parity: the affordance is the canonical send predicate. + var bits int64 + if c.role != nil { + bits = c.role.Permissions + } + want := permissions.CanSendMessage(permissions.Subject{ + RolePerms: bits, Override: permOverride(c.o), Channel: permissions.ChannelRef{Type: c.ctype}, + }) == nil + if want != c.want { + t.Errorf("%s: CanSendMessage = %v, channelCanSend table says %v", c.name, want, c.want) + } } } diff --git a/Server/ws/coverage_voice_lifecycle_test.go b/Server/ws/coverage_voice_lifecycle_test.go index 2e166314..c5abd19a 100644 --- a/Server/ws/coverage_voice_lifecycle_test.go +++ b/Server/ws/coverage_voice_lifecycle_test.go @@ -67,16 +67,18 @@ func TestHandleVoiceTokenRefresh_InVoice_ReturnsToken(t *testing.T) { func TestHandleVoiceTokenRefresh_NilUser(t *testing.T) { hub, database := newCoverageHub(t) // The client deliberately carries no *db.User — that is what this test - // covers — but the row must exist so the CONNECT_VOICE re-check can resolve - // a role. Without it the handler stops at FORBIDDEN and never reaches the - // missing-voice-state branch under test. + // covers — but the user row and the channel row must exist so the join + // gate the refresh re-runs (permissions.CanJoinVoice) can resolve a role + // and a channel. Without them the handler stops at FORBIDDEN and never + // reaches the missing-voice-state branch under test. user := seedCoverageOwner(t, database, "vtr-nil-user") + chID := seedVoiceChannel(t, database, "vtr-nil-user-chan") send := make(chan []byte, 16) c := ws.NewTestClient(hub, user.ID, send) hub.Register(c) waitRegistered(t, hub, c) - ws.SetVoiceChIDForTest(c, 42) + ws.SetVoiceChIDForTest(c, chID) hub.HandleMessageForTest(c, voiceTokenRefreshMsg()) diff --git a/Server/ws/deps.go b/Server/ws/deps.go index 3598e2d7..cb6e9cd1 100644 --- a/Server/ws/deps.go +++ b/Server/ws/deps.go @@ -2,6 +2,7 @@ package ws import ( "context" + "errors" "log/slog" "github.com/J3vb/OwnCord/Server/auth" @@ -178,125 +179,82 @@ func hasPerm(ctx context.Context, database *db.DB, perms *permissions.Checker, p return perms.HasChannelPerm(ctx, role.Permissions, role.ID, userID, channelID, perm) } -// hasPermChecked is hasPerm's error-preserving counterpart: it distinguishes -// "the role/override lookup failed" (err != nil, verdict meaningless) from -// "the lookup answered and the bit is absent" (false, nil error). hasPerm and -// hasChannelAccess both collapse that distinction to a fail-closed false, -// which is the correct posture for every gate that sends FORBIDDEN on denial -// (requireChannelAccess, requirePerm, and friends) — do not route those -// through this helper. It exists for a caller like applySetChannelID's -// post-Subscribe revalidation (OC-0266), which documents that a transient -// lookup error must NOT be treated as a denial: unwinding on a DB hiccup -// would silently kill the channel's live message stream with no error frame -// ever sent to the client. -func hasPermChecked(ctx context.Context, database *db.DB, perms *permissions.Checker, permSvc *service.PermissionService, userID, channelID, perm int64) (bool, error) { +// subjectFor resolves userID's role bits and both override layers for +// channelID — from the PermissionService cache when one is wired, else live +// through the Checker — as a permissions.Subject for the value-taking +// predicates (CanSendMessage, CanJoinVoice, ...). Channel flags and DM state +// stay the caller's to fill in. A lookup failure is returned, not collapsed +// (hasPerm and hasChannelAccess collapse it to a fail-closed false, the right +// posture for every gate that sends FORBIDDEN on denial), so an error-aware +// caller like applySetChannelID's post-Subscribe revalidation (OC-0266) can +// tell a transient read failure from a denial; a missing role row is the +// zero Subject, which every predicate refuses. +func subjectFor(ctx context.Context, database *db.DB, perms *permissions.Checker, permSvc *service.PermissionService, userID, channelID int64) (permissions.Subject, error) { if permSvc != nil { - return permSvc.HasChannelPermChecked(ctx, userID, channelID, perm) + return permSvc.Subject(ctx, userID, channelID) } if database == nil || perms == nil { - return false, nil + return permissions.Subject{}, nil } role, err := database.GetRoleForUser(ctx, userID) if err != nil { - return false, err + return permissions.Subject{}, err } if role == nil { - return false, nil + return permissions.Subject{}, nil } - return perms.HasChannelPerm(ctx, role.Permissions, role.ID, userID, channelID, perm), nil + return perms.Subject(ctx, role.Permissions, role.ID, userID, channelID) } -// 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. -// -// With a PermissionService the role-bit gate is answered from its per-user -// cache (the channel-type lookup and the DM membership check stay live — -// dm_participants rows are membership, not permission, state and are never -// cached). Both branches enforce the same rule: role bit required on top, DM -// membership via the single shared IsDMParticipant definition. -func hasChannelAccess(ctx context.Context, database *db.DB, perms *permissions.Checker, permSvc *service.PermissionService, userID, channelID, perm int64) bool { - if database == nil { - return false - } - if permSvc == nil { - return hasChannelAccessLive(ctx, database, perms, userID, channelID, perm) - } - if !permSvc.HasChannelPerm(ctx, userID, 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 - } - // A missing channel row takes the non-DM branch, 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. - if ch == nil || ch.Type != "dm" { - return true - } - // DM: for "dm" the service's RequireChannelAccess is exactly the - // IsDMParticipant membership rule (it waives the role check, which was - // already enforced above). - return permSvc.RequireChannelAccess(ctx, userID, ch.Type, channelID, perm) == nil +// subjectFor is the hub-wired form of the package-level subjectFor. +func (h *Hub) subjectFor(ctx context.Context, userID, channelID int64) (permissions.Subject, error) { + return subjectFor(ctx, h.db, h.permChecker, h.perms, userID, channelID) } -// hasChannelAccessLive is the uncached hasChannelAccess path, kept verbatim for -// hubs and deps constructed without a PermissionService (bare test fixtures). -func hasChannelAccessLive(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, userID, channelID, perm) { - return false - } - ch, err := database.GetChannel(ctx, channelID) +// channelSubject is subjectFor plus the channel's flags and, for a DM, the +// membership and (withBlock) two-party block state — everything CanJoinVoice +// and CanModerateVoice consult. Membership and blocks are always read live +// (dm_participants rows are membership, not permission, state and are never +// cached). An error is a lookup failure, never a denial; callers decide +// whether that fails closed. +func channelSubject(ctx context.Context, database *db.DB, perms *permissions.Checker, permSvc *service.PermissionService, userID int64, ch *db.Channel, withBlock bool) (permissions.Subject, error) { + sub, err := subjectFor(ctx, database, perms, permSvc, userID, ch.ID) 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 + return permissions.Subject{}, err } - // A missing channel row takes the non-DM branch, 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. - if ch == nil || ch.Type != "dm" { - // For every non-DM type, RequireChannelAccess is defined as exactly the - // HasChannelPerm call already made above, so re-invoking it would only - // repeat the same override lookup. The role verdict is the answer. - return true + sub.Channel = channelRef(ch) + if ch.Type != "dm" || database == nil { + return sub, nil + } + ok, err := database.IsDMParticipant(ctx, userID, ch.ID) + if err != nil { + return permissions.Subject{}, err + } + sub.DMParticipant = ok + if ok && withBlock { + switch err := service.RequireDMNotBlocked(ctx, database, userID, ch.ID); { + case errors.Is(err, service.ErrBlocked): + sub.DMBlocked = true + case err != nil: + return permissions.Subject{}, err + } + } + return sub, nil +} + +// joinDenial maps a CanJoinVoice refusal to the error frame the voice_join +// gate has always sent for that reason. +func joinDenial(err error) ClientError { + switch { + case errors.Is(err, permissions.ErrNotVoiceChannel): + return ClientError{Code: ErrCodeBadRequest, Message: "not a voice channel"} + case errors.Is(err, permissions.ErrArchived): + return ClientError{Code: ErrCodeBadRequest, Message: "channel is archived"} + case errors.Is(err, permissions.ErrBlocked): + return ClientError{Code: ErrCodeForbidden, Message: "cannot join voice: blocked"} + default: + return ClientError{Code: ErrCodeForbidden, Message: "missing CONNECT_VOICE permission"} } - // DM: the role bit above stays required on top; the membership rule keeps - // its single shared definition in RequireChannelAccess (IsDMParticipant), - // which waives the role check for DMs. - return perms.RequireChannelAccess(ctx, userID, role.Permissions, role.ID, ch.Type, channelID, perm) == nil } // ── V2 handler type ───────────────────────────────────────────────────────── diff --git a/Server/ws/handlers.go b/Server/ws/handlers.go index 91df62f3..4c613bb3 100644 --- a/Server/ws/handlers.go +++ b/Server/ws/handlers.go @@ -271,33 +271,33 @@ func (h *Hub) applySetChannelID(c *Client, newChID int64) { return } h.pubsub.Subscribe(c, ChannelTopic(newChID)) - // The re-validation mirrors HandleChannelFocus's admission gate: DMs are - // participant-gated (the READ role bit is deliberately waived), non-DMs - // need READ_MESSAGES, and a deleted channel is a denial. A transient - // lookup error is NOT a denial — the recheck exists to catch a concrete + // The re-validation is permissions.CanAdmitSession — the same predicate + // as HandleChannelFocus's admission gate (S-12): DMs are participant-gated + // (the READ role bit is deliberately waived), non-DMs need READ_MESSAGES + // and no archive, and a deleted channel is a denial. A transient lookup + // error is NOT a denial (OC-0266) — the recheck exists to catch a concrete // revoke in the Subscribe race window, the sweeps stay authoritative, and // unwinding on error would turn any DB hiccup into a silently dead - // message stream with no error frame sent to the client. + // message stream with no error frame sent to the client; subjectFor and + // the DM lookup both report a failure instead of collapsing it. ch, chErr := h.db.GetChannel(c.ctx, newChID) if chErr != nil { return } - if ch != nil && ch.Type == "dm" { - if ok, dmErr := h.db.IsDMParticipant(c.ctx, c.userID, newChID); dmErr != nil || ok { + if ch != nil { + sub, subErr := h.subjectFor(c.ctx, c.userID, newChID) + if subErr != nil { return } - } else if ch != nil && !ch.Archived { - // hasPermChecked, not hasChannelAccess: ch is already in hand and known - // non-DM here, so the role/override bit is the whole answer (a second - // hasChannelAccess-internal GetChannel would only repeat the read - // above), and unlike hasChannelAccess it reports a lookup failure - // instead of collapsing it to a denial — required by the "transient - // lookup error is NOT a denial" contract documented above (OC-0266). - allowed, permErr := hasPermChecked(c.ctx, h.db, h.permChecker, h.perms, c.userID, newChID, permissions.ReadMessages) - if permErr != nil { - return + sub.Channel = channelRef(ch) + if ch.Type == "dm" { + ok, dmErr := h.db.IsDMParticipant(c.ctx, c.userID, newChID) + if dmErr != nil { + return + } + sub.DMParticipant = ok } - if allowed { + if permissions.CanAdmitSession(sub) == nil { return } } @@ -333,23 +333,6 @@ func (h *Hub) hasChannelPerm(ctx context.Context, c *Client, channelID int64, pe return h.permChecker.HasChannelPerm(ctx, role.Permissions, role.ID, c.userID, 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, h.perms, c.userID, channelID, perm) { - return true - } - slog.Warn("ws permission denied", "user_id", c.userID, "channel_id", channelID, "perm", permLabel) - c.sendMsg(buildErrorMsg(ErrCodeForbidden, "missing "+permLabel+" permission")) - return false -} - // broadcastExcludeLow sends a message at low priority to all clients in the // sender's channel EXCEPT the sender. Messages sent via this function are NOT // stored in the replay ring buffer — they are ephemeral. This is correct for diff --git a/Server/ws/hub_broadcast.go b/Server/ws/hub_broadcast.go index 1646051e..13e3fac3 100644 --- a/Server/ws/hub_broadcast.go +++ b/Server/ws/hub_broadcast.go @@ -225,6 +225,7 @@ func (h *Hub) channelReadAudienceImpl(ctx context.Context, channelID int64, igno // events to the whole server. Resolve the DM's real audience (its // participants, intersected with who is actually connected) instead, // mirroring the IsDMParticipant membership rule hasChannelAccess uses. + var ref permissions.ChannelRef if h.db != nil { ch, err := h.db.GetChannel(ctx, channelID) if err != nil { @@ -244,43 +245,36 @@ func (h *Hub) channelReadAudienceImpl(ctx context.Context, channelID int64, igno if ch == nil { return []int64{} } - // Archived channels are hidden from every client regardless of - // permissions, mirroring RefreshChannelVisibility and VisibleChannelIDs. - // Without this, an admin edit to an archived channel (or a voice - // teardown inside one) fans out straight to every connected user whose - // base role holds READ_MESSAGES, none of whom have the channel in their - // ready payload or sidebar. ignoreArchived opts a caller out of this - // specific check only — see channelReadAudienceIgnoringArchived. - if ch.Archived && !ignoreArchived { - return []int64{} - } if ch.Type == "dm" { return h.channelReadAudienceDM(ctx, channelID, userIDs) } + ref = channelRef(ch) + // CanViewChannel hides an archived channel from everyone, mirroring + // RefreshChannelVisibility and VisibleChannelIDs: without that, an + // admin edit to an archived channel (or a voice teardown inside one) + // would fan out to every connected user whose base role holds + // READ_MESSAGES, none of whom have the channel in their sidebar. + // ignoreArchived resolves the pre-archival audience instead — see + // channelReadAudienceIgnoringArchived. + if ignoreArchived { + ref.Archived = false + } } - audience := make([]int64, 0, len(userIDs)) - if h.perms != nil { - for _, uid := range userIDs { - if h.perms.HasChannelPerm(ctx, uid, channelID, permissions.ReadMessages) { - audience = append(audience, uid) - } - } - return audience - } - if h.db == nil || h.permChecker == nil { - return audience - } // Resolved per USER, not memoised per role: channel_user_overrides is the // last layer of the resolution order, so two members of the same role can // legitimately disagree about one channel and a per-role memo would hand - // one of them the other's verdict. + // one of them the other's verdict. The verdict is CanViewChannel over + // subjectFor (cached service or live checker); an unresolvable user is + // left out. + audience := make([]int64, 0, len(userIDs)) for _, uid := range userIDs { - role, err := h.db.GetRoleForUser(ctx, uid) - if err != nil || role == nil { + sub, err := h.subjectFor(ctx, uid, channelID) + if err != nil { continue } - if h.permChecker.HasChannelPerm(ctx, role.Permissions, role.ID, uid, channelID, permissions.ReadMessages) { + sub.Channel = ref + if permissions.CanViewChannel(sub) == nil { audience = append(audience, uid) } } @@ -399,56 +393,33 @@ func (h *Hub) RefreshChannelVisibility(ch *db.Channel) { // the targeted re-sync must complete regardless of the triggering request. ctx := context.Background() - // Visibility is resolved per user. With a PermissionService it comes from - // the per-user cache — safe because the admin handlers invalidate - // (InvalidateAll on override change, InvalidateUser on role change) before - // calling into the hub, so the lookups below repopulate from post-change - // data; the 30s TTL is only a backstop and the F6 gen-counter guard keeps - // a racing populate from caching stale rows. Without a service (bare test - // hubs) each client is resolved live. + // Visibility is CanViewChannel — the single predicate shared with + // buildReady / REST ListVisibleChannels — resolved per user from their + // CURRENT role (c.user is a connect-time snapshot). With a + // PermissionService the subject comes from the per-user cache — safe + // because the admin handlers invalidate (InvalidateAll on override + // change, InvalidateUser on role change) before calling into the hub, so + // the lookups below repopulate from post-change data; the 30s TTL is only + // a backstop and the F6 gen-counter guard keeps a racing populate from + // caching stale rows. Without a service (bare test hubs) each client is + // resolved live. Fails closed: an unresolvable role loses visibility + // rather than keeping a stale grant. // // Deliberately NOT memoised per role: channel_user_overrides is the last // layer of the resolution order, so two members of the same role can // legitimately disagree about one channel — exactly the case a per-user // override edit creates, and exactly the fan-out this function targets. - userVisible := func(userID, roleID int64) bool { - role, err := h.db.GetRoleByID(ctx, roleID) - if err != nil || role == nil { - return false - } - // Single visibility predicate shared with buildReady / REST - // ListVisibleChannels; the checker fails closed on a lookup error - // and bypasses for admins, matching the other sites exactly. - return h.permChecker.HasChannelPerm(ctx, role.Permissions, roleID, userID, ch.ID, permissions.ReadMessages) - } - for _, c := range clients { if c.user == nil { continue } - var visible bool - switch { - case ch.Archived: - // Archived channels are hidden from every client regardless of - // permissions, mirroring VisibleChannelIDs. - visible = false - case h.perms != nil: - // The service resolves the user's CURRENT role internally (c.user - // is a connect-time snapshot), failing closed — an unresolvable - // role loses visibility rather than keeping a stale grant. - visible = h.perms.HasChannelPerm(ctx, c.user.ID, ch.ID, permissions.ReadMessages) - default: - // c.user is a connect-time snapshot; an admin may have changed the - // user's role mid-session, so resolve the current role from the DB. - // Fail closed: on error send nothing rather than mis-target. - fresh, err := h.db.GetUserByID(ctx, c.user.ID) - if err != nil || fresh == nil { - slog.Warn("hub: RefreshChannelVisibility could not resolve user role", - "user_id", c.user.ID, "err", err) - continue - } - visible = userVisible(fresh.ID, fresh.RoleID) + sub, err := h.subjectFor(ctx, c.user.ID, ch.ID) + if err != nil { + slog.Warn("hub: RefreshChannelVisibility could not resolve permissions, revoking", + "user_id", c.user.ID, "channel_id", ch.ID, "err", err) } + sub.Channel = channelRef(ch) + visible := err == nil && permissions.CanViewChannel(sub) == nil if refreshChannelVisibilityRaceHook != nil { refreshChannelVisibilityRaceHook(c.user.ID) @@ -475,7 +446,7 @@ func (h *Hub) RefreshChannelVisibility(ch *db.Channel) { // Addressed per client so it can carry this recipient's own // can_send verdict — the whole point of this fan-out is that a // permission change just made those verdicts diverge. - live.sendMsg(buildChannelCreateFor(ch, h.refreshChannelVisibilityCanSend(ctx, ch, c.user.ID, c.user.RoleID))) + live.sendMsg(buildChannelCreateFor(ch, h.refreshChannelVisibilityCanSend(ctx, ch, c.user.ID))) continue } live.sendMsg(buildChannelDelete(ch.ID)) @@ -496,33 +467,22 @@ func (h *Hub) RefreshChannelVisibility(ch *db.Channel) { h.bumpVisibilityWatermark() } -// refreshChannelVisibilityCanSend mirrors channelCanSend (serve_ready.go) — the value the ready -// payload ships per channel — but expressed as per-user permission checks -// so it works in both the service and bare-hub branches without needing a -// resolved *db.Role. HasChannelPerm already bypasses for admins and fails -// closed on a lookup error, matching channelCanSend's own admin shortcut. +// refreshChannelVisibilityCanSend is the can_send verdict the ready payload +// ships per channel (channelCanSend), recomputed for one live user from their +// CURRENT role: permissions.CanSendMessage over the subject subjectFor +// resolves in either the service or the bare-hub branch, failing closed on a +// lookup error (S-12). // // Without this, can_send is only ever computed at connect time, so a role // edit or override edit leaves every connected client's composer stuck on // its stale connect-time verdict until the socket is rebuilt. -func (h *Hub) refreshChannelVisibilityCanSend(ctx context.Context, ch *db.Channel, userID, roleID int64) bool { - has := func(perm int64) bool { - if h.perms != nil { - return h.perms.HasChannelPerm(ctx, userID, ch.ID, perm) - } - role, err := h.db.GetRoleByID(ctx, roleID) - if err != nil || role == nil { - return false - } - return h.permChecker.HasChannelPerm(ctx, role.Permissions, roleID, userID, ch.ID, perm) - } - if !has(permissions.ReadMessages) || !has(permissions.SendMessages) { +func (h *Hub) refreshChannelVisibilityCanSend(ctx context.Context, ch *db.Channel, userID int64) bool { + sub, err := h.subjectFor(ctx, userID, ch.ID) + if err != nil { return false } - if ch.Type == "announcement" { - return has(permissions.ManageMessages) - } - return true + sub.Channel = channelRef(ch) + return permissions.CanSendMessage(sub) == nil } // RefreshAllChannelVisibility re-runs RefreshChannelVisibility for every diff --git a/Server/ws/hub_sweep.go b/Server/ws/hub_sweep.go index 43719234..09d1dfdd 100644 --- a/Server/ws/hub_sweep.go +++ b/Server/ws/hub_sweep.go @@ -182,10 +182,10 @@ func (h *Hub) sweepStaleVoiceEvictRevoked(ctx context.Context) { if chID == 0 { continue } - allowed, err := h.hasChannelPermChecked(ctx, c.userID, chID, permissions.ConnectVoice) + allowed, err := h.voiceStillAllowed(ctx, c.userID, chID) if err != nil { // A transient read failure (I/O error, lock contention, a - // maintenance window) is not a revocation — hasChannelPerm and + // maintenance window) is not a revocation — hasChannelAccess and // permissions.Checker.HasChannelPerm both collapse any DB error // to "denied", which would otherwise evict every in-voice // participant on one bad read. Skip this client this tick; the @@ -317,40 +317,29 @@ func (h *Hub) sweepStaleVoiceStates() { // tests use this hook to reproduce it deterministically. var sweepStaleVoiceJoinRaceHook func(userID, channelID int64, joinedAt string) -// hasChannelPermChecked is hasChannelPerm's error-aware counterpart: it -// distinguishes a genuine permission denial (role missing, or the effective -// permission bits don't include perm) from a DB read failure, by inlining the -// same resolution hasChannelPerm/permissions.Checker.HasChannelPerm perform — -// both of which collapse any error into "denied", indistinguishable from a -// real revocation. sweepStaleVoiceStates needs that distinction: unlike a -// handler answering one client's request, it evicts a live voice session on -// "denied", so a transient read failure must not be treated as a revocation. -func (h *Hub) hasChannelPermChecked(ctx context.Context, userID, channelID int64, perm int64) (allowed bool, err error) { - role, err := h.db.GetRoleForUser(ctx, userID) +// voiceStillAllowed is the sweep's error-aware re-run of the join gate: it +// distinguishes a genuine refusal (permissions.CanJoinVoice over the live +// subject — the bit revoked, the channel archived or deleted, DM membership +// or block state changed) from a DB read failure. sweepStaleVoiceStates +// needs that distinction: unlike a handler answering one client's request, +// it evicts a live voice session on "denied", so a transient read failure +// must not be treated as a revocation. Deliberately read live, never through +// the cached PermissionService: this is the last-line backstop, and staying +// authoritative for a change that somehow bypassed the invalidation hooks is +// worth the handful of reads a minute it costs for the clients in voice. +func (h *Hub) voiceStillAllowed(ctx context.Context, userID, channelID int64) (allowed bool, err error) { + ch, err := h.db.GetChannel(ctx, channelID) if err != nil { return false, err } - if role == nil { - // No role row is a genuine deny, not an error — mirrors - // hasChannelPerm's role == nil case. + if ch == nil { return false, nil } - if permissions.HasAdmin(role.Permissions) { - return true, nil - } - allow, deny, err := h.db.GetChannelPermissions(ctx, channelID, role.ID) + sub, err := channelSubject(ctx, h.db, h.permChecker, nil, userID, ch, true) if err != nil { return false, err } - o := permissions.ChannelOverride{Allow: allow, Deny: deny} - if userID != 0 { - uAllow, uDeny, uErr := h.db.GetUserChannelPermissions(ctx, channelID, userID) - if uErr != nil { - return false, uErr - } - o.UserAllow, o.UserDeny = uAllow, uDeny - } - return permissions.EffectiveChannelPerms(role.Permissions, o)&perm == perm, nil + return permissions.CanJoinVoice(sub) == nil, nil } // cleanupVoiceRaceClearHook, when non-nil, runs immediately before diff --git a/Server/ws/predicate_parity_internal_test.go b/Server/ws/predicate_parity_internal_test.go new file mode 100644 index 00000000..0cf9eeaa --- /dev/null +++ b/Server/ws/predicate_parity_internal_test.go @@ -0,0 +1,419 @@ +package ws + +import ( + "context" + "encoding/json" + "fmt" + "testing" + "time" + + "github.com/J3vb/OwnCord/Server/auth" + "github.com/J3vb/OwnCord/Server/db" + "github.com/J3vb/OwnCord/Server/permissions" + "github.com/J3vb/OwnCord/Server/service" +) + +// B2-5 parity tables for the ws call sites that decide a security property: +// each site is run against the canonical permissions predicate over the same +// fixture, in both the PermissionService-wired and the bare-hub branch, so +// the two resolution paths and the rule can never disagree (S-12). + +// parityOverrideCases are the override layers every ws parity table walks: +// each one flips a bit the predicates consult. +var parityOverrideCases = []struct { + name string + allow, deny int64 // role layer + uAllow int64 // user layer + uDeny int64 +}{ + {"no override", 0, 0, 0, 0}, + {"role deny SEND", 0, permissions.SendMessages, 0, 0}, + {"role deny READ", 0, permissions.ReadMessages, 0, 0}, + {"role deny CONNECT", 0, permissions.ConnectVoice, 0, 0}, + {"role deny MUTE", 0, permissions.MuteMembers, 0, 0}, + {"role allow MANAGE", permissions.ManageMessages, 0, 0, 0}, + {"user deny SEND", 0, 0, 0, permissions.SendMessages}, + {"user deny READ", 0, 0, 0, permissions.ReadMessages}, + {"user deny MUTE", 0, 0, 0, permissions.MuteMembers}, + {"user allow MANAGE", 0, 0, permissions.ManageMessages, 0}, + {"user allow beats role deny", 0, permissions.SendMessages, permissions.SendMessages, 0}, +} + +// setParityOverrides installs one case's layers for (role, user) on the +// channel and drops the permission cache so the service branch re-reads. +func setParityOverrides(t *testing.T, database *db.DB, permSvc *service.PermissionService, chID, roleID, userID int64, c struct { + name string + allow, deny int64 + uAllow int64 + uDeny int64 +}, +) { + t.Helper() + ctx := context.Background() + if err := database.UpsertChannelOverride(ctx, chID, roleID, c.allow, c.deny); err != nil { + t.Fatalf("%s: UpsertChannelOverride: %v", c.name, err) + } + if err := database.UpsertChannelUserOverride(ctx, chID, userID, c.uAllow, c.uDeny); err != nil { + t.Fatalf("%s: UpsertChannelUserOverride: %v", c.name, err) + } + permSvc.InvalidateAll() +} + +// paritySubject resolves the subject the way the test wants it, straight from +// the Checker, so the site under test is compared against an independent +// resolution rather than its own. +func paritySubject(t *testing.T, database *db.DB, userID int64, ch *db.Channel) permissions.Subject { + t.Helper() + ctx := context.Background() + role, err := database.GetRoleForUser(ctx, userID) + if err != nil || role == nil { + t.Fatalf("GetRoleForUser(%d): %v", userID, err) + } + sub, err := permissions.NewChecker(database).Subject(ctx, role.Permissions, role.ID, userID, ch.ID) + if err != nil { + t.Fatalf("Checker.Subject: %v", err) + } + sub.Channel = channelRef(ch) + return sub +} + +// TestRefreshChannelVisibilityCanSend_Parity: the composer refresh verdict is +// CanSendMessage in both branches, for text and announcement channels. +func TestRefreshChannelVisibilityCanSend_Parity(t *testing.T) { + ctx := context.Background() + database := newHarvestVoiceDB(t) + uid := seedHarvestVoiceUser(t, database, "refresh-parity-user") + textID := mustCreateVoiceChannel(t, database, "refresh-parity-text") + if _, err := database.ExecContext(ctx, `UPDATE channels SET type = 'text' WHERE id = ?`, textID); err != nil { + t.Fatalf("retype: %v", err) + } + newsID := mustCreateVoiceChannel(t, database, "refresh-parity-news") + if _, err := database.ExecContext(ctx, `UPDATE channels SET type = 'announcement' WHERE id = ?`, newsID); err != nil { + t.Fatalf("retype: %v", err) + } + + h := NewHub(database, auth.NewRateLimiter(), nil) + permSvc := service.NewPermissionService(database, h.permChecker) + + for _, chID := range []int64{textID, newsID} { + ch, err := database.GetChannel(ctx, chID) + if err != nil || ch == nil { + t.Fatalf("GetChannel(%d): %v", chID, err) + } + for _, c := range parityOverrideCases { + setParityOverrides(t, database, permSvc, chID, harvestVoiceRoleID, uid, c) + want := permissions.CanSendMessage(paritySubject(t, database, uid, ch)) == nil + + h.perms = nil + if got := h.refreshChannelVisibilityCanSend(ctx, ch, uid); got != want { + t.Errorf("%s/%s bare hub: refreshChannelVisibilityCanSend = %v, CanSendMessage = %v", ch.Type, c.name, got, want) + } + h.perms = permSvc + if got := h.refreshChannelVisibilityCanSend(ctx, ch, uid); got != want { + t.Errorf("%s/%s service: refreshChannelVisibilityCanSend = %v, CanSendMessage = %v", ch.Type, c.name, got, want) + } + } + } +} + +// viewParityFixture is a bare hub with one registered client on a text +// channel plus a second, archived channel, shared by the view-property +// parity tables below. +type viewParityFixture struct { + h *Hub + database *db.DB + permSvc *service.PermissionService + user *db.User + textID int64 + oldID int64 // archived + client *Client + send chan []byte +} + +func newViewParityFixture(t *testing.T) *viewParityFixture { + t.Helper() + ctx := context.Background() + database := newHarvestVoiceDB(t) + uid := seedHarvestVoiceUser(t, database, "view-parity-user") + textID := mustCreateVoiceChannel(t, database, "view-parity-text") + oldID := mustCreateVoiceChannel(t, database, "view-parity-old") + if _, err := database.ExecContext(ctx, `UPDATE channels SET type = 'text' WHERE id IN (?, ?)`, textID, oldID); err != nil { + t.Fatalf("retype: %v", err) + } + if _, err := database.ExecContext(ctx, `UPDATE channels SET archived = 1 WHERE id = ?`, oldID); err != nil { + t.Fatalf("archive: %v", err) + } + h := NewHub(database, auth.NewRateLimiter(), nil) + user, err := database.GetUserByID(ctx, uid) + if err != nil || user == nil { + t.Fatalf("GetUserByID: %v", err) + } + send := make(chan []byte, 64) + c := NewTestClientWithUser(h, user, textID, send) + h.RegisterNowForTest(c) + return &viewParityFixture{ + h: h, database: database, permSvc: service.NewPermissionService(database, h.permChecker), + user: user, textID: textID, oldID: oldID, client: c, send: send, + } +} + +func (f *viewParityFixture) channel(t *testing.T, id int64) *db.Channel { + t.Helper() + ch, err := f.database.GetChannel(context.Background(), id) + if err != nil || ch == nil { + t.Fatalf("GetChannel(%d): %v", id, err) + } + return ch +} + +// eachBranch runs fn once with the bare hub and once with the cached +// PermissionService wired, labelling the branch. +func (f *viewParityFixture) eachBranch(fn func(branch string)) { + f.h.perms = nil + fn("bare") + f.h.perms = f.permSvc + fn("service") +} + +// TestApplySetChannelID_Parity: the post-Subscribe revalidation keeps the +// subscription exactly when CanAdmitSession allows it — for every override +// layer, and for an archived channel. +func TestApplySetChannelID_Parity(t *testing.T) { + f := newViewParityFixture(t) + for _, chID := range []int64{f.textID, f.oldID} { + ch := f.channel(t, chID) + for _, c := range parityOverrideCases { + setParityOverrides(t, f.database, f.permSvc, chID, harvestVoiceRoleID, f.user.ID, c) + want := permissions.CanAdmitSession(paritySubject(t, f.database, f.user.ID, ch)) == nil + f.eachBranch(func(branch string) { + f.h.applySetChannelID(f.client, 0) // a same-channel focus is a no-op; refocus from scratch + f.h.applySetChannelID(f.client, chID) + if got := f.h.SubscribedToChannelTopicForTest(f.client, chID); got != want { + t.Errorf("chan=%d/%s/%s: subscribed = %v, CanAdmitSession = %v", chID, c.name, branch, got, want) + } + }) + } + } +} + +// TestChannelReadAudience_Parity: a connected user is in a channel's read +// audience exactly when CanViewChannel allows it. +func TestChannelReadAudience_Parity(t *testing.T) { + f := newViewParityFixture(t) + ctx := context.Background() + for _, chID := range []int64{f.textID, f.oldID} { + ch := f.channel(t, chID) + for _, c := range parityOverrideCases { + setParityOverrides(t, f.database, f.permSvc, chID, harvestVoiceRoleID, f.user.ID, c) + want := permissions.CanViewChannel(paritySubject(t, f.database, f.user.ID, ch)) == nil + f.eachBranch(func(branch string) { + got := false + for _, uid := range f.h.channelReadAudience(ctx, chID) { + if uid == f.user.ID { + got = true + } + } + if got != want { + t.Errorf("chan=%d/%s/%s: in audience = %v, CanViewChannel = %v", chID, c.name, branch, got, want) + } + }) + } + } +} + +// voiceParityFixture adds to the view fixture a voice channel, an archived +// voice channel, and a DM with a second user (optionally blocking). +type voiceParityFixture struct { + *viewParityFixture + voiceID, oldVoiceID, dmID int64 + other int64 +} + +func newVoiceParityFixture(t *testing.T) *voiceParityFixture { + t.Helper() + f := newViewParityFixture(t) + ctx := context.Background() + voiceID := mustCreateVoiceChannel(t, f.database, "voice-parity") + oldVoiceID := mustCreateVoiceChannel(t, f.database, "voice-parity-old") + if _, err := f.database.ExecContext(ctx, `UPDATE channels SET archived = 1 WHERE id = ?`, oldVoiceID); err != nil { + t.Fatalf("archive: %v", err) + } + other := seedHarvestVoiceUser(t, f.database, "voice-parity-other") + res, err := f.database.ExecContext(ctx, `INSERT INTO channels (name, type, position) VALUES ('dm-parity', 'dm', 0)`) + if err != nil { + t.Fatalf("insert dm: %v", err) + } + dmID, _ := res.LastInsertId() + for _, uid := range []int64{f.user.ID, other} { + if _, err := f.database.ExecContext(ctx, `INSERT INTO dm_participants (channel_id, user_id) VALUES (?, ?)`, dmID, uid); err != nil { + t.Fatalf("insert dm participant: %v", err) + } + } + return &voiceParityFixture{viewParityFixture: f, voiceID: voiceID, oldVoiceID: oldVoiceID, dmID: dmID, other: other} +} + +// setBlocked makes other block the fixture user (or clears the block). +func (f *voiceParityFixture) setBlocked(t *testing.T, blocked bool) { + t.Helper() + ctx := context.Background() + if _, err := f.database.ExecContext(ctx, `DELETE FROM user_blocks WHERE blocker_id = ? AND blocked_id = ?`, f.other, f.user.ID); err != nil { + t.Fatalf("clear block: %v", err) + } + if blocked { + if _, err := f.database.ExecContext(ctx, `INSERT INTO user_blocks (blocker_id, blocked_id) VALUES (?, ?)`, f.other, f.user.ID); err != nil { + t.Fatalf("insert block: %v", err) + } + } +} + +// joinWant is the independent CanJoinVoice verdict for the fixture user on +// ch: Checker-resolved bits plus DM state read straight from the tables. +func (f *voiceParityFixture) joinWant(t *testing.T, ch *db.Channel, blocked bool) error { + t.Helper() + sub := paritySubject(t, f.database, f.user.ID, ch) + if ch.Type == "dm" { + sub.DMParticipant = true + sub.DMBlocked = blocked + } + return permissions.CanJoinVoice(sub) +} + +// TestChannelSubject_Parity: the shared resolver every voice site feeds the +// predicates agrees with an independent resolution in both branches, +// including DM membership and block state. +func TestChannelSubject_Parity(t *testing.T) { + f := newVoiceParityFixture(t) + ctx := context.Background() + for _, chID := range []int64{f.voiceID, f.oldVoiceID, f.dmID} { + ch := f.channel(t, chID) + for _, blocked := range []bool{false, true} { + f.setBlocked(t, blocked) + for _, c := range parityOverrideCases { + setParityOverrides(t, f.database, f.permSvc, chID, harvestVoiceRoleID, f.user.ID, c) + want := paritySubject(t, f.database, f.user.ID, ch) + if ch.Type == "dm" { + want.DMParticipant = true + want.DMBlocked = blocked + } + f.eachBranch(func(branch string) { + got, err := channelSubject(ctx, f.database, f.h.permChecker, f.h.perms, f.user.ID, ch, true) + if err != nil { + t.Fatalf("chan=%d/%s/%s: channelSubject: %v", chID, c.name, branch, err) + } + if got != want { + t.Errorf("chan=%d/blocked=%v/%s/%s: channelSubject = %+v, want %+v", chID, blocked, c.name, branch, got, want) + } + }) + } + } + } +} + +// TestVoiceJoinPrecheck_Parity: the voice_join gate refuses exactly when +// CanJoinVoice refuses, with the frame joinDenial maps that reason to; when +// the predicate allows, the only refusal left is the fixture having no +// LiveKit (VOICE_ERROR), which proves the gate was passed. +func TestVoiceJoinPrecheck_Parity(t *testing.T) { + f := newVoiceParityFixture(t) + f.h.limiter = nil // the table would trip the per-user join limit + ctx := context.Background() + for _, chID := range []int64{f.voiceID, f.oldVoiceID, f.textID, f.dmID} { + ch := f.channel(t, chID) + for _, blocked := range []bool{false, true} { + f.setBlocked(t, blocked) + for _, c := range parityOverrideCases { + setParityOverrides(t, f.database, f.permSvc, chID, harvestVoiceRoleID, f.user.ID, c) + want := f.joinWant(t, ch, blocked) + f.eachBranch(func(branch string) { + for len(f.send) > 0 { + <-f.send + } + _, _, ok := f.h.voiceJoinPrecheck(ctx, f.client, json.RawMessage(fmt.Sprintf(`{"channel_id":%d}`, chID))) + if ok { + t.Fatalf("chan=%d/%s/%s: precheck passed with no LiveKit configured", chID, c.name, branch) + } + var env struct { + Payload struct { + Code string `json:"code"` + } `json:"payload"` + } + select { + case raw := <-f.send: + if err := json.Unmarshal(raw, &env); err != nil { + t.Fatalf("unmarshal: %v", err) + } + case <-time.After(2 * time.Second): + t.Fatalf("chan=%d/%s/%s: no error frame", chID, c.name, branch) + } + wantCode := ErrCodeVoiceError // allowed: refused only by the missing LiveKit + if want != nil { + wantCode = joinDenial(want).Code + } + if env.Payload.Code != wantCode { + t.Errorf("chan=%d/blocked=%v/%s/%s: frame %s, CanJoinVoice says %v (want %s)", chID, blocked, c.name, branch, env.Payload.Code, want, wantCode) + } + }) + } + } + } +} + +// TestVoiceStillAllowed_Parity: the sweep's re-check is CanJoinVoice over the +// live subject, so it evicts exactly what the join gate would now refuse. +func TestVoiceStillAllowed_Parity(t *testing.T) { + f := newVoiceParityFixture(t) + ctx := context.Background() + for _, chID := range []int64{f.voiceID, f.oldVoiceID, f.dmID} { + ch := f.channel(t, chID) + for _, blocked := range []bool{false, true} { + f.setBlocked(t, blocked) + for _, c := range parityOverrideCases { + setParityOverrides(t, f.database, f.permSvc, chID, harvestVoiceRoleID, f.user.ID, c) + want := f.joinWant(t, ch, blocked) == nil + got, err := f.h.voiceStillAllowed(ctx, f.user.ID, chID) + if err != nil { + t.Fatalf("chan=%d/%s: voiceStillAllowed: %v", chID, c.name, err) + } + if got != want { + t.Errorf("chan=%d/blocked=%v/%s: voiceStillAllowed = %v, CanJoinVoice = %v", chID, blocked, c.name, got, want) + } + } + } + } + if got, err := f.h.voiceStillAllowed(ctx, f.user.ID, 999999); err != nil || got { + t.Errorf("deleted channel: allowed=%v err=%v, want a refusal with no error", got, err) + } +} + +// TestRefreshChannelVisibility_Parity: the fan-out sends channel_create +// exactly when CanViewChannel allows and channel_delete otherwise. +func TestRefreshChannelVisibility_Parity(t *testing.T) { + f := newViewParityFixture(t) + for _, chID := range []int64{f.textID, f.oldID} { + ch := f.channel(t, chID) + for _, c := range parityOverrideCases { + setParityOverrides(t, f.database, f.permSvc, chID, harvestVoiceRoleID, f.user.ID, c) + want := MsgTypeChannelDelete + if permissions.CanViewChannel(paritySubject(t, f.database, f.user.ID, ch)) == nil { + want = MsgTypeChannelCreate + } + f.eachBranch(func(branch string) { + f.h.RefreshChannelVisibility(ch) + var env struct { + Type string `json:"type"` + } + select { + case raw := <-f.send: + if err := json.Unmarshal(raw, &env); err != nil { + t.Fatalf("unmarshal: %v", err) + } + case <-time.After(2 * time.Second): + t.Fatalf("chan=%d/%s/%s: no frame from RefreshChannelVisibility", chID, c.name, branch) + } + if env.Type != want { + t.Errorf("chan=%d/%s/%s: got %s, CanViewChannel says %s", chID, c.name, branch, env.Type, want) + } + }) + } + } +} diff --git a/Server/ws/serve_ready.go b/Server/ws/serve_ready.go index 5a676cff..53a56367 100644 --- a/Server/ws/serve_ready.go +++ b/Server/ws/serve_ready.go @@ -125,38 +125,35 @@ func channelRefs(channels []db.Channel) []permissions.ChannelRef { func permOverrides(overrides map[int64]db.ChannelOverride) map[int64]permissions.ChannelOverride { out := make(map[int64]permissions.ChannelOverride, len(overrides)) for id, o := range overrides { - out[id] = permissions.ChannelOverride{ - Allow: o.Allow, - Deny: o.Deny, - UserAllow: o.UserAllow, - UserDeny: o.UserDeny, - } + out[id] = permOverride(o) } return out } // channelCanSend reports whether a user with the given role and per-channel -// override may post in a channel of chanType. It mirrors the non-DM branch of -// MessageService.checkSendPermission so the client can pre-disable the composer -// without a round-trip; the server still enforces the rule authoritatively. +// override may post in a channel of chanType — the ready payload's can_send +// affordance, so the client can pre-disable the composer without a +// round-trip. It is permissions.CanSendMessage, the same predicate the send +// path enforces, so the affordance cannot drift from the rule (S-12). func channelCanSend(role *db.Role, o db.ChannelOverride, chanType string) bool { if role == nil { return false } - if permissions.HasAdmin(role.Permissions) { - return true - } - eff := permissions.EffectiveChannelPerms(role.Permissions, permissions.ChannelOverride{ - Allow: o.Allow, Deny: o.Deny, UserAllow: o.UserAllow, UserDeny: o.UserDeny, - }) - need := permissions.ReadMessages | permissions.SendMessages - if eff&need != need { - return false - } - if chanType == "announcement" { - return eff&permissions.ManageMessages == permissions.ManageMessages - } - return true + return permissions.CanSendMessage(permissions.Subject{ + RolePerms: role.Permissions, + Override: permOverride(o), + Channel: permissions.ChannelRef{Type: chanType}, + }) == nil +} + +// channelRef maps one db channel to the predicates' db-agnostic ChannelRef. +func channelRef(ch *db.Channel) permissions.ChannelRef { + return permissions.ChannelRef{ID: ch.ID, Type: ch.Type, Archived: ch.Archived} +} + +// permOverride maps one db override (both layers) to the checker's type. +func permOverride(o db.ChannelOverride) permissions.ChannelOverride { + return permissions.ChannelOverride{Allow: o.Allow, Deny: o.Deny, UserAllow: o.UserAllow, UserDeny: o.UserDeny} } // readyVisibleChannels resolves the channels the user may see for the ready diff --git a/Server/ws/voice_join.go b/Server/ws/voice_join.go index 262e7873..5f8114f9 100644 --- a/Server/ws/voice_join.go +++ b/Server/ws/voice_join.go @@ -10,7 +10,6 @@ import ( "github.com/J3vb/OwnCord/Server/auth" "github.com/J3vb/OwnCord/Server/db" "github.com/J3vb/OwnCord/Server/permissions" - "github.com/J3vb/OwnCord/Server/service" ) // Voice join/leave rate limits. voice_join and voice_leave each fan out a @@ -99,13 +98,6 @@ func (h *Hub) voiceJoinPrecheck(ctx context.Context, c *Client, payload json.Raw return 0, nil, false } - // 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 0, nil, false - } - // Validate the target channel exists before any state changes (leaving // the current voice channel, persisting join, etc.). ch, err := h.db.GetChannel(ctx, channelID) @@ -114,39 +106,30 @@ func (h *Hub) voiceJoinPrecheck(ctx context.Context, c *Client, payload json.Raw return 0, nil, false } - // channel_id is attacker-controlled and requireChannelAccess above only - // gates CONNECT_VOICE, which says nothing about channel type — a text or - // announcement channel would otherwise accept a join, persist a - // voice_states row, mint a LiveKit room and broadcast voice_state for a - // channel the UI can never render or moderate. 'dm' stays allowed: DM and - // group voice calls join through this same handler. - if ch.Type != "voice" && ch.Type != "dm" { - c.sendMsg(buildErrorMsg(ErrCodeBadRequest, "not a voice channel")) + // channel_id is attacker-controlled, so the gate is + // permissions.CanJoinVoice over the channel-TYPE-aware subject: the + // CONNECT_VOICE bit (a role-only check passes for any DM id — DMs have no + // overrides — and the token minted below carries RoomJoin+CanSubscribe + // for that room), a channel that has a room (a text channel would + // otherwise persist a voice_states row and mint a LiveKit room the UI can + // never render or moderate; DM and group calls join through this same + // handler), no archive (a caller still holding the id of a channel nobody + // can see must not join its room; the archive transition also evicts + // whoever is inside), and for a DM membership plus no block (blocking + // never touches dm_participants, so membership alone would let a blocked + // user into the blocker's call — same rule as every other DM sink, + // service.requireDMNotBlocked, group DMs exempt). The same predicate + // gates the token refresh and a moderator move's destination. + sub, subErr := channelSubject(ctx, h.db, h.permChecker, h.perms, c.userID, ch, true) + if subErr != nil { + slog.Error("ws voice_join: permission lookup failed, denying", "user_id", c.userID, "channel_id", channelID, "err", subErr) + c.sendMsg(buildErrorMsg(ErrCodeInternal, "permission check failed")) return 0, nil, false } - - // A blocked user is still a DM participant — blocking never touches - // dm_participants (service/block.go), so the CONNECT_VOICE + IsDMParticipant - // gate above passes them straight through into the blocker's DM voice room. - // Every other 1:1-DM interaction sink (send, edit, react, pin, typing, - // call_ring) already routes through this same check - // (service.requireDMNotBlocked); voice was the one gap. Group DMs are - // exempt inside it, matching every other sink. h.db satisfies - // service.Store directly, so no MessageService wiring is needed here. - if ch.Type == "dm" { - if err := service.RequireDMNotBlocked(ctx, h.db, c.userID, channelID); err != nil { - c.sendMsg(buildErrorMsg(ErrCodeForbidden, "cannot join voice: blocked")) - return 0, nil, false - } - } - - // Archived channels are hidden from every client and their voice states are - // dropped from `ready`, but `archived` was consulted only by the visibility - // predicate — so a caller still holding the id could join the room of a - // channel nobody can see or moderate. Refuse the join outright; the sibling - // archive transition also evicts whoever is already inside. - if ch.Archived { - c.sendMsg(buildErrorMsg(ErrCodeBadRequest, "channel is archived")) + if joinErr := permissions.CanJoinVoice(sub); joinErr != nil { + slog.Warn("ws voice_join refused", "user_id", c.userID, "channel_id", channelID, "reason", joinErr) + refusal := joinDenial(joinErr) + c.sendMsg(buildErrorMsg(refusal.Code, refusal.Message)) return 0, nil, false } @@ -581,35 +564,33 @@ func handleVoiceTokenRefreshV2(ctx context.Context, cmd Command, info ClientInfo return Result{Error: ClientError{Code: ErrCodeInternal, Message: "voice not configured"}} } - // Re-check CONNECT_VOICE where the credential is minted. The channel comes - // from the client's own session state, and voice_join (voice_join.go:61) was - // the only place this bit was ever checked — so a user whose CONNECT_VOICE - // was revoked mid-session kept minting fresh SFU room-join grants. Refusing + // Re-run the join gate (permissions.CanJoinVoice, exactly as voice_join + // applies it) where the credential is minted. The channel comes from the + // client's own session state, and voice_join used to be the only place + // the bit was checked — so a user whose CONNECT_VOICE was revoked + // mid-session kept minting fresh SFU room-join grants, and a block imposed + // mid-session (OC-0018) kept re-issuing one for the blocker's DM. Refusing // 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. - // 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, d.PermSvc, userID, channelID, permissions.ConnectVoice) { + // deletes the voice_states row and removes the LiveKit participant. Fails + // closed: a deleted channel or a lookup failure is a refusal too. + ch, chErr := d.DB.GetChannel(ctx, channelID) + if chErr != nil || ch == nil { return Result{ Error: ClientError{Code: ErrCodeForbidden, Message: "missing CONNECT_VOICE permission"}, LeaveVoice: true, } } - - // Same block gate as voice_join (voice_join.go, OC-0018): a block imposed - // mid-session must not let the refresh keep minting a fresh SFU credential - // for a DM the other participant has since blocked. RequireDMNotBlocked is - // a safe no-op for a non-DM channelID (no dm_participants row to match), so - // this needs no channel-type fetch of its own. d.DB satisfies service.Store - // directly. - if err := service.RequireDMNotBlocked(ctx, d.DB, userID, channelID); err != nil { + sub, subErr := channelSubject(ctx, d.DB, d.Permissions, d.PermSvc, userID, ch, true) + if subErr != nil { return Result{ - Error: ClientError{Code: ErrCodeForbidden, Message: "cannot refresh voice token: blocked"}, + Error: ClientError{Code: ErrCodeForbidden, Message: "missing CONNECT_VOICE permission"}, LeaveVoice: true, } } + if joinErr := permissions.CanJoinVoice(sub); joinErr != nil { + return Result{Error: joinDenial(joinErr), LeaveVoice: true} + } // With a PermissionService these three are cache hits after the gate above // populated the user's entry — the refresh drops from ~9 DB reads to at diff --git a/Server/ws/voice_moderation.go b/Server/ws/voice_moderation.go index a145108a..312ce55c 100644 --- a/Server/ws/voice_moderation.go +++ b/Server/ws/voice_moderation.go @@ -2,6 +2,7 @@ package ws import ( "context" + "errors" "fmt" "log/slog" @@ -85,26 +86,36 @@ func voiceModTarget(ctx context.Context, d VoiceDeps, actorID, targetID int64) ( return nil, &Result{Error: ClientError{Code: ErrCodeVoiceError, Message: "user is not in a voice channel"}} } - // MUTE_MEMBERS authorizes moderating server voice channels, not a private - // DM call the actor happens not to be part of — voice_mod_kick and friends - // carry no channel id from the client, so without this a moderator could - // reach into any two users' DM call by targeting a user id alone. Refused - // with the exact same shape as "target not in voice" so the actor learns - // nothing about a DM call they are not in. + // The decision is permissions.CanModerateVoice over the actor's subject in + // the TARGET's channel: effective MUTE_MEMBERS there, so a role-layer or + // user-layer deny on that channel holds (SEC-02), READ_MESSAGES so a room + // hidden from the actor cannot be moderated, and for a DM call the actor's + // own membership — voice_mod_kick and friends carry no channel id from + // the client, so without that a moderator could reach into any two users' + // DM call by targeting a user id alone. The DM refusal keeps the exact + // shape of "target not in voice" so the actor learns nothing about a call + // they are not in. The base-bit check above is only an early rejection + // (it never admits): it keeps FORBIDDEN ahead of the voice-state lookup + // for actors with no MUTE_MEMBERS at all, which also means a channel + // allow cannot grant the bit to a role whose base lacks it. ch, err := d.DB.GetChannel(ctx, state.ChannelID) if err != nil { slog.Error("ws voiceModTarget GetChannel", "err", err, "channel_id", state.ChannelID) return nil, &Result{Error: ClientError{Code: ErrCodeInternal, Message: "failed to read channel"}} } - if ch != nil && ch.Type == "dm" { - participant, err := d.DB.IsDMParticipant(ctx, actorID, state.ChannelID) - if err != nil { - slog.Error("ws voiceModTarget IsDMParticipant", "err", err, "channel_id", state.ChannelID) - return nil, &Result{Error: ClientError{Code: ErrCodeInternal, Message: "failed to verify DM membership"}} - } - if !participant { - return nil, &Result{Error: ClientError{Code: ErrCodeVoiceError, Message: "user is not in a voice channel"}} - } + if ch == nil { + return nil, &Result{Error: ClientError{Code: ErrCodeVoiceError, Message: "user is not in a voice channel"}} + } + sub, subErr := channelSubject(ctx, d.DB, d.Permissions, d.PermSvc, actorID, ch, false) + if subErr != nil { + slog.Error("ws voiceModTarget channelSubject", "err", subErr, "channel_id", state.ChannelID) + return nil, &Result{Error: ClientError{Code: ErrCodeInternal, Message: "failed to verify channel access"}} + } + switch modErr := permissions.CanModerateVoice(sub); { + case errors.Is(modErr, permissions.ErrNotDMParticipant): + return nil, &Result{Error: ClientError{Code: ErrCodeVoiceError, Message: "user is not in a voice channel"}} + case modErr != nil: + return nil, &Result{Error: ClientError{Code: ErrCodeForbidden, Message: "missing MUTE_MEMBERS permission"}} } return state, nil @@ -416,16 +427,20 @@ func handleVoiceModMoveV2(ctx context.Context, cmd Command, info ClientInfo, dep if dest.Type != "voice" { return Result{Error: ClientError{Code: ErrCodeBadRequest, Message: "destination is not a voice channel"}} } - // The re-join this move hands off to (handleVoiceJoin) refuses an - // archived channel outright; check it here too, or the pre-flight commits - // the destructive half of the move for a re-join guaranteed to bounce. - if dest.Archived { - return Result{Error: ClientError{Code: ErrCodeBadRequest, Message: "channel is archived"}} + // The destination is gated on the TARGET's access with the same predicate + // the re-join this move hands off to (handleVoiceJoin) will apply — + // permissions.CanJoinVoice — so a move can neither place someone in a + // channel they could not join themselves nor commit the destructive half + // of the move for a re-join guaranteed to bounce (an archived channel). + targetSub, subErr := channelSubject(ctx, d.DB, d.Permissions, d.PermSvc, c.TargetID(), dest, false) + if subErr != nil { + slog.Error("ws handleVoiceModMoveV2 channelSubject", "err", subErr, "channel_id", c.ToChannelID()) + return Result{Error: ClientError{Code: ErrCodeInternal, Message: "failed to check destination access"}} } - // The destination is gated on the TARGET's access, not the moderator's: - // a move must not become a way to place someone in a channel they could - // not join themselves. - if !hasChannelAccess(ctx, d.DB, d.Permissions, d.PermSvc, c.TargetID(), c.ToChannelID(), permissions.ConnectVoice) { + switch joinErr := permissions.CanJoinVoice(targetSub); { + case errors.Is(joinErr, permissions.ErrArchived): + return Result{Error: ClientError{Code: ErrCodeBadRequest, Message: "channel is archived"}} + case joinErr != nil: return Result{Error: ClientError{ Code: ErrCodeForbidden, Message: "user cannot connect to that voice channel", diff --git a/Server/ws/voice_moderation_deafen_race_test.go b/Server/ws/voice_moderation_deafen_race_test.go index d4d1a09b..a54d5c83 100644 --- a/Server/ws/voice_moderation_deafen_race_test.go +++ b/Server/ws/voice_moderation_deafen_race_test.go @@ -23,6 +23,7 @@ import ( "testing" "github.com/J3vb/OwnCord/Server/db" + "github.com/J3vb/OwnCord/Server/permissions" ) // deafenRaceRoleAdmin / deafenRaceRoleMember reuse the default seeded roles @@ -98,7 +99,7 @@ func TestVoiceModDeafen_RollbackFollowsTargetChannelMove(t *testing.T) { cmd := VoiceModDeafenCmd{userID: actorID, channelID: chanA, targetID: targetID, deafened: true} info := ClientInfo{UserID: actorID} - deps := VoiceDeps{DB: database} + deps := VoiceDeps{DB: database, Permissions: permissions.NewChecker(database)} result := handleVoiceModDeafenV2(ctx, cmd, info, deps) @@ -181,7 +182,7 @@ func TestVoiceModDeafen_UndeafenRollbackDoesNotApplyOnUnauthorizedChannel(t *tes // test above. cmd := VoiceModDeafenCmd{userID: actorID, channelID: chanA, targetID: targetID, deafened: false} info := ClientInfo{UserID: actorID} - deps := VoiceDeps{DB: database} + deps := VoiceDeps{DB: database, Permissions: permissions.NewChecker(database)} result := handleVoiceModDeafenV2(ctx, cmd, info, deps) diff --git a/Server/ws/voice_moderation_overrides_test.go b/Server/ws/voice_moderation_overrides_test.go new file mode 100644 index 00000000..4ea8c3ab --- /dev/null +++ b/Server/ws/voice_moderation_overrides_test.go @@ -0,0 +1,72 @@ +package ws_test + +import ( + "context" + "testing" + + "github.com/J3vb/OwnCord/Server/permissions" + "github.com/J3vb/OwnCord/Server/ws" +) + +// TestVoiceMod_ChannelOverridesApply locks SEC-02's server half: the actor's +// authority is their EFFECTIVE permission in the target's channel +// (permissions.CanModerateVoice), so a role-layer or user-layer deny of +// MUTE_MEMBERS on that channel refuses the action even though the base role +// holds the bit, and a channel the actor cannot see (READ_MESSAGES denied) +// cannot be moderated either. Administrator keeps its bypass. +func TestVoiceMod_ChannelOverridesApply(t *testing.T) { + cases := []struct { + name string + actorRole int // 2 Admin: MUTE_MEMBERS without ADMINISTRATOR; 1 Owner: ADMINISTRATOR + roleDeny int64 + userDeny int64 + wantCode string // "" = allowed (target ends up server-muted) + }{ + {"no override: allowed", 2, 0, 0, ""}, + {"role deny MUTE_MEMBERS in this channel", 2, permissions.MuteMembers, 0, "FORBIDDEN"}, + {"user deny MUTE_MEMBERS in this channel", 2, 0, permissions.MuteMembers, "FORBIDDEN"}, + {"role deny READ_MESSAGES: hidden channel", 2, permissions.ReadMessages, 0, "FORBIDDEN"}, + {"administrator bypasses the deny", 1, permissions.MuteMembers, permissions.MuteMembers, ""}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + hub, database := newVoiceModHub(t) + chanID := seedVoiceChan(t, database, "vc-override") + actor := seedVoiceUserWithRole(t, database, "mod-override", tc.actorRole) + target := seedVoiceUserWithRole(t, database, "target-override", 4) // Member + ctx := context.Background() + if err := database.UpsertChannelOverride(ctx, chanID, int64(tc.actorRole), 0, tc.roleDeny); err != nil { + t.Fatalf("UpsertChannelOverride: %v", err) + } + if err := database.UpsertChannelUserOverride(ctx, chanID, actor.ID, 0, tc.userDeny); err != nil { + t.Fatalf("UpsertChannelUserOverride: %v", err) + } + + joinVoice(t, hub, target, chanID) + + send := make(chan []byte, 16) + c := ws.NewTestClientWithUser(hub, actor, chanID, send) + hub.Register(c) + waitRegistered(t, hub, c) + + hub.HandleMessageForTest(c, voiceModMuteMsg(chanID, target.ID, true)) + + state, err := database.GetVoiceState(ctx, target.ID) + if err != nil || state == nil { + t.Fatalf("GetVoiceState: state=%v err=%v", state, err) + } + if tc.wantCode == "" { + if !state.ServerMuted { + t.Fatal("expected the target to be server muted") + } + return + } + if code := receiveErrorCode(send, waitTimeout); code != tc.wantCode { + t.Fatalf("error code = %q, want %s", code, tc.wantCode) + } + if state.ServerMuted { + t.Fatal("target must not be server muted after a refused action") + } + }) + } +} diff --git a/docs/plans/b2-protocol-trust-compat-2026-08-28.md b/docs/plans/b2-protocol-trust-compat-2026-08-28.md index 60eca8df..36ae88cd 100644 --- a/docs/plans/b2-protocol-trust-compat-2026-08-28.md +++ b/docs/plans/b2-protocol-trust-compat-2026-08-28.md @@ -5,7 +5,7 @@ `v1.2.0-alpha.4` — claims verified at `64d2e108`; the branch was rebased onto `dd7ed091` (#1432) before merge **Status:** in progress — entry gate 1 of 3 met at draft time (see below); B2-0, -B2-1 and B2-8 landed 2026-08-28, B2-2 (with B2-3 and B2-4 folded in) on 2026-08-29 (evidence in their sections); B2-5 is next. +B2-1 and B2-8 landed 2026-08-28, B2-2 (with B2-3 and B2-4 folded in) and B2-5 on 2026-08-29 (evidence in their sections); B2-6 and B2-7 are next. Update this line, not only the step table, when a step lands. Primary inputs: @@ -41,7 +41,7 @@ one to one and a half weeks with agents working steps in parallel. | **B2-2** | Protocol epoch and negotiation — **DONE 2026-08-29 (slim; absorbs B2-3, B2-4)** | 1 day | serialized, after B2-8 | | **B2-3** | Server-first updates through the signed manifest — folded into B2-2 | ½ day | after B2-2 | | **B2-4** | Compatibility matrix — folded into B2-2 | ½ day | after B2-2 | -| **B2-5** | One permission predicate per security property | 1–2 days | serialized | +| **B2-5** | One permission predicate per security property — **DONE 2026-08-29 (PR #1440)** | 1–2 days | serialized | | **B2-6** | Safe audit coverage | ½ day | B2-1, B2-7 | | **B2-7** | Trust model, absence proofs, plugin boundary | 1 day | B2-1, B2-6 | | **B2-8** | The nine B2-tagged findings | 1 day | before B2-2 | @@ -339,6 +339,100 @@ half stands on those two tests. one. If residual calls remain with a reason, record them in HP-2 and leave the rule to B3 (roadmap B3 item 15). +**Evidence, 2026-08-29** — branch `feat/b2-5-permission-predicates` from +`dev` `9c9b8be6`; PR #1440 to `dev`. HP-2 question 5 cites this block. + +- Pre-squash SHAs, one commit per property: `00761523` (predicates + + `Checker` delegation), `94aba833` (send — S-01), `0271cbbe` (view / + session admission — S-12), `802101a0` (voice join), `aeee37e8` (voice + moderation — SEC-02 server half). +- Predicates (`Server/permissions/predicates.go`), each pure over a + `Subject` (role bits, both override layers, channel flags, DM membership + and block state): `CanViewChannel`, `CanAdmitSession` (= view), + `CanSendMessage`, `CanType` (= send), `CanJoinVoice`, `CanModerateVoice`; + `Subject.Has` is the one value-taking bit predicate `Checker` and + `PermissionService` route through. Refusals are sentinels + (`ErrPermissionDenied` + bit name, `ErrArchived`, `ErrBlocked`, + `ErrNotDMParticipant`, `ErrNotVoiceChannel`) so each site keeps its own + status codes. Permission is checked before the archive flag everywhere, so + an unauthorized caller learns nothing from the error. +- Parity tables (site vs predicate over the same fixture, both the + cached-service and bare-hub branch, every override layer): + `Server/service/predicate_parity_test.go` (`CanPost`, `HandleTyping`, + `HandleChannelFocus`) and `Server/ws/predicate_parity_internal_test.go` + (`channelCanSend`, `refreshChannelVisibilityCanSend`, `applySetChannelID`, + `channelReadAudience`, `RefreshChannelVisibility`, `channelSubject`, + `voiceJoinPrecheck`, `voiceStillAllowed`). Red before delegation: S-01 (19 + typing rows) and SEC-02 (`TestVoiceMod_ChannelOverridesApply`, three deny + rows); every other site already agreed with its predicate. +- Decision recorded for SEC-02's open question ("READ_MESSAGES or + CONNECT_VOICE?"): `CanModerateVoice` requires effective `READ_MESSAGES` + + `MUTE_MEMBERS` in the target's channel — a moderator acts only where they + can see. The base-bit `HasServerPerm` check stays as an early rejection + (never admits), which keeps FORBIDDEN ahead of the voice-state lookup and + means a channel allow cannot grant `MUTE_MEMBERS` to a base role lacking it. +- Inventory, step 1 grep plus the hand-rolled sites, before → after: + + | Site (before) | Property | After | + | ------------------------------------------------------------------- | --------------- | --------------------------------------------- | + | `permissions/checker.go` HasChannelPerm / Batch / VisibleChannelIDs | view (bit) | `Subject.Has` / `CanViewChannel` | + | `service/message_perms.go:93-100` checkSendPermission | send | `CanSendMessage` | + | `service/channel.go:132` HandleTyping (READ only — S-01) | type | `CanType` | + | `service/channel.go:256` HandleChannelFocus | admit | `CanAdmitSession` | + | `ws/serve_ready.go:149-157` channelCanSend | send | `CanSendMessage` | + | `ws/hub_broadcast.go:519-523` refreshChannelVisibilityCanSend | send | `CanSendMessage` | + | `ws/hub_broadcast.go:265,283` channelReadAudience | view | `CanViewChannel` | + | `ws/hub_broadcast.go:422-439` RefreshChannelVisibility | view | `CanViewChannel` | + | `ws/handlers.go:296` applySetChannelID (hasPermChecked) | admit | `CanAdmitSession`; helper deleted | + | `ws/voice_join.go:105-151` voiceJoinPrecheck (requireChannelAccess) | join | `CanJoinVoice`; helper deleted | + | `ws/voice_join.go:594-612` handleVoiceTokenRefreshV2 | join | `CanJoinVoice` | + | `ws/voice_moderation.go:416-433` move destination | join | `CanJoinVoice` | + | `ws/hub_sweep.go:353` hasChannelPermChecked (EffectiveChannelPerms) | join (bit only) | `CanJoinVoice` (whole rule, error-aware) | + | `ws/voice_moderation.go:64` voiceModTarget (HasServerPerm — SEC-02) | moderate | `CanModerateVoice` + base-bit early rejection | + | `ws/deps.go` hasChannelAccess / hasChannelAccessLive | join/admit glue | deleted (`channelSubject` + predicates) | + +- Residue after migration (direct bit-helper calls outside + `Server/permissions`, non-test), each with its reason — so step 5's + condition is not met and the `authz-chokepoint` rule stays with B3 item + 15, consistent with the 2026-08-18 measurement that dropped it (1 hit in + `api/`, a false positive; 30 widened, 87% legitimate): + - Server-scoped permissions with no channel — `HasServerPerm` in + `api/middleware.go:200`, `admin/middleware.go:109`, `service/emoji.go:95`, + `service/moderation.go:51`, `service/role.go:82`, and `HasAnyPerm` + (`AdminPerimeter`) in `admin/middleware.go:84`. These ARE the canonical + server-wide predicate; there is no channel to resolve a `Subject` for. + - `HasAdmin` as a fetch short-circuit (skip the override query for admins) + in `service/channel.go:59`, `service/message_perms.go:25`, + `service/permission.go:224`, `ws/serve.go:780`, `ws/serve_ready.go:169`, + `ws/voice_join.go:355`; as an authorization input in + `admin/handlers_channel_perms.go:95,325`, `admin/logstream.go:452`, + `api/upload_handler.go:404`, `service/role.go:104` (role hierarchy — the + measurement's "no `Outranks`" class). + - `& permissions.AllPerms` masks on admin input (`admin/handlers_channel_perms.go:131-358`, + `service/role.go:210,307`) — sanitisation, not a decision. + - `service/mentions.go:262-266,302-304` — the bulk @everyone reader walk + resolves the role layer per role and the user layer as a set difference; + the owner declined the mechanical `HasPerm` conversion on 2026-08-18 + (memory `owncord-invariant-rule-measurement-2026-08-18`). + - `ws/voice_moderation.go:65` — the base-bit early rejection described + above. +- Behaviour deltas beyond the three findings, all narrowing: the stale-voice + sweep re-runs the whole join rule (deleted/archived channel, lost DM + membership, new block evict too); the token refresh refuses a deleted + channel; the bare-hub `RefreshChannelVisibility` branch fails closed on a + lookup error like the service branch always did. Two fixtures needed + completing, assertions untouched: the deafen-race `VoiceDeps` gain a + `Checker`, and `TestHandleVoiceTokenRefresh_NilUser` seeds the channel it + refreshes. +- Gates at `aeee37e8`, from `Server/`: four build-tag variants, `go vet`, + `go test -race ./...`, `go test -tags deadlock ./ws/`, `golangci-lint run` + — all exit 0, run before each of the five commits. +- Codex review on #1440 (P2): `CanJoinVoice`'s DM branch returned before the + archive flag, while the old `voiceJoinPrecheck` refused every archived + channel and the admin PATCH accepts `archived` for a DM. Fixed in + `fdd2a3ff` (archive checked after membership and block for both kinds, + pinned in the predicate table), same gate green; thread resolved. + ## B2-6 — Safe audit coverage 1. Enumerate the security-sensitive mutations: credential and TOTP changes, @@ -465,15 +559,15 @@ The seven local reports in `docs/security-findings/` (gitignored, never committed; the directory-to-row mapping lives in its local README) and where each goes: -| Public row | Owner phase | Acceptance test lives | Lands with | -| ---------- | ------------------------------- | ---------------------------------------------------------------------------------------- | --------------------------------- | -| S-01 | **B2** | beside the report until B2-5 merges | B2-5 | -| SEC-02 | **B2** (server half) | beside the report until B2-5 merges | B2-5; UI half in B5 | -| C-09 | **B2** (contract) / B7 (client) | beside the report | contract in B2-7 docs; code in B7 | -| SEC-03 | B2 if small, else **B5** | beside the report | B2-9 or B5 item 11 | -| SEC-01 | **B4** | private GitHub advisory (owner creates it) | B4 | -| SEC-04 | **B3/B6** | private GitHub advisory (owner creates it) | B6 | -| OC-0324 | **B4** | beside the report; no advisory — the tracked ledger already carries this finding in full | B4 | +| Public row | Owner phase | Acceptance test lives | Lands with | +| ---------- | ------------------------------- | ---------------------------------------------------------------------------------------- | ------------------------------------- | +| S-01 | **B2** | landed with B2-5 (`Server/service/predicate_parity_test.go`) | B2-5 (PR #1440) — done | +| SEC-02 | **B2** (server half) | landed with B2-5 (`Server/ws/voice_moderation_overrides_test.go`) | B2-5 (PR #1440) — done; UI half in B5 | +| C-09 | **B2** (contract) / B7 (client) | beside the report | contract in B2-7 docs; code in B7 | +| SEC-03 | B2 if small, else **B5** | beside the report | B2-9 or B5 item 11 | +| SEC-01 | **B4** | private GitHub advisory (owner creates it) | B4 | +| SEC-04 | **B3/B6** | private GitHub advisory (owner creates it) | B6 | +| OC-0324 | **B4** | beside the report; no advisory — the tracked ledger already carries this finding in full | B4 | An acceptance test demonstrates the defect, so it is exploit detail: it stays local until its fix lands, then lands publicly in the same PR. The two diff --git a/docs/plans/repo-health-issue-register-2026-08-23.md b/docs/plans/repo-health-issue-register-2026-08-23.md index acb9a90d..040cb2ce 100644 --- a/docs/plans/repo-health-issue-register-2026-08-23.md +++ b/docs/plans/repo-health-issue-register-2026-08-23.md @@ -149,12 +149,12 @@ This register carries only non-sensitive security properties and opaque remediation families; an apparently related engineering row is not evidence that any private report is fixed. -| ID | Pri | State | Opaque remediation family | Phase | Public closure evidence | -| ------ | --: | --------- | --------------------------------------------------------- | ----- | ------------------------------------------------------------------------------------------------------------------------------------------------- | -| SEC-01 | P1 | confirmed | Atomic concurrent password-confirmation admission. | B4 | One server-owned admission decision, bounded concurrent attempts, and race/load regression coverage. | -| SEC-02 | P1 | confirmed | Effective channel-level voice moderation permissions. | B5 | Voice moderation delegates to the same effective-permission policy as the authoritative channel action, with override and denial tests. | -| SEC-03 | P1 | confirmed | Bounded per-response and aggregate preview/media reads. | B2/B5 | Streaming limits are enforced before buffering; aggregate memory/concurrency budgets, timeout, cancellation, and adversarial boundary tests pass. | -| SEC-04 | P1 | confirmed | Durable per-user/server storage quotas and disk headroom. | B3/B6 | Transaction-safe quotas cover files and cumulative storage; low-disk behavior fails safely and is exercised by restart/concurrency tests. | +| ID | Pri | State | Opaque remediation family | Phase | Public closure evidence | +| ------ | --: | ------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----- | ------------------------------------------------------------------------------------------------------------------------------------------------- | +| SEC-01 | P1 | confirmed | Atomic concurrent password-confirmation admission. | B4 | One server-owned admission decision, bounded concurrent attempts, and race/load regression coverage. | +| SEC-02 | P1 | resolved/superseded | Effective channel-level voice moderation permissions. Server half landed in B2-5 (PR #1440): voice moderation decides on the effective permission in the target channel, with override and denial tests. | B5 | UI half only: the moderation controls surface effective permissions (B5 item 11). | +| SEC-03 | P1 | confirmed | Bounded per-response and aggregate preview/media reads. | B2/B5 | Streaming limits are enforced before buffering; aggregate memory/concurrency budgets, timeout, cancellation, and adversarial boundary tests pass. | +| SEC-04 | P1 | confirmed | Durable per-user/server storage quotas and disk headroom. | B3/B6 | Transaction-safe quotas cover files and cumulative storage; low-disk behavior fails safely and is exercised by restart/concurrency tests. | ## Client engineering issues @@ -184,25 +184,25 @@ not recounted here. ## Server engineering issues -| ID | Pri | State | Issue and evidence | Phase | Closure evidence | -| ---- | --: | --------- | ----------------------------------------------------------------------------------------------------------------------------------- | ------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| S-01 | P1 | confirmed | Typing currently checks a weaker permission than posting. | B2/B3 | Typing delegates to the same send-policy predicate; denial, announcement, and override tests prevent drift. | -| S-02 | P1 | confirmed | Invite create/revoke are privileged mutations without the audit coverage used by sibling mutation families. | B4/B5 | Successful create/revoke produce safe, non-secret audit events; failure behavior is tested. | -| S-03 | P2 | confirmed | Admin channel name/topic/category validation lacks one explicit rune/normalization contract. | B3/B5 | Shared limits cover admin and user writers; boundary tests count runes, not bytes. | -| S-04 | P2 | confirmed | Sibling admin channel lookups expose inconsistent DM/not-found response contracts. | B3 | One non-DM resolution policy and response contract covers both paths. | -| S-05 | P2 | confirmed | Repository-wide Go formatting is not a required gate. | B1 | Tree is formatted and a fast required gate fails future drift. | -| S-06 | P2 | confirmed | Server coverage is uploaded without a global or core-package regression floor; current aggregate is 74.6%. | B3/B10 | Documented baseline/exclusions and ratcheted global/core thresholds. | -| S-07 | P2 | confirmed | Thousands of tests and 17 fuzz targets exist, but there are no Go benchmarks for hub/replay, permission, DB, or fan-out hot paths. | B6/B10 | Stable microbenchmarks and reference load baselines cover the highest-risk paths. | -| S-08 | P2 | confirmed | Large lifecycle/hub/serve files remain structural hotspots. | B3 | Cohesive extractions preserve lifecycle, locking, race, and deadlock invariants. | -| S-09 | P2 | confirmed | API/admin/WebSocket layers still contain many direct database call sites. | B3 | Each use moves behind a narrow service/store seam or is documented as an intentional transaction/composition boundary. | -| S-10 | P2 | confirmed | Auth routes still consume raw database ownership and are the first intended S-09 migration slice. | B3/B4 | Tested AuthService/narrow interfaces preserve enumeration and sentinel-error behavior. | -| S-11 | P2 | confirmed | Hub construction uses post-construction collaborator setters, leaving required wiring temporally coupled to `Run`. | B3 | Required collaborators are validated constructor/options inputs; only genuinely dynamic dependencies remain mutable. | -| S-12 | P2 | confirmed | Ready/refresh/WebSocket paths mirror message send-permission policy by hand. | B3 | All paths delegate to one value-taking predicate with parity tests. | -| S-13 | P2 | confirmed | Durable TOTP used-code and partial-auth persister work remains incomplete. | B4 | Hash-only persistence, expiry, restart, and failure-mode tests land without persisting sliding rate-limit windows. | -| S-14 | P1 | confirmed | Load tooling exists, but no supported capacity result is published for the approved 250 users / 100 connections / 25 voice profile. | B6/B10 | Reproducible report states hardware/software, CPU, memory, DB waits, p95/p99 latency, and pass/fail thresholds. | -| S-15 | P3 | verify | `voice_speakers` and `member_leave` remain reserved protocol entries with no production emit site. | B2 | Compatibility review removes unused entries before the epoch freeze or explicitly reserves and fixtures them; schema, generated types, docs, and tests agree. | -| S-16 | P3 | verify | Voice key-holder TOCTOU hardening remains a documented follow-up without a demonstrated contract failure. | B2/B3 | Threat-model review either records why outer checks suffice or adds an in-function recheck and race-focused private test. | -| S-17 | P3 | watch | Vulnerability tooling found no reachable Go advisory, while non-called/unmaintained upstream paths remain. | B6/B10 | Dependency path is monitored, compatible fixes are applied, and reachable-symbol scanning remains required. | +| ID | Pri | State | Issue and evidence | Phase | Closure evidence | +| ---- | --: | ------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| S-01 | P1 | resolved/superseded | Typing currently checks a weaker permission than posting. Landed in B2-5 (PR #1440): typing delegates to the send-policy predicate; denial, announcement, archive and override tests prevent drift. | B2/B3 | No separate action; the parity table locks it. | +| S-02 | P1 | confirmed | Invite create/revoke are privileged mutations without the audit coverage used by sibling mutation families. | B4/B5 | Successful create/revoke produce safe, non-secret audit events; failure behavior is tested. | +| S-03 | P2 | confirmed | Admin channel name/topic/category validation lacks one explicit rune/normalization contract. | B3/B5 | Shared limits cover admin and user writers; boundary tests count runes, not bytes. | +| S-04 | P2 | confirmed | Sibling admin channel lookups expose inconsistent DM/not-found response contracts. | B3 | One non-DM resolution policy and response contract covers both paths. | +| S-05 | P2 | confirmed | Repository-wide Go formatting is not a required gate. | B1 | Tree is formatted and a fast required gate fails future drift. | +| S-06 | P2 | confirmed | Server coverage is uploaded without a global or core-package regression floor; current aggregate is 74.6%. | B3/B10 | Documented baseline/exclusions and ratcheted global/core thresholds. | +| S-07 | P2 | confirmed | Thousands of tests and 17 fuzz targets exist, but there are no Go benchmarks for hub/replay, permission, DB, or fan-out hot paths. | B6/B10 | Stable microbenchmarks and reference load baselines cover the highest-risk paths. | +| S-08 | P2 | confirmed | Large lifecycle/hub/serve files remain structural hotspots. | B3 | Cohesive extractions preserve lifecycle, locking, race, and deadlock invariants. | +| S-09 | P2 | confirmed | API/admin/WebSocket layers still contain many direct database call sites. | B3 | Each use moves behind a narrow service/store seam or is documented as an intentional transaction/composition boundary. | +| S-10 | P2 | confirmed | Auth routes still consume raw database ownership and are the first intended S-09 migration slice. | B3/B4 | Tested AuthService/narrow interfaces preserve enumeration and sentinel-error behavior. | +| S-11 | P2 | confirmed | Hub construction uses post-construction collaborator setters, leaving required wiring temporally coupled to `Run`. | B3 | Required collaborators are validated constructor/options inputs; only genuinely dynamic dependencies remain mutable. | +| S-12 | P2 | resolved/superseded | Ready/refresh/WebSocket paths mirror message send-permission policy by hand. Landed in B2-5 (PR #1440): all paths delegate to one value-taking predicate with parity tests in both resolution branches. | B3 | No separate action; the authz-chokepoint invariant rule remains B3 item 15. | +| S-13 | P2 | confirmed | Durable TOTP used-code and partial-auth persister work remains incomplete. | B4 | Hash-only persistence, expiry, restart, and failure-mode tests land without persisting sliding rate-limit windows. | +| S-14 | P1 | confirmed | Load tooling exists, but no supported capacity result is published for the approved 250 users / 100 connections / 25 voice profile. | B6/B10 | Reproducible report states hardware/software, CPU, memory, DB waits, p95/p99 latency, and pass/fail thresholds. | +| S-15 | P3 | verify | `voice_speakers` and `member_leave` remain reserved protocol entries with no production emit site. | B2 | Compatibility review removes unused entries before the epoch freeze or explicitly reserves and fixtures them; schema, generated types, docs, and tests agree. | +| S-16 | P3 | verify | Voice key-holder TOCTOU hardening remains a documented follow-up without a demonstrated contract failure. | B2/B3 | Threat-model review either records why outer checks suffice or adds an in-function recheck and race-focused private test. | +| S-17 | P3 | watch | Vulnerability tooling found no reachable Go advisory, while non-called/unmaintained upstream paths remain. | B6/B10 | Dependency path is monitored, compatible fixes are applied, and reachable-symbol scanning remains required. | ## Repository, CI, documentation, and supply chain