fix(service): apply the full read/announcement gate to message edits (F3)

EditMessage authorized the non-DM path with permissions.SendMessages alone
while every sibling message sink requires ReadMessages plus the mutate bit, so
a user denied READ_MESSAGES could still rewrite an old post and have the edit
broadcast to the channel. The edit gate now calls the existing
checkSendPermission helper and collapses its error into the sink's
pre-existing opaque ErrForbidden, so the reply stays a non-oracle.

Verified by a panel of agents; the added test fails against the unpatched
tree, showing the edit succeeded before the fix.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
J3vb
2026-07-30 15:01:21 +02:00
co-authored by Claude Opus 5
parent 6258681731
commit 1da2aaddf0
2 changed files with 96 additions and 2 deletions
+16 -2
View File
@@ -283,7 +283,11 @@ func (s *MessageService) EditMessage(ctx context.Context, userID, msgID int64, r
// Channel type for DM-aware permissions.
ch, chErr := s.st.GetChannel(ctx, msg.ChannelID)
isDM := chErr == nil && ch != nil && ch.Type == "dm"
chanType := ""
if chErr == nil && ch != nil {
chanType = ch.Type
}
isDM := chanType == "dm"
if isDM {
ok, dmErr := s.st.IsDMParticipant(ctx, userID, msg.ChannelID)
@@ -293,7 +297,17 @@ func (s *MessageService) EditMessage(ctx context.Context, userID, msgID int64, r
if blkErr := requireDMNotBlocked(ctx, s.st, userID, msg.ChannelID); blkErr != nil {
return nil, blkErr
}
} else if !s.perms.HasChannelPerm(ctx, userID, msg.ChannelID, permissions.SendMessages) {
} else if permErr := s.checkSendPermission(ctx, userID, msg.ChannelID, chanType); permErr != nil {
// An edit injects new text into the channel and is fanned out to every
// reader, so it must clear the same gate as a send rather than
// SEND_MESSAGES alone: READ_MESSAGES so a role locked out of a private
// channel (the panel's "Can access" toggle denies
// READ_MESSAGES|CONNECT_VOICE and leaves SEND_MESSAGES intact) cannot
// rewrite its old posts, and the announcement rule so a demoted
// moderator cannot rewrite a trusted broadcast. Mirrors DeleteMessage,
// SetMessagePinned and handleReaction, which already require
// READ_MESSAGES. The reason is collapsed into this sink's single opaque
// error so the reply stays an ownership/permission non-oracle.
return nil, fmt.Errorf("%w: cannot edit this message", ErrForbidden)
}
+80
View File
@@ -386,6 +386,86 @@ func TestEditMessage_EmptyContentFails(t *testing.T) {
}
}
// TestEditMessage_DeniedReadCannotEdit locks the channel-lockout invariant on
// the edit sink, alongside TestDeleteMessage_DeniedReadCannotDelete and
// TestSetMessagePinned_DeniedReadCannotPin. Editing fans new text out to every
// reader of the channel, so it must clear the send gate: unchecking "Can
// access" writes deny = READ_MESSAGES|CONNECT_VOICE and leaves SEND_MESSAGES
// intact, and an announcement channel accepts new text only from
// MANAGE_MESSAGES — so a demoted moderator must not be able to rewrite the
// broadcast they posted while privileged.
func TestEditMessage_DeniedReadCannotEdit(t *testing.T) {
database := newTestDB(t)
seedRole(t, database, &db.Role{
ID: permissions.MemberRoleID,
Name: "member",
Permissions: permissions.SendMessages | permissions.ReadMessages,
Position: 1,
})
seedRole(t, database, &db.Role{
ID: permissions.ModeratorRoleID,
Name: "moderator",
Permissions: permissions.SendMessages | permissions.ReadMessages | permissions.ManageMessages,
Position: 10,
})
seedUser(t, database, &db.User{ID: 1, Username: "alice"})
seedUser(t, database, &db.User{ID: 2, Username: "mod_bob"})
seedUserRole(t, database, 1, permissions.MemberRoleID)
seedUserRole(t, database, 2, permissions.ModeratorRoleID)
seedChannel(t, database, &db.Channel{ID: 10, Name: "staff-private", Type: "text"})
seedChannel(t, database, &db.Channel{ID: 11, Name: "announcements", Type: "announcement"})
permSvc := NewPermissionService(database, permissions.NewChecker(database))
svc := NewMessageService(database, permSvc, nil)
sent, err := svc.SendMessage(context.Background(), SendMessageParams{
ChannelID: 10, UserID: 1, Username: "alice", Content: "private discussion",
})
if err != nil {
t.Fatalf("send: %v", err)
}
announced, err := svc.SendMessage(context.Background(), SendMessageParams{
ChannelID: 11, UserID: 2, Username: "mod_bob", Content: "payroll is on the 1st",
})
if err != nil {
t.Fatalf("send announcement: %v", err)
}
// Admin unchecks "Can access" for the member role: READ_MESSAGES and
// CONNECT_VOICE are denied, SEND_MESSAGES survives the deny mask.
seedChannelOverride(t, database, permissions.MemberRoleID, 10, 0, permissions.ReadMessages|permissions.ConnectVoice)
permSvc.InvalidateChannel(10)
if _, err := svc.EditMessage(context.Background(), 1, sent.MessageID, "payroll moved to http://evil/"); !errors.Is(err, ErrForbidden) {
t.Fatalf("author denied READ_MESSAGES must not edit: got %v", err)
}
// The moderator is demoted to member: MANAGE_MESSAGES is gone, so the
// announcement they authored is no longer theirs to rewrite.
seedUserRole(t, database, 2, permissions.MemberRoleID)
permSvc.InvalidateUser(2)
if _, err := svc.EditMessage(context.Background(), 2, announced.MessageID, "payroll moved to http://evil/"); !errors.Is(err, ErrForbidden) {
t.Fatalf("demoted author must not edit an announcement: got %v", err)
}
for _, tc := range []struct {
id int64
want string
}{
{sent.MessageID, "private discussion"},
{announced.MessageID, "payroll is on the 1st"},
} {
msg, err := database.GetMessage(context.Background(), tc.id)
if err != nil || msg == nil {
t.Fatalf("GetMessage(%d): %v", tc.id, err)
}
if msg.Content != tc.want {
t.Fatalf("message %d must survive the refused edit: got %q", tc.id, msg.Content)
}
}
}
func TestDeleteMessage_OwnerCanDelete(t *testing.T) {
svc, _ := newTestMessageService(t)