Server panic via outgoing webhook responses
Description
Mattermost versions 11.6.x <= 11.6.0, 11.5.x <= 11.5.3, 11.4.x <= 11.4.4, 10.11.x <= 10.11.14 fail to filter nil elements from outgoing webhook attachment payloads before processing, which allows an authenticated user to cause a denial of service (server process termination) via a crafted webhook callback response containing a null attachment entry.. Mattermost Advisory ID: MMSA-2026-00641
AI Insight
LLM-synthesized narrative grounded in this CVE's description and references.
Mattermost fails to filter nil elements from webhook attachment payloads, allowing authenticated users to cause denial of service via crafted callback response.
Vulnerability
Mattermost versions 11.6.x <= 11.6.0, 11.5.x <= 11.5.3, 11.4.x <= 11.4.4, and 10.11.x <= 10.11.14 fail to filter nil elements from outgoing webhook attachment payloads before processing. This allows an authenticated user to craft a webhook callback response containing a null attachment entry, leading to denial of service.
Exploitation
An authenticated user with the ability to configure outgoing webhooks can send a crafted callback response that includes a null attachment entry. The server processes this payload without filtering nil elements, causing the server process to terminate.
Impact
Successful exploitation results in denial of service (server process termination), disrupting service availability for all users. The attacker must be authenticated and have permissions to configure outgoing webhooks.
Mitigation
Mattermost has released security updates. Affected versions should be upgraded to the latest patched versions. Refer to the Mattermost Security Updates page [1] for details. No workarounds are mentioned.
AI Insight generated on May 25, 2026. Synthesized from this CVE's description and the cited reference URLs; citations are validated against the source bundle.
Affected products
1- Range: 10.11.x <=10.11.14, 11.4.x <=11.4.4, 11.5.x <=11.5.3, 11.6.x <=11.6.0
Patches
10a07e4b05ff82Added nil checks (#35755) (#36134)
3 files changed · +250 −7
server/channels/app/channel.go+6 −6 modified@@ -1580,6 +1580,12 @@ func (a *App) DeleteChannel(rctx request.CTX, channel *model.Channel, userID str return err } + deleteAt := model.GetMillis() + + if err := a.Srv().Store().Channel().Delete(channel.Id, deleteAt); err != nil { + return model.NewAppError("DeleteChannel", "app.channel.delete.app_error", nil, "", http.StatusInternalServerError).Wrap(err) + } + if user != nil { T := i18n.GetUserTranslations(user.Locale) @@ -1635,12 +1641,6 @@ func (a *App) DeleteChannel(rctx request.CTX, channel *model.Channel, userID str return model.NewAppError("DeleteChannel", "app.post_persistent_notification.delete_by_channel.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } - deleteAt := model.GetMillis() - - if err := a.Srv().Store().Channel().Delete(channel.Id, deleteAt); err != nil { - return model.NewAppError("DeleteChannel", "app.channel.delete.app_error", nil, "", http.StatusInternalServerError).Wrap(err) - } - a.Srv().Platform().InvalidateCacheForChannel(channel) var message *model.WebSocketEvent
server/channels/app/post_persistent_notification.go+26 −1 modified@@ -163,13 +163,25 @@ func (a *App) forEachPersistentNotificationPost(posts []*model.Post, fn func(pos return err } + var postsForPersistentNotificationCleanup []*model.Post + for _, post := range posts { channel := channelsMap[post.ChannelId] + if channel == nil { + postsForPersistentNotificationCleanup = append(postsForPersistentNotificationCleanup, post) + continue + } + team := teamsMap[channel.TeamId] // GMs and DMs don't belong to any team if channel.IsGroupOrDirect() { team = &model.Team{} + } else if team == nil { + // cleanup persistent notification for posts with missing teams when they are not DM or GM + postsForPersistentNotificationCleanup = append(postsForPersistentNotificationCleanup, post) + continue } + profileMap := channelProfileMap[channel.Id] // Ensure the sender is always in the profile map: for example, system admins can post @@ -210,6 +222,14 @@ func (a *App) forEachPersistentNotificationPost(posts []*model.Post, fn func(pos } } + if len(postsForPersistentNotificationCleanup) > 0 { + for _, post := range postsForPersistentNotificationCleanup { + if appErr := a.DeletePersistentNotification(request.EmptyContext(a.Log()), post); appErr != nil { + a.Log().Warn("Failed to delete persistent notification for post", mlog.String("post_id", post.Id), mlog.String("channel_id", post.ChannelId), mlog.Err(appErr)) + } + } + } + return nil } @@ -219,9 +239,14 @@ func (a *App) persistentNotificationsAuxiliaryData(channelsMap map[string]*model channelKeywords := make(map[string]MentionKeywords, len(channelsMap)) channelNotifyProps := make(map[string]map[string]model.StringMap, len(channelsMap)) for _, c := range channelsMap { + team := teamsMap[c.TeamId] + if team == nil && !c.IsGroupOrDirect() { + continue + } + // In DM, notifications can't be send to any 3rd person. if c.Type != model.ChannelTypeDirect { - groups, err := a.getGroupsAllowedForReferenceInChannel(c, teamsMap[c.TeamId]) + groups, err := a.getGroupsAllowedForReferenceInChannel(c, team) if err != nil { return nil, nil, nil, nil, errors.Wrapf(err, "failed to get profiles for channel %s", c.Id) }
server/channels/app/post_persistent_notification_test.go+218 −0 modified@@ -192,6 +192,224 @@ func TestDeletePersistentNotification(t *testing.T) { }) } +func TestForEachPersistentNotificationPost(t *testing.T) { + mainHelper.Parallel(t) + + t.Run("should cleanup posts whose channel no longer exists", func(t *testing.T) { + th := SetupWithStoreMock(t) + + user1 := &model.User{Id: "uid1", Username: "user-1"} + profileMap := map[string]*model.User{user1.Id: user1} + team := &model.Team{Id: "tid"} + channel := &model.Channel{Id: "chid", TeamId: team.Id, Type: model.ChannelTypeOpen} + + // post1 belongs to an existing channel; post2 belongs to a deleted/missing channel + post1 := &model.Post{Id: "pid1", ChannelId: channel.Id, Message: "hello @user-1", UserId: user1.Id} + post2 := &model.Post{Id: "pid2", ChannelId: "deleted-channel-id", Message: "hello", UserId: user1.Id} + + mockStore := th.App.Srv().Store().(*storemocks.Store) + + mockChannel := storemocks.ChannelStore{} + mockStore.On("Channel").Return(&mockChannel) + // Only return channel for post1; post2's channel is missing + mockChannel.On("GetChannelsByIds", mock.Anything, mock.Anything).Return([]*model.Channel{channel}, nil) + mockChannel.On("GetAllChannelMembersNotifyPropsForChannel", mock.Anything, mock.Anything).Return(map[string]model.StringMap{}, nil) + + mockTeam := storemocks.TeamStore{} + mockStore.On("Team").Return(&mockTeam) + mockTeam.On("GetMany", mock.Anything).Return([]*model.Team{team}, nil) + + mockUser := storemocks.UserStore{} + mockStore.On("User").Return(&mockUser) + mockUser.On("GetAllProfilesInChannel", mock.Anything, mock.Anything, mock.Anything).Return(profileMap, nil) + + mockGroup := storemocks.GroupStore{} + mockStore.On("Group").Return(&mockGroup) + mockGroup.On("GetGroups", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]*model.Group{}, nil) + + // DeletePersistentNotification mocks - the cleanup path calls GetSingle then Delete + mockPostPersistentNotification := storemocks.PostPersistentNotificationStore{} + mockStore.On("PostPersistentNotification").Return(&mockPostPersistentNotification) + mockPostPersistentNotification.On("GetSingle", post2.Id).Return(&model.PostPersistentNotifications{PostId: post2.Id}, nil) + mockPostPersistentNotification.On("Delete", []string{post2.Id}).Return(nil) + + th.App.Srv().SetLicense(getLicWithSkuShortName(model.LicenseShortSkuProfessional)) + cfg := th.App.Config() + *cfg.ServiceSettings.PostPriority = true + *cfg.ServiceSettings.AllowPersistentNotifications = true + + fnCalled := []string{} + err := th.App.forEachPersistentNotificationPost([]*model.Post{post1, post2}, func(post *model.Post, _ *model.Channel, _ *model.Team, _ *MentionResults, _ model.UserMap, _ map[string]map[string]model.StringMap) error { + fnCalled = append(fnCalled, post.Id) + return nil + }) + require.NoError(t, err) + + // The callback should only be called for post1 (valid channel) + assert.Equal(t, []string{"pid1"}, fnCalled) + // post2 persistent notification should have been cleaned up + mockPostPersistentNotification.AssertCalled(t, "Delete", []string{post2.Id}) + }) + + t.Run("should cleanup posts whose team no longer exists", func(t *testing.T) { + th := SetupWithStoreMock(t) + + user1 := &model.User{Id: "uid1", Username: "user-1"} + user2 := &model.User{Id: "uid2", Username: "user-2"} + profileMap := map[string]*model.User{user1.Id: user1, user2.Id: user2} + team := &model.Team{Id: "tid"} + channel := &model.Channel{Id: "chid", TeamId: team.Id, Type: model.ChannelTypeOpen} + // channelWithMissingTeam has a TeamId that won't be in teamsMap + channelWithMissingTeam := &model.Channel{Id: "chid2", TeamId: "deleted-team-id", Type: model.ChannelTypeOpen} + + post1 := &model.Post{Id: "pid1", ChannelId: channel.Id, Message: "hello @user-1", UserId: user2.Id} + post2 := &model.Post{Id: "pid2", ChannelId: channelWithMissingTeam.Id, Message: "hello @user-1", UserId: user2.Id} + + mockStore := th.App.Srv().Store().(*storemocks.Store) + + mockChannel := storemocks.ChannelStore{} + mockStore.On("Channel").Return(&mockChannel) + // Both channels exist, but only one team exists + mockChannel.On("GetChannelsByIds", mock.Anything, mock.Anything).Return([]*model.Channel{channel, channelWithMissingTeam}, nil) + mockChannel.On("GetAllChannelMembersNotifyPropsForChannel", mock.Anything, mock.Anything).Return(map[string]model.StringMap{}, nil) + + mockTeam := storemocks.TeamStore{} + mockStore.On("Team").Return(&mockTeam) + // Only return the team for channel, not for channelWithMissingTeam + mockTeam.On("GetMany", mock.Anything).Return([]*model.Team{team}, nil) + + mockUser := storemocks.UserStore{} + mockStore.On("User").Return(&mockUser) + mockUser.On("GetAllProfilesInChannel", mock.Anything, mock.Anything, mock.Anything).Return(profileMap, nil) + + mockGroup := storemocks.GroupStore{} + mockStore.On("Group").Return(&mockGroup) + mockGroup.On("GetGroups", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]*model.Group{}, nil) + + mockPostPersistentNotification := storemocks.PostPersistentNotificationStore{} + mockStore.On("PostPersistentNotification").Return(&mockPostPersistentNotification) + mockPostPersistentNotification.On("GetSingle", post2.Id).Return(&model.PostPersistentNotifications{PostId: post2.Id}, nil) + mockPostPersistentNotification.On("Delete", []string{post2.Id}).Return(nil) + + th.App.Srv().SetLicense(getLicWithSkuShortName(model.LicenseShortSkuProfessional)) + cfg := th.App.Config() + *cfg.ServiceSettings.PostPriority = true + *cfg.ServiceSettings.AllowPersistentNotifications = true + + fnCalled := []string{} + err := th.App.forEachPersistentNotificationPost([]*model.Post{post1, post2}, func(post *model.Post, _ *model.Channel, _ *model.Team, _ *MentionResults, _ model.UserMap, _ map[string]map[string]model.StringMap) error { + fnCalled = append(fnCalled, post.Id) + return nil + }) + require.NoError(t, err) + + // The callback should only be called for post1 (valid team) + assert.Equal(t, []string{"pid1"}, fnCalled) + // post2 persistent notification should have been cleaned up due to missing team + mockPostPersistentNotification.AssertCalled(t, "Delete", []string{post2.Id}) + }) + + t.Run("should not cleanup DM posts that have no team", func(t *testing.T) { + th := SetupWithStoreMock(t) + + user1 := &model.User{Id: "uid1", Username: "user-1"} + user2 := &model.User{Id: "uid2", Username: "user-2"} + profileMap := map[string]*model.User{user1.Id: user1, user2.Id: user2} + dmChannel := &model.Channel{Id: "dm-chid", TeamId: "", Type: model.ChannelTypeDirect, Name: model.GetDMNameFromIds(user1.Id, user2.Id)} + + post1 := &model.Post{Id: "pid1", ChannelId: dmChannel.Id, Message: "hello", UserId: user1.Id} + + mockStore := th.App.Srv().Store().(*storemocks.Store) + + mockChannel := storemocks.ChannelStore{} + mockStore.On("Channel").Return(&mockChannel) + mockChannel.On("GetChannelsByIds", mock.Anything, mock.Anything).Return([]*model.Channel{dmChannel}, nil) + + mockTeam := storemocks.TeamStore{} + mockStore.On("Team").Return(&mockTeam) + mockTeam.On("GetMany", mock.Anything).Return([]*model.Team{}, nil) + + mockUser := storemocks.UserStore{} + mockStore.On("User").Return(&mockUser) + mockUser.On("GetAllProfilesInChannel", mock.Anything, mock.Anything, mock.Anything).Return(profileMap, nil) + + mockGroup := storemocks.GroupStore{} + mockStore.On("Group").Return(&mockGroup) + mockGroup.On("GetGroups", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]*model.Group{}, nil) + + mockPostPersistentNotification := storemocks.PostPersistentNotificationStore{} + mockStore.On("PostPersistentNotification").Return(&mockPostPersistentNotification) + + th.App.Srv().SetLicense(getLicWithSkuShortName(model.LicenseShortSkuProfessional)) + cfg := th.App.Config() + *cfg.ServiceSettings.PostPriority = true + *cfg.ServiceSettings.AllowPersistentNotifications = true + + fnCalled := []string{} + err := th.App.forEachPersistentNotificationPost([]*model.Post{post1}, func(post *model.Post, _ *model.Channel, _ *model.Team, _ *MentionResults, _ model.UserMap, _ map[string]map[string]model.StringMap) error { + fnCalled = append(fnCalled, post.Id) + return nil + }) + require.NoError(t, err) + + // The callback should be called for the DM post even though there's no team + assert.Equal(t, []string{"pid1"}, fnCalled) + // Delete should NOT have been called — DMs don't need a team + mockPostPersistentNotification.AssertNotCalled(t, "Delete", mock.Anything) + }) + + t.Run("should not cleanup GM posts that have no team", func(t *testing.T) { + th := SetupWithStoreMock(t) + + user1 := &model.User{Id: "uid1", Username: "user-1"} + user2 := &model.User{Id: "uid2", Username: "user-2"} + user3 := &model.User{Id: "uid3", Username: "user-3"} + profileMap := map[string]*model.User{user1.Id: user1, user2.Id: user2, user3.Id: user3} + gmChannel := &model.Channel{Id: "gm-chid", TeamId: "", Type: model.ChannelTypeGroup} + + post1 := &model.Post{Id: "pid1", ChannelId: gmChannel.Id, Message: "hello @user-2", UserId: user1.Id} + + mockStore := th.App.Srv().Store().(*storemocks.Store) + + mockChannel := storemocks.ChannelStore{} + mockStore.On("Channel").Return(&mockChannel) + mockChannel.On("GetChannelsByIds", mock.Anything, mock.Anything).Return([]*model.Channel{gmChannel}, nil) + mockChannel.On("GetAllChannelMembersNotifyPropsForChannel", mock.Anything, mock.Anything).Return(map[string]model.StringMap{}, nil) + + mockTeam := storemocks.TeamStore{} + mockStore.On("Team").Return(&mockTeam) + mockTeam.On("GetMany", mock.Anything).Return([]*model.Team{}, nil) + + mockUser := storemocks.UserStore{} + mockStore.On("User").Return(&mockUser) + mockUser.On("GetAllProfilesInChannel", mock.Anything, mock.Anything, mock.Anything).Return(profileMap, nil) + + mockGroup := storemocks.GroupStore{} + mockStore.On("Group").Return(&mockGroup) + mockGroup.On("GetGroups", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]*model.Group{}, nil) + + mockPostPersistentNotification := storemocks.PostPersistentNotificationStore{} + mockStore.On("PostPersistentNotification").Return(&mockPostPersistentNotification) + + th.App.Srv().SetLicense(getLicWithSkuShortName(model.LicenseShortSkuProfessional)) + cfg := th.App.Config() + *cfg.ServiceSettings.PostPriority = true + *cfg.ServiceSettings.AllowPersistentNotifications = true + + fnCalled := []string{} + err := th.App.forEachPersistentNotificationPost([]*model.Post{post1}, func(post *model.Post, _ *model.Channel, _ *model.Team, _ *MentionResults, _ model.UserMap, _ map[string]map[string]model.StringMap) error { + fnCalled = append(fnCalled, post.Id) + return nil + }) + require.NoError(t, err) + + // The callback should be called for the GM post even though there's no team + assert.Equal(t, []string{"pid1"}, fnCalled) + // Delete should NOT have been called — GMs don't need a team + mockPostPersistentNotification.AssertNotCalled(t, "Delete", mock.Anything) + }) +} + func TestSendPersistentNotifications(t *testing.T) { mainHelper.Parallel(t) th := Setup(t).InitBasic(t)
3b21498788a7Added nil checks (#35755) (#36133)
3 files changed · +250 −7
server/channels/app/channel.go+6 −6 modified@@ -1605,6 +1605,12 @@ func (a *App) DeleteChannel(rctx request.CTX, channel *model.Channel, userID str return err } + deleteAt := model.GetMillis() + + if err := a.Srv().Store().Channel().Delete(channel.Id, deleteAt); err != nil { + return model.NewAppError("DeleteChannel", "app.channel.delete.app_error", nil, "", http.StatusInternalServerError).Wrap(err) + } + if user != nil { T := i18n.GetUserTranslations(user.Locale) @@ -1660,12 +1666,6 @@ func (a *App) DeleteChannel(rctx request.CTX, channel *model.Channel, userID str return model.NewAppError("DeleteChannel", "app.post_persistent_notification.delete_by_channel.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } - deleteAt := model.GetMillis() - - if err := a.Srv().Store().Channel().Delete(channel.Id, deleteAt); err != nil { - return model.NewAppError("DeleteChannel", "app.channel.delete.app_error", nil, "", http.StatusInternalServerError).Wrap(err) - } - a.Srv().Platform().InvalidateCacheForChannel(channel) var message *model.WebSocketEvent
server/channels/app/post_persistent_notification.go+26 −1 modified@@ -163,13 +163,25 @@ func (a *App) forEachPersistentNotificationPost(posts []*model.Post, fn func(pos return err } + var postsForPersistentNotificationCleanup []*model.Post + for _, post := range posts { channel := channelsMap[post.ChannelId] + if channel == nil { + postsForPersistentNotificationCleanup = append(postsForPersistentNotificationCleanup, post) + continue + } + team := teamsMap[channel.TeamId] // GMs and DMs don't belong to any team if channel.IsGroupOrDirect() { team = &model.Team{} + } else if team == nil { + // cleanup persistent notification for posts with missing teams when they are not DM or GM + postsForPersistentNotificationCleanup = append(postsForPersistentNotificationCleanup, post) + continue } + profileMap := channelProfileMap[channel.Id] // Ensure the sender is always in the profile map: for example, system admins can post @@ -210,6 +222,14 @@ func (a *App) forEachPersistentNotificationPost(posts []*model.Post, fn func(pos } } + if len(postsForPersistentNotificationCleanup) > 0 { + for _, post := range postsForPersistentNotificationCleanup { + if appErr := a.DeletePersistentNotification(request.EmptyContext(a.Log()), post); appErr != nil { + a.Log().Warn("Failed to delete persistent notification for post", mlog.String("post_id", post.Id), mlog.String("channel_id", post.ChannelId), mlog.Err(appErr)) + } + } + } + return nil } @@ -219,9 +239,14 @@ func (a *App) persistentNotificationsAuxiliaryData(channelsMap map[string]*model channelKeywords := make(map[string]MentionKeywords, len(channelsMap)) channelNotifyProps := make(map[string]map[string]model.StringMap, len(channelsMap)) for _, c := range channelsMap { + team := teamsMap[c.TeamId] + if team == nil && !c.IsGroupOrDirect() { + continue + } + // In DM, notifications can't be send to any 3rd person. if c.Type != model.ChannelTypeDirect { - groups, err := a.getGroupsAllowedForReferenceInChannel(c, teamsMap[c.TeamId]) + groups, err := a.getGroupsAllowedForReferenceInChannel(c, team) if err != nil { return nil, nil, nil, nil, errors.Wrapf(err, "failed to get profiles for channel %s", c.Id) }
server/channels/app/post_persistent_notification_test.go+218 −0 modified@@ -192,6 +192,224 @@ func TestDeletePersistentNotification(t *testing.T) { }) } +func TestForEachPersistentNotificationPost(t *testing.T) { + mainHelper.Parallel(t) + + t.Run("should cleanup posts whose channel no longer exists", func(t *testing.T) { + th := SetupWithStoreMock(t) + + user1 := &model.User{Id: "uid1", Username: "user-1"} + profileMap := map[string]*model.User{user1.Id: user1} + team := &model.Team{Id: "tid"} + channel := &model.Channel{Id: "chid", TeamId: team.Id, Type: model.ChannelTypeOpen} + + // post1 belongs to an existing channel; post2 belongs to a deleted/missing channel + post1 := &model.Post{Id: "pid1", ChannelId: channel.Id, Message: "hello @user-1", UserId: user1.Id} + post2 := &model.Post{Id: "pid2", ChannelId: "deleted-channel-id", Message: "hello", UserId: user1.Id} + + mockStore := th.App.Srv().Store().(*storemocks.Store) + + mockChannel := storemocks.ChannelStore{} + mockStore.On("Channel").Return(&mockChannel) + // Only return channel for post1; post2's channel is missing + mockChannel.On("GetChannelsByIds", mock.Anything, mock.Anything).Return([]*model.Channel{channel}, nil) + mockChannel.On("GetAllChannelMembersNotifyPropsForChannel", mock.Anything, mock.Anything).Return(map[string]model.StringMap{}, nil) + + mockTeam := storemocks.TeamStore{} + mockStore.On("Team").Return(&mockTeam) + mockTeam.On("GetMany", mock.Anything).Return([]*model.Team{team}, nil) + + mockUser := storemocks.UserStore{} + mockStore.On("User").Return(&mockUser) + mockUser.On("GetAllProfilesInChannel", mock.Anything, mock.Anything, mock.Anything).Return(profileMap, nil) + + mockGroup := storemocks.GroupStore{} + mockStore.On("Group").Return(&mockGroup) + mockGroup.On("GetGroups", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]*model.Group{}, nil) + + // DeletePersistentNotification mocks - the cleanup path calls GetSingle then Delete + mockPostPersistentNotification := storemocks.PostPersistentNotificationStore{} + mockStore.On("PostPersistentNotification").Return(&mockPostPersistentNotification) + mockPostPersistentNotification.On("GetSingle", post2.Id).Return(&model.PostPersistentNotifications{PostId: post2.Id}, nil) + mockPostPersistentNotification.On("Delete", []string{post2.Id}).Return(nil) + + th.App.Srv().SetLicense(getLicWithSkuShortName(model.LicenseShortSkuProfessional)) + cfg := th.App.Config() + *cfg.ServiceSettings.PostPriority = true + *cfg.ServiceSettings.AllowPersistentNotifications = true + + fnCalled := []string{} + err := th.App.forEachPersistentNotificationPost([]*model.Post{post1, post2}, func(post *model.Post, _ *model.Channel, _ *model.Team, _ *MentionResults, _ model.UserMap, _ map[string]map[string]model.StringMap) error { + fnCalled = append(fnCalled, post.Id) + return nil + }) + require.NoError(t, err) + + // The callback should only be called for post1 (valid channel) + assert.Equal(t, []string{"pid1"}, fnCalled) + // post2 persistent notification should have been cleaned up + mockPostPersistentNotification.AssertCalled(t, "Delete", []string{post2.Id}) + }) + + t.Run("should cleanup posts whose team no longer exists", func(t *testing.T) { + th := SetupWithStoreMock(t) + + user1 := &model.User{Id: "uid1", Username: "user-1"} + user2 := &model.User{Id: "uid2", Username: "user-2"} + profileMap := map[string]*model.User{user1.Id: user1, user2.Id: user2} + team := &model.Team{Id: "tid"} + channel := &model.Channel{Id: "chid", TeamId: team.Id, Type: model.ChannelTypeOpen} + // channelWithMissingTeam has a TeamId that won't be in teamsMap + channelWithMissingTeam := &model.Channel{Id: "chid2", TeamId: "deleted-team-id", Type: model.ChannelTypeOpen} + + post1 := &model.Post{Id: "pid1", ChannelId: channel.Id, Message: "hello @user-1", UserId: user2.Id} + post2 := &model.Post{Id: "pid2", ChannelId: channelWithMissingTeam.Id, Message: "hello @user-1", UserId: user2.Id} + + mockStore := th.App.Srv().Store().(*storemocks.Store) + + mockChannel := storemocks.ChannelStore{} + mockStore.On("Channel").Return(&mockChannel) + // Both channels exist, but only one team exists + mockChannel.On("GetChannelsByIds", mock.Anything, mock.Anything).Return([]*model.Channel{channel, channelWithMissingTeam}, nil) + mockChannel.On("GetAllChannelMembersNotifyPropsForChannel", mock.Anything, mock.Anything).Return(map[string]model.StringMap{}, nil) + + mockTeam := storemocks.TeamStore{} + mockStore.On("Team").Return(&mockTeam) + // Only return the team for channel, not for channelWithMissingTeam + mockTeam.On("GetMany", mock.Anything).Return([]*model.Team{team}, nil) + + mockUser := storemocks.UserStore{} + mockStore.On("User").Return(&mockUser) + mockUser.On("GetAllProfilesInChannel", mock.Anything, mock.Anything, mock.Anything).Return(profileMap, nil) + + mockGroup := storemocks.GroupStore{} + mockStore.On("Group").Return(&mockGroup) + mockGroup.On("GetGroups", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]*model.Group{}, nil) + + mockPostPersistentNotification := storemocks.PostPersistentNotificationStore{} + mockStore.On("PostPersistentNotification").Return(&mockPostPersistentNotification) + mockPostPersistentNotification.On("GetSingle", post2.Id).Return(&model.PostPersistentNotifications{PostId: post2.Id}, nil) + mockPostPersistentNotification.On("Delete", []string{post2.Id}).Return(nil) + + th.App.Srv().SetLicense(getLicWithSkuShortName(model.LicenseShortSkuProfessional)) + cfg := th.App.Config() + *cfg.ServiceSettings.PostPriority = true + *cfg.ServiceSettings.AllowPersistentNotifications = true + + fnCalled := []string{} + err := th.App.forEachPersistentNotificationPost([]*model.Post{post1, post2}, func(post *model.Post, _ *model.Channel, _ *model.Team, _ *MentionResults, _ model.UserMap, _ map[string]map[string]model.StringMap) error { + fnCalled = append(fnCalled, post.Id) + return nil + }) + require.NoError(t, err) + + // The callback should only be called for post1 (valid team) + assert.Equal(t, []string{"pid1"}, fnCalled) + // post2 persistent notification should have been cleaned up due to missing team + mockPostPersistentNotification.AssertCalled(t, "Delete", []string{post2.Id}) + }) + + t.Run("should not cleanup DM posts that have no team", func(t *testing.T) { + th := SetupWithStoreMock(t) + + user1 := &model.User{Id: "uid1", Username: "user-1"} + user2 := &model.User{Id: "uid2", Username: "user-2"} + profileMap := map[string]*model.User{user1.Id: user1, user2.Id: user2} + dmChannel := &model.Channel{Id: "dm-chid", TeamId: "", Type: model.ChannelTypeDirect, Name: model.GetDMNameFromIds(user1.Id, user2.Id)} + + post1 := &model.Post{Id: "pid1", ChannelId: dmChannel.Id, Message: "hello", UserId: user1.Id} + + mockStore := th.App.Srv().Store().(*storemocks.Store) + + mockChannel := storemocks.ChannelStore{} + mockStore.On("Channel").Return(&mockChannel) + mockChannel.On("GetChannelsByIds", mock.Anything, mock.Anything).Return([]*model.Channel{dmChannel}, nil) + + mockTeam := storemocks.TeamStore{} + mockStore.On("Team").Return(&mockTeam) + mockTeam.On("GetMany", mock.Anything).Return([]*model.Team{}, nil) + + mockUser := storemocks.UserStore{} + mockStore.On("User").Return(&mockUser) + mockUser.On("GetAllProfilesInChannel", mock.Anything, mock.Anything, mock.Anything).Return(profileMap, nil) + + mockGroup := storemocks.GroupStore{} + mockStore.On("Group").Return(&mockGroup) + mockGroup.On("GetGroups", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]*model.Group{}, nil) + + mockPostPersistentNotification := storemocks.PostPersistentNotificationStore{} + mockStore.On("PostPersistentNotification").Return(&mockPostPersistentNotification) + + th.App.Srv().SetLicense(getLicWithSkuShortName(model.LicenseShortSkuProfessional)) + cfg := th.App.Config() + *cfg.ServiceSettings.PostPriority = true + *cfg.ServiceSettings.AllowPersistentNotifications = true + + fnCalled := []string{} + err := th.App.forEachPersistentNotificationPost([]*model.Post{post1}, func(post *model.Post, _ *model.Channel, _ *model.Team, _ *MentionResults, _ model.UserMap, _ map[string]map[string]model.StringMap) error { + fnCalled = append(fnCalled, post.Id) + return nil + }) + require.NoError(t, err) + + // The callback should be called for the DM post even though there's no team + assert.Equal(t, []string{"pid1"}, fnCalled) + // Delete should NOT have been called — DMs don't need a team + mockPostPersistentNotification.AssertNotCalled(t, "Delete", mock.Anything) + }) + + t.Run("should not cleanup GM posts that have no team", func(t *testing.T) { + th := SetupWithStoreMock(t) + + user1 := &model.User{Id: "uid1", Username: "user-1"} + user2 := &model.User{Id: "uid2", Username: "user-2"} + user3 := &model.User{Id: "uid3", Username: "user-3"} + profileMap := map[string]*model.User{user1.Id: user1, user2.Id: user2, user3.Id: user3} + gmChannel := &model.Channel{Id: "gm-chid", TeamId: "", Type: model.ChannelTypeGroup} + + post1 := &model.Post{Id: "pid1", ChannelId: gmChannel.Id, Message: "hello @user-2", UserId: user1.Id} + + mockStore := th.App.Srv().Store().(*storemocks.Store) + + mockChannel := storemocks.ChannelStore{} + mockStore.On("Channel").Return(&mockChannel) + mockChannel.On("GetChannelsByIds", mock.Anything, mock.Anything).Return([]*model.Channel{gmChannel}, nil) + mockChannel.On("GetAllChannelMembersNotifyPropsForChannel", mock.Anything, mock.Anything).Return(map[string]model.StringMap{}, nil) + + mockTeam := storemocks.TeamStore{} + mockStore.On("Team").Return(&mockTeam) + mockTeam.On("GetMany", mock.Anything).Return([]*model.Team{}, nil) + + mockUser := storemocks.UserStore{} + mockStore.On("User").Return(&mockUser) + mockUser.On("GetAllProfilesInChannel", mock.Anything, mock.Anything, mock.Anything).Return(profileMap, nil) + + mockGroup := storemocks.GroupStore{} + mockStore.On("Group").Return(&mockGroup) + mockGroup.On("GetGroups", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]*model.Group{}, nil) + + mockPostPersistentNotification := storemocks.PostPersistentNotificationStore{} + mockStore.On("PostPersistentNotification").Return(&mockPostPersistentNotification) + + th.App.Srv().SetLicense(getLicWithSkuShortName(model.LicenseShortSkuProfessional)) + cfg := th.App.Config() + *cfg.ServiceSettings.PostPriority = true + *cfg.ServiceSettings.AllowPersistentNotifications = true + + fnCalled := []string{} + err := th.App.forEachPersistentNotificationPost([]*model.Post{post1}, func(post *model.Post, _ *model.Channel, _ *model.Team, _ *MentionResults, _ model.UserMap, _ map[string]map[string]model.StringMap) error { + fnCalled = append(fnCalled, post.Id) + return nil + }) + require.NoError(t, err) + + // The callback should be called for the GM post even though there's no team + assert.Equal(t, []string{"pid1"}, fnCalled) + // Delete should NOT have been called — GMs don't need a team + mockPostPersistentNotification.AssertNotCalled(t, "Delete", mock.Anything) + }) +} + func TestSendPersistentNotifications(t *testing.T) { mainHelper.Parallel(t) th := Setup(t).InitBasic(t)
2f2dabe523f8Added nil checks (#35755) (#36132)
3 files changed · +249 −7
server/channels/app/channel.go+5 −6 modified@@ -1605,6 +1605,11 @@ func (a *App) DeleteChannel(rctx request.CTX, channel *model.Channel, userID str return err } + deleteAt := model.GetMillis() + + if err := a.Srv().Store().Channel().Delete(channel.Id, deleteAt); err != nil { + return model.NewAppError("DeleteChannel", "app.channel.delete.app_error", nil, "", http.StatusInternalServerError).Wrap(err) + } if user != nil { T := i18n.GetUserTranslations(user.Locale) @@ -1660,12 +1665,6 @@ func (a *App) DeleteChannel(rctx request.CTX, channel *model.Channel, userID str return model.NewAppError("DeleteChannel", "app.post_persistent_notification.delete_by_channel.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } - deleteAt := model.GetMillis() - - if err := a.Srv().Store().Channel().Delete(channel.Id, deleteAt); err != nil { - return model.NewAppError("DeleteChannel", "app.channel.delete.app_error", nil, "", http.StatusInternalServerError).Wrap(err) - } - a.Srv().Platform().InvalidateCacheForChannel(channel) var message *model.WebSocketEvent
server/channels/app/post_persistent_notification.go+26 −1 modified@@ -163,13 +163,25 @@ func (a *App) forEachPersistentNotificationPost(posts []*model.Post, fn func(pos return err } + var postsForPersistentNotificationCleanup []*model.Post + for _, post := range posts { channel := channelsMap[post.ChannelId] + if channel == nil { + postsForPersistentNotificationCleanup = append(postsForPersistentNotificationCleanup, post) + continue + } + team := teamsMap[channel.TeamId] // GMs and DMs don't belong to any team if channel.IsGroupOrDirect() { team = &model.Team{} + } else if team == nil { + // cleanup persistent notification for posts with missing teams when they are not DM or GM + postsForPersistentNotificationCleanup = append(postsForPersistentNotificationCleanup, post) + continue } + profileMap := channelProfileMap[channel.Id] // Ensure the sender is always in the profile map: for example, system admins can post @@ -210,6 +222,14 @@ func (a *App) forEachPersistentNotificationPost(posts []*model.Post, fn func(pos } } + if len(postsForPersistentNotificationCleanup) > 0 { + for _, post := range postsForPersistentNotificationCleanup { + if appErr := a.DeletePersistentNotification(request.EmptyContext(a.Log()), post); appErr != nil { + a.Log().Warn("Failed to delete persistent notification for post", mlog.String("post_id", post.Id), mlog.String("channel_id", post.ChannelId), mlog.Err(appErr)) + } + } + } + return nil } @@ -219,9 +239,14 @@ func (a *App) persistentNotificationsAuxiliaryData(channelsMap map[string]*model channelKeywords := make(map[string]MentionKeywords, len(channelsMap)) channelNotifyProps := make(map[string]map[string]model.StringMap, len(channelsMap)) for _, c := range channelsMap { + team := teamsMap[c.TeamId] + if team == nil && !c.IsGroupOrDirect() { + continue + } + // In DM, notifications can't be send to any 3rd person. if c.Type != model.ChannelTypeDirect { - groups, err := a.getGroupsAllowedForReferenceInChannel(c, teamsMap[c.TeamId]) + groups, err := a.getGroupsAllowedForReferenceInChannel(c, team) if err != nil { return nil, nil, nil, nil, errors.Wrapf(err, "failed to get profiles for channel %s", c.Id) }
server/channels/app/post_persistent_notification_test.go+218 −0 modified@@ -192,6 +192,224 @@ func TestDeletePersistentNotification(t *testing.T) { }) } +func TestForEachPersistentNotificationPost(t *testing.T) { + mainHelper.Parallel(t) + + t.Run("should cleanup posts whose channel no longer exists", func(t *testing.T) { + th := SetupWithStoreMock(t) + + user1 := &model.User{Id: "uid1", Username: "user-1"} + profileMap := map[string]*model.User{user1.Id: user1} + team := &model.Team{Id: "tid"} + channel := &model.Channel{Id: "chid", TeamId: team.Id, Type: model.ChannelTypeOpen} + + // post1 belongs to an existing channel; post2 belongs to a deleted/missing channel + post1 := &model.Post{Id: "pid1", ChannelId: channel.Id, Message: "hello @user-1", UserId: user1.Id} + post2 := &model.Post{Id: "pid2", ChannelId: "deleted-channel-id", Message: "hello", UserId: user1.Id} + + mockStore := th.App.Srv().Store().(*storemocks.Store) + + mockChannel := storemocks.ChannelStore{} + mockStore.On("Channel").Return(&mockChannel) + // Only return channel for post1; post2's channel is missing + mockChannel.On("GetChannelsByIds", mock.Anything, mock.Anything).Return([]*model.Channel{channel}, nil) + mockChannel.On("GetAllChannelMembersNotifyPropsForChannel", mock.Anything, mock.Anything).Return(map[string]model.StringMap{}, nil) + + mockTeam := storemocks.TeamStore{} + mockStore.On("Team").Return(&mockTeam) + mockTeam.On("GetMany", mock.Anything).Return([]*model.Team{team}, nil) + + mockUser := storemocks.UserStore{} + mockStore.On("User").Return(&mockUser) + mockUser.On("GetAllProfilesInChannel", mock.Anything, mock.Anything, mock.Anything).Return(profileMap, nil) + + mockGroup := storemocks.GroupStore{} + mockStore.On("Group").Return(&mockGroup) + mockGroup.On("GetGroups", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]*model.Group{}, nil) + + // DeletePersistentNotification mocks - the cleanup path calls GetSingle then Delete + mockPostPersistentNotification := storemocks.PostPersistentNotificationStore{} + mockStore.On("PostPersistentNotification").Return(&mockPostPersistentNotification) + mockPostPersistentNotification.On("GetSingle", post2.Id).Return(&model.PostPersistentNotifications{PostId: post2.Id}, nil) + mockPostPersistentNotification.On("Delete", []string{post2.Id}).Return(nil) + + th.App.Srv().SetLicense(getLicWithSkuShortName(model.LicenseShortSkuProfessional)) + cfg := th.App.Config() + *cfg.ServiceSettings.PostPriority = true + *cfg.ServiceSettings.AllowPersistentNotifications = true + + fnCalled := []string{} + err := th.App.forEachPersistentNotificationPost([]*model.Post{post1, post2}, func(post *model.Post, _ *model.Channel, _ *model.Team, _ *MentionResults, _ model.UserMap, _ map[string]map[string]model.StringMap) error { + fnCalled = append(fnCalled, post.Id) + return nil + }) + require.NoError(t, err) + + // The callback should only be called for post1 (valid channel) + assert.Equal(t, []string{"pid1"}, fnCalled) + // post2 persistent notification should have been cleaned up + mockPostPersistentNotification.AssertCalled(t, "Delete", []string{post2.Id}) + }) + + t.Run("should cleanup posts whose team no longer exists", func(t *testing.T) { + th := SetupWithStoreMock(t) + + user1 := &model.User{Id: "uid1", Username: "user-1"} + user2 := &model.User{Id: "uid2", Username: "user-2"} + profileMap := map[string]*model.User{user1.Id: user1, user2.Id: user2} + team := &model.Team{Id: "tid"} + channel := &model.Channel{Id: "chid", TeamId: team.Id, Type: model.ChannelTypeOpen} + // channelWithMissingTeam has a TeamId that won't be in teamsMap + channelWithMissingTeam := &model.Channel{Id: "chid2", TeamId: "deleted-team-id", Type: model.ChannelTypeOpen} + + post1 := &model.Post{Id: "pid1", ChannelId: channel.Id, Message: "hello @user-1", UserId: user2.Id} + post2 := &model.Post{Id: "pid2", ChannelId: channelWithMissingTeam.Id, Message: "hello @user-1", UserId: user2.Id} + + mockStore := th.App.Srv().Store().(*storemocks.Store) + + mockChannel := storemocks.ChannelStore{} + mockStore.On("Channel").Return(&mockChannel) + // Both channels exist, but only one team exists + mockChannel.On("GetChannelsByIds", mock.Anything, mock.Anything).Return([]*model.Channel{channel, channelWithMissingTeam}, nil) + mockChannel.On("GetAllChannelMembersNotifyPropsForChannel", mock.Anything, mock.Anything).Return(map[string]model.StringMap{}, nil) + + mockTeam := storemocks.TeamStore{} + mockStore.On("Team").Return(&mockTeam) + // Only return the team for channel, not for channelWithMissingTeam + mockTeam.On("GetMany", mock.Anything).Return([]*model.Team{team}, nil) + + mockUser := storemocks.UserStore{} + mockStore.On("User").Return(&mockUser) + mockUser.On("GetAllProfilesInChannel", mock.Anything, mock.Anything, mock.Anything).Return(profileMap, nil) + + mockGroup := storemocks.GroupStore{} + mockStore.On("Group").Return(&mockGroup) + mockGroup.On("GetGroups", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]*model.Group{}, nil) + + mockPostPersistentNotification := storemocks.PostPersistentNotificationStore{} + mockStore.On("PostPersistentNotification").Return(&mockPostPersistentNotification) + mockPostPersistentNotification.On("GetSingle", post2.Id).Return(&model.PostPersistentNotifications{PostId: post2.Id}, nil) + mockPostPersistentNotification.On("Delete", []string{post2.Id}).Return(nil) + + th.App.Srv().SetLicense(getLicWithSkuShortName(model.LicenseShortSkuProfessional)) + cfg := th.App.Config() + *cfg.ServiceSettings.PostPriority = true + *cfg.ServiceSettings.AllowPersistentNotifications = true + + fnCalled := []string{} + err := th.App.forEachPersistentNotificationPost([]*model.Post{post1, post2}, func(post *model.Post, _ *model.Channel, _ *model.Team, _ *MentionResults, _ model.UserMap, _ map[string]map[string]model.StringMap) error { + fnCalled = append(fnCalled, post.Id) + return nil + }) + require.NoError(t, err) + + // The callback should only be called for post1 (valid team) + assert.Equal(t, []string{"pid1"}, fnCalled) + // post2 persistent notification should have been cleaned up due to missing team + mockPostPersistentNotification.AssertCalled(t, "Delete", []string{post2.Id}) + }) + + t.Run("should not cleanup DM posts that have no team", func(t *testing.T) { + th := SetupWithStoreMock(t) + + user1 := &model.User{Id: "uid1", Username: "user-1"} + user2 := &model.User{Id: "uid2", Username: "user-2"} + profileMap := map[string]*model.User{user1.Id: user1, user2.Id: user2} + dmChannel := &model.Channel{Id: "dm-chid", TeamId: "", Type: model.ChannelTypeDirect, Name: model.GetDMNameFromIds(user1.Id, user2.Id)} + + post1 := &model.Post{Id: "pid1", ChannelId: dmChannel.Id, Message: "hello", UserId: user1.Id} + + mockStore := th.App.Srv().Store().(*storemocks.Store) + + mockChannel := storemocks.ChannelStore{} + mockStore.On("Channel").Return(&mockChannel) + mockChannel.On("GetChannelsByIds", mock.Anything, mock.Anything).Return([]*model.Channel{dmChannel}, nil) + + mockTeam := storemocks.TeamStore{} + mockStore.On("Team").Return(&mockTeam) + mockTeam.On("GetMany", mock.Anything).Return([]*model.Team{}, nil) + + mockUser := storemocks.UserStore{} + mockStore.On("User").Return(&mockUser) + mockUser.On("GetAllProfilesInChannel", mock.Anything, mock.Anything, mock.Anything).Return(profileMap, nil) + + mockGroup := storemocks.GroupStore{} + mockStore.On("Group").Return(&mockGroup) + mockGroup.On("GetGroups", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]*model.Group{}, nil) + + mockPostPersistentNotification := storemocks.PostPersistentNotificationStore{} + mockStore.On("PostPersistentNotification").Return(&mockPostPersistentNotification) + + th.App.Srv().SetLicense(getLicWithSkuShortName(model.LicenseShortSkuProfessional)) + cfg := th.App.Config() + *cfg.ServiceSettings.PostPriority = true + *cfg.ServiceSettings.AllowPersistentNotifications = true + + fnCalled := []string{} + err := th.App.forEachPersistentNotificationPost([]*model.Post{post1}, func(post *model.Post, _ *model.Channel, _ *model.Team, _ *MentionResults, _ model.UserMap, _ map[string]map[string]model.StringMap) error { + fnCalled = append(fnCalled, post.Id) + return nil + }) + require.NoError(t, err) + + // The callback should be called for the DM post even though there's no team + assert.Equal(t, []string{"pid1"}, fnCalled) + // Delete should NOT have been called — DMs don't need a team + mockPostPersistentNotification.AssertNotCalled(t, "Delete", mock.Anything) + }) + + t.Run("should not cleanup GM posts that have no team", func(t *testing.T) { + th := SetupWithStoreMock(t) + + user1 := &model.User{Id: "uid1", Username: "user-1"} + user2 := &model.User{Id: "uid2", Username: "user-2"} + user3 := &model.User{Id: "uid3", Username: "user-3"} + profileMap := map[string]*model.User{user1.Id: user1, user2.Id: user2, user3.Id: user3} + gmChannel := &model.Channel{Id: "gm-chid", TeamId: "", Type: model.ChannelTypeGroup} + + post1 := &model.Post{Id: "pid1", ChannelId: gmChannel.Id, Message: "hello @user-2", UserId: user1.Id} + + mockStore := th.App.Srv().Store().(*storemocks.Store) + + mockChannel := storemocks.ChannelStore{} + mockStore.On("Channel").Return(&mockChannel) + mockChannel.On("GetChannelsByIds", mock.Anything, mock.Anything).Return([]*model.Channel{gmChannel}, nil) + mockChannel.On("GetAllChannelMembersNotifyPropsForChannel", mock.Anything, mock.Anything).Return(map[string]model.StringMap{}, nil) + + mockTeam := storemocks.TeamStore{} + mockStore.On("Team").Return(&mockTeam) + mockTeam.On("GetMany", mock.Anything).Return([]*model.Team{}, nil) + + mockUser := storemocks.UserStore{} + mockStore.On("User").Return(&mockUser) + mockUser.On("GetAllProfilesInChannel", mock.Anything, mock.Anything, mock.Anything).Return(profileMap, nil) + + mockGroup := storemocks.GroupStore{} + mockStore.On("Group").Return(&mockGroup) + mockGroup.On("GetGroups", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]*model.Group{}, nil) + + mockPostPersistentNotification := storemocks.PostPersistentNotificationStore{} + mockStore.On("PostPersistentNotification").Return(&mockPostPersistentNotification) + + th.App.Srv().SetLicense(getLicWithSkuShortName(model.LicenseShortSkuProfessional)) + cfg := th.App.Config() + *cfg.ServiceSettings.PostPriority = true + *cfg.ServiceSettings.AllowPersistentNotifications = true + + fnCalled := []string{} + err := th.App.forEachPersistentNotificationPost([]*model.Post{post1}, func(post *model.Post, _ *model.Channel, _ *model.Team, _ *MentionResults, _ model.UserMap, _ map[string]map[string]model.StringMap) error { + fnCalled = append(fnCalled, post.Id) + return nil + }) + require.NoError(t, err) + + // The callback should be called for the GM post even though there's no team + assert.Equal(t, []string{"pid1"}, fnCalled) + // Delete should NOT have been called — GMs don't need a team + mockPostPersistentNotification.AssertNotCalled(t, "Delete", mock.Anything) + }) +} + func TestSendPersistentNotifications(t *testing.T) { mainHelper.Parallel(t) th := Setup(t).InitBasic(t)
9292c9e9b1eaImproved processing of attachments (#35854) (#36102)
3 files changed · +35 −1
server/channels/app/slack.go+1 −1 modified@@ -90,7 +90,7 @@ func (a *App) ProcessSlackText(rctx request.CTX, text string) string { // documented here: https://api.slack.com/docs/attachments func (a *App) ProcessSlackAttachments(rctx request.CTX, attachments []*model.SlackAttachment) []*model.SlackAttachment { var nonNilAttachments = model.StringifySlackFieldValue(attachments) - for _, attachment := range attachments { + for _, attachment := range nonNilAttachments { attachment.Pretext = a.ProcessSlackText(rctx, attachment.Pretext) attachment.Text = a.ProcessSlackText(rctx, attachment.Text) attachment.Title = a.ProcessSlackText(rctx, attachment.Title)
server/channels/app/slack_test.go+26 −0 modified@@ -6,6 +6,8 @@ package app import ( "testing" + "github.com/stretchr/testify/require" + "github.com/mattermost/mattermost/server/public/model" ) @@ -32,6 +34,30 @@ func TestProcessSlackText(t *testing.T) { } } +func TestProcessMessageAttachmentsWithNilEntries(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + attachments := []*model.SlackAttachment{ + nil, + { + Pretext: "pretext", + Text: "text", + Title: "title", + }, + nil, + { + Pretext: "pretext2", + Text: "text2", + }, + } + + result := th.App.ProcessSlackAttachments(th.Context, attachments) + require.Len(t, result, 2) + require.Equal(t, "pretext", result[0].Pretext) + require.Equal(t, "pretext2", result[1].Pretext) +} + func TestProcessSlackAnnouncement(t *testing.T) { mainHelper.Parallel(t) th := Setup(t).InitBasic(t)
server/channels/app/webhook.go+8 −0 modified@@ -127,6 +127,14 @@ func (a *App) TriggerWebhook(rctx request.CTX, payload *model.OutgoingWebhookPay go func() { defer wg.Done() + defer func() { + if r := recover(); r != nil { + logger.Error("Recovered from panic in outgoing webhook goroutine", + mlog.String("url", url), + mlog.Any("panic", r), + ) + } + }() var accessToken *model.OutgoingOAuthConnectionToken
667dffe31dbbImproved processing of attachments (#35854) (#36103)
3 files changed · +36 −1
server/channels/app/slack.go+1 −1 modified@@ -91,7 +91,7 @@ func (a *App) ProcessSlackText(text string) string { // documented here: https://api.slack.com/docs/attachments func (a *App) ProcessSlackAttachments(attachments []*model.SlackAttachment) []*model.SlackAttachment { var nonNilAttachments = model.StringifySlackFieldValue(attachments) - for _, attachment := range attachments { + for _, attachment := range nonNilAttachments { attachment.Pretext = a.ProcessSlackText(attachment.Pretext) attachment.Text = a.ProcessSlackText(attachment.Text) attachment.Title = a.ProcessSlackText(attachment.Title)
server/channels/app/slack_test.go+27 −0 modified@@ -6,6 +6,8 @@ package app import ( "testing" + "github.com/stretchr/testify/require" + "github.com/mattermost/mattermost/server/public/model" ) @@ -33,6 +35,31 @@ func TestProcessSlackText(t *testing.T) { } } +func TestProcessMessageAttachmentsWithNilEntries(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic() + defer th.TearDown() + + attachments := []*model.SlackAttachment{ + nil, + { + Pretext: "pretext", + Text: "text", + Title: "title", + }, + nil, + { + Pretext: "pretext2", + Text: "text2", + }, + } + + result := th.App.ProcessSlackAttachments(attachments) + require.Len(t, result, 2) + require.Equal(t, "pretext", result[0].Pretext) + require.Equal(t, "pretext2", result[1].Pretext) +} + func TestProcessSlackAnnouncement(t *testing.T) { mainHelper.Parallel(t) th := Setup(t).InitBasic()
server/channels/app/webhook.go+8 −0 modified@@ -125,6 +125,14 @@ func (a *App) TriggerWebhook(c request.CTX, payload *model.OutgoingWebhookPayloa go func() { defer wg.Done() + defer func() { + if r := recover(); r != nil { + logger.Error("Recovered from panic in outgoing webhook goroutine", + mlog.String("url", url), + mlog.Any("panic", r), + ) + } + }() var accessToken *model.OutgoingOAuthConnectionToken
7526844c5052Fixed URL validation for integration actions (#35857) (#36108)
2 files changed · +203 −11
server/channels/app/integration_action.go+18 −11 modified@@ -331,16 +331,7 @@ func (a *App) DoActionRequest(c request.CTX, rawURL string, body []byte) (*http. req.Header.Set("Content-Type", "application/json") req.Header.Set("Accept", "application/json") - // Allow access to plugin routes for action buttons - var httpClient *http.Client - subpath, _ := utils.GetSubpathFromConfig(a.Config()) - siteURL, _ := url.Parse(*a.Config().ServiceSettings.SiteURL) - if inURL.Hostname() == siteURL.Hostname() && strings.HasPrefix(inURL.Path, path.Join(subpath, "plugins")) { - req.Header.Set(model.HeaderAuth, "Bearer "+c.Session().Token) - httpClient = a.HTTPService().MakeClient(true) - } else { - httpClient = a.HTTPService().MakeClient(false) - } + httpClient := a.getPostActionClient(c, inURL, req) resp, httpErr := httpClient.Do(req) if httpErr != nil { @@ -354,6 +345,20 @@ func (a *App) DoActionRequest(c request.CTX, rawURL string, body []byte) (*http. return resp, nil } +func (a *App) getPostActionClient(rctx request.CTX, inURL *url.URL, req *http.Request) *http.Client { + // Allow access to plugin routes for action buttons + var httpClient *http.Client + subpath, _ := utils.GetSubpathFromConfig(a.Config()) + siteURL, _ := url.Parse(*a.Config().ServiceSettings.SiteURL) + if inURL.Hostname() == siteURL.Hostname() && strings.HasPrefix(path.Clean(inURL.Path), path.Join(subpath, "plugins")) { + req.Header.Set(model.HeaderAuth, "Bearer "+rctx.Session().Token) + httpClient = a.HTTPService().MakeClient(true) + } else { + httpClient = a.HTTPService().MakeClient(false) + } + return httpClient +} + type LocalResponseWriter struct { data []byte headers http.Header @@ -387,13 +392,15 @@ func (ch *Channels) doPluginRequest(c request.CTX, method, rawURL string, values if err != nil { return nil, model.NewAppError("doPluginRequest", "api.post.do_action.action_integration.app_error", nil, "", http.StatusBadRequest).Wrap(err) } - result := strings.Split(inURL.Path, "/") + result := strings.Split(path.Clean(inURL.Path), "/") if len(result) < 2 { return nil, model.NewAppError("doPluginRequest", "api.post.do_action.action_integration.app_error", nil, "err=Unable to find pluginId", http.StatusBadRequest) } + if result[0] != "plugins" { return nil, model.NewAppError("doPluginRequest", "api.post.do_action.action_integration.app_error", nil, "err=plugins not in path", http.StatusBadRequest) } + pluginID := result[1] path := strings.TrimPrefix(inURL.Path, "plugins/"+pluginID)
server/channels/app/integration_action_test.go+185 −0 modified@@ -1198,6 +1198,105 @@ func TestPostActionRelativePluginURL(t *testing.T) { }) } +func TestGetPostActionClient(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic() + + tests := []struct { + name string + siteURL string + subpath string + requestURL string + expectAuth bool + }{ + { + name: "same host with plugin path gets auth", + siteURL: "http://localhost:8065", + requestURL: "http://localhost:8065/plugins/myplugin/action", + expectAuth: true, + }, + { + name: "same host with non-plugin path does not get auth", + siteURL: "http://localhost:8065", + requestURL: "http://localhost:8065/api/v4/posts", + expectAuth: false, + }, + { + name: "different host with plugin path does not get auth", + siteURL: "http://localhost:8065", + requestURL: "http://evil.com/plugins/myplugin/action", + expectAuth: false, + }, + { + name: "different host same port does not get auth", + siteURL: "http://localhost:8065", + requestURL: "http://attacker.com:8065/plugins/myplugin/action", + expectAuth: false, + }, + { + name: "path traversal to reach plugins does not get auth", + siteURL: "http://localhost:8065", + requestURL: "http://localhost:8065/api/../../plugins/myplugin", + expectAuth: true, // path.Clean normalizes to /plugins/myplugin + }, + { + name: "path traversal escaping plugins does not get auth", + siteURL: "http://localhost:8065", + requestURL: "http://localhost:8065/plugins/../api/v4/posts", + expectAuth: false, // path.Clean normalizes to /api/v4/posts + }, + { + name: "subpath with plugin path gets auth", + siteURL: "http://localhost:8065/mattermost", + subpath: "/mattermost", + requestURL: "http://localhost:8065/mattermost/plugins/myplugin/action", + expectAuth: true, + }, + { + name: "subpath without subpath prefix does not get auth", + siteURL: "http://localhost:8065/mattermost", + subpath: "/mattermost", + requestURL: "http://localhost:8065/plugins/myplugin/action", + expectAuth: false, // plugins path doesn't include subpath + }, + { + name: "empty path does not get auth", + siteURL: "http://localhost:8065", + requestURL: "http://localhost:8065/", + expectAuth: false, + }, + { + name: "plugins as query param does not get auth", + siteURL: "http://localhost:8065", + requestURL: "http://localhost:8065/api?path=plugins/myplugin", + expectAuth: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.SiteURL = tc.siteURL + }) + + inURL, err := url.Parse(tc.requestURL) + require.NoError(t, err) + + req, err := http.NewRequest("POST", tc.requestURL, nil) + require.NoError(t, err) + + _ = th.App.getPostActionClient(th.Context, inURL, req) + + if tc.expectAuth { + assert.NotEmpty(t, req.Header.Get(model.HeaderAuth), "expected auth header to be set") + assert.Contains(t, req.Header.Get(model.HeaderAuth), "Bearer ") + } else { + assert.Empty(t, req.Header.Get(model.HeaderAuth), "expected no auth header") + } + }) + } +} + func TestDoPluginRequest(t *testing.T) { mainHelper.Parallel(t) th := Setup(t) @@ -1303,4 +1402,90 @@ func TestDoPluginRequest(t *testing.T) { require.NotNil(t, resp) body, _ = io.ReadAll(resp.Body) assert.Equal(t, "param multiple not correct", string(body)) + + t.Run("should handle URLs with path traversals", func(t *testing.T) { + tests := []struct { + name string + rawURL string + expectErr bool + errDetail string + }{ + { + name: "path traversal to escape plugins directory", + rawURL: "/plugins/../../../etc/passwd", + expectErr: true, + errDetail: "plugins not in path", + }, + { + name: "path traversal with encoded slashes", + rawURL: "/plugins/..%2F..%2F..%2Fetc%2Fpasswd", + expectErr: true, // url.Parse decodes %2F, path.Clean normalizes traversal + errDetail: "plugins not in path", + }, + { + name: "double dot in plugin path", + rawURL: "/plugins/../plugins/myplugin/action", + expectErr: false, // path.Clean normalizes this back to plugins/myplugin/action + }, + { + name: "path traversal without leading slash", + rawURL: "plugins/../../../etc/passwd", + expectErr: true, + errDetail: "plugins not in path", + }, + { + name: "only plugins with no plugin ID", + rawURL: "/plugins/", + expectErr: true, + errDetail: "Unable to find pluginId", + }, + { + name: "just plugins no trailing slash", + rawURL: "/plugins", + expectErr: true, + errDetail: "Unable to find pluginId", + }, + { + name: "non-plugins path", + rawURL: "/api/v4/users", + expectErr: true, + errDetail: "plugins not in path", + }, + { + name: "path traversal via dot segments after plugin ID", + rawURL: "/plugins/myplugin/../../etc/passwd", + expectErr: true, + errDetail: "plugins not in path", + }, + { + name: "backslash traversal attempt", + rawURL: "/plugins/myplugin/..\\..\\etc\\passwd", + expectErr: false, // backslashes are not path separators in URL paths; treated as literal + }, + { + name: "null byte injection attempt", + rawURL: "/plugins/myplugin\x00/action", + expectErr: true, // url.Parse rejects URLs with null bytes + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + resp, appErr := th.App.doPluginRequest(th.Context, "GET", tc.rawURL, nil, nil) + if tc.expectErr { + require.NotNil(t, appErr, "expected error for URL: %s", tc.rawURL) + if tc.errDetail != "" { + assert.Contains(t, appErr.DetailedError, tc.errDetail) + } + } else { + // Should not return an app error from path validation; + // may still get a 404 if the plugin doesn't exist, which is fine. + assert.Nil(t, appErr, "unexpected error for URL: %s - %v", tc.rawURL, appErr) + if resp != nil { + resp.Body.Close() + } + } + }) + } + }) }
ef5911eeaeadImproved processing of attachments (#35854) (#35967)
3 files changed · +35 −1
server/channels/app/slack.go+1 −1 modified@@ -91,7 +91,7 @@ func (a *App) ProcessSlackText(rctx request.CTX, text string) string { // It's based on the spec from slack: https://api.slack.com/docs/attachments. func (a *App) ProcessMessageAttachments(rctx request.CTX, attachments []*model.MessageAttachment) []*model.MessageAttachment { var nonNilAttachments = model.StringifyMessageAttachmentFieldValue(attachments) - for _, attachment := range attachments { + for _, attachment := range nonNilAttachments { attachment.Pretext = a.ProcessSlackText(rctx, attachment.Pretext) attachment.Text = a.ProcessSlackText(rctx, attachment.Text) attachment.Title = a.ProcessSlackText(rctx, attachment.Title)
server/channels/app/slack_test.go+26 −0 modified@@ -6,6 +6,8 @@ package app import ( "testing" + "github.com/stretchr/testify/require" + "github.com/mattermost/mattermost/server/public/model" ) @@ -32,6 +34,30 @@ func TestProcessSlackText(t *testing.T) { } } +func TestProcessMessageAttachmentsWithNilEntries(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + attachments := []*model.MessageAttachment{ + nil, + { + Pretext: "pretext", + Text: "text", + Title: "title", + }, + nil, + { + Pretext: "pretext2", + Text: "text2", + }, + } + + result := th.App.ProcessMessageAttachments(th.Context, attachments) + require.Len(t, result, 2) + require.Equal(t, "pretext", result[0].Pretext) + require.Equal(t, "pretext2", result[1].Pretext) +} + func TestProcessSlackAnnouncement(t *testing.T) { mainHelper.Parallel(t) th := Setup(t).InitBasic(t)
server/channels/app/webhook.go+8 −0 modified@@ -127,6 +127,14 @@ func (a *App) TriggerWebhook(rctx request.CTX, payload *model.OutgoingWebhookPay go func() { defer wg.Done() + defer func() { + if r := recover(); r != nil { + logger.Error("Recovered from panic in outgoing webhook goroutine", + mlog.String("url", url), + mlog.Any("panic", r), + ) + } + }() var accessToken *model.OutgoingOAuthConnectionToken
f6760151c4a7Support Elasticsearch v9 (for v10.11) (#35925)
8 files changed · +182 −11
.github/workflows/server-ci-template.yml+24 −0 modified@@ -264,6 +264,30 @@ jobs: datasource: mmuser:mostest@tcp(mysql:3306)/mattermost_test?charset=utf8mb4&multiStatements=true&maxAllowedPacket=4194304 drivername: mysql logsartifact: mysql-server-test-logs + test-elasticsearch-v8: + name: Elasticsearch v8 Compatibility + needs: check-mattermost-vet + uses: ./.github/workflows/server-test-template.yml + secrets: inherit + with: + name: Elasticsearch v8 Compatibility + datasource: postgres://mmuser:mostest@postgres:5432/mattermost_test?sslmode=disable&connect_timeout=10 + drivername: postgres + logsartifact: elasticsearch-v8-server-test-logs + elasticsearch-version: "8.9.0" + test-target: "test-server-elasticsearch" + test-elasticsearch-v7: + name: Elasticsearch v7 Compatibility + needs: check-mattermost-vet + uses: ./.github/workflows/server-test-template.yml + secrets: inherit + with: + name: Elasticsearch v7 Compatibility + datasource: postgres://mmuser:mostest@postgres:5432/mattermost_test?sslmode=disable&connect_timeout=10 + drivername: postgres + logsartifact: elasticsearch-v7-server-test-logs + elasticsearch-version: "7.17.29" + test-target: "test-server-elasticsearch" test-coverage: # Skip coverage generation for cherry-pick PRs into release branches. if: ${{ github.event_name != 'pull_request' || !startsWith(github.event.pull_request.base.ref, 'release-') }}
.github/workflows/server-test-template.yml+15 −2 modified@@ -22,6 +22,14 @@ on: required: false type: boolean default: false + elasticsearch-version: + required: false + type: string + default: "9.0.0" + test-target: + required: false + type: string + default: "test-server" # -- Test sharding inputs (leave defaults for non-sharded callers) -- shard-index: required: false @@ -75,6 +83,8 @@ jobs: echo "${{ inputs.name }}" > server/test-name echo "${{ github.event.pull_request.number }}" > server/pr-number - name: Run docker compose + env: + ELASTICSEARCH_VERSION: ${{ inputs.elasticsearch-version }} run: | cd server/build docker compose --ansi never run --rm start_dependencies @@ -143,11 +153,11 @@ jobs: env: BUILD_IMAGE: mattermost/mattermost-build-server:${{ steps.go.outputs.GO_VERSION }} run: | - if [[ ${{ github.ref_name }} == 'master' && ${{ inputs.fullyparallel }} != true ]]; then + if [[ ${{ github.ref_name }} == 'master' && ${{ inputs.fullyparallel }} != true && "${{ inputs.test-target }}" == "test-server" ]]; then export RACE_MODE="-race" fi - TEST_TARGET="test-server${RACE_MODE}" + TEST_TARGET="${{ inputs.test-target }}${RACE_MODE}" BUILD_NUMBER="${GITHUB_HEAD_REF}-${GITHUB_RUN_ID}" DOCKER_CMD="make ${TEST_TARGET}" @@ -186,8 +196,10 @@ jobs: disable_search: true files: server/cover.out - name: Stop docker compose + if: ${{ always() }} run: | cd server/build + docker compose --ansi never logs --no-color > ../../docker-compose.log 2>&1 docker compose --ansi never stop - name: Archive logs if: ${{ always() }} @@ -200,4 +212,5 @@ jobs: server/cover.out server/test-name server/pr-number + docker-compose.log
server/build/docker-compose.common.yml+5 −1 modified@@ -87,7 +87,11 @@ services: LDAP_DOMAIN: "mm.test.com" LDAP_ADMIN_PASSWORD: "mostest" elasticsearch: - image: "mattermostdevelopment/mattermost-elasticsearch:8.9.0" + build: + context: . + dockerfile: ./Dockerfile.elasticsearch + args: + ELASTICSEARCH_VERSION: ${ELASTICSEARCH_VERSION:-9.0.0} networks: - mm-test environment:
server/build/Dockerfile.elasticsearch+4 −0 added@@ -0,0 +1,4 @@ +ARG ELASTICSEARCH_VERSION=9.0.0 +FROM docker.elastic.co/elasticsearch/elasticsearch:${ELASTICSEARCH_VERSION} + +RUN /usr/share/elasticsearch/bin/elasticsearch-plugin install --batch analysis-icu analysis-nori analysis-kuromoji analysis-smartcn
server/enterprise/elasticsearch/elasticsearch/check_version_test.go+109 −0 added@@ -0,0 +1,109 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.enterprise for license information. + +package elasticsearch + +import ( + "fmt" + "net/http" + "net/http/httptest" + "testing" + + elastic "github.com/elastic/go-elasticsearch/v8" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func newTestClient(t *testing.T, handler http.Handler) *elastic.TypedClient { + t.Helper() + ts := httptest.NewServer(handler) + t.Cleanup(ts.Close) + + client, err := elastic.NewTypedClient(elastic.Config{ + Addresses: []string{ts.URL}, + }) + require.NoError(t, err) + return client +} + +func infoHandler(version string) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.Header().Set("X-Elastic-Product", "Elasticsearch") + fmt.Fprintf(w, `{"cluster_name":"test","version":{"number":%q,"build_flavor":"default","build_hash":"abc","build_date":"2024-01-01","build_snapshot":false,"build_type":"docker","lucene_version":"9.0.0","minimum_wire_compatibility_version":"7.0.0","minimum_index_compatibility_version":"7.0.0"}}`, version) + } +} + +func TestCheckVersion(t *testing.T) { + tests := []struct { + name string + version string + wantVersion string + wantMajor int + wantErrID string + }{ + { + name: "ES 8 is supported", + version: "8.9.0", + wantVersion: "8.9.0", + wantMajor: 8, + }, + { + name: "ES 9 is supported", + version: "9.0.0", + wantVersion: "9.0.0", + wantMajor: 9, + }, + { + name: "ES 7 is supported", + version: "7.17.0", + wantVersion: "7.17.0", + wantMajor: 7, + }, + { + name: "ES 6 is too old", + version: "6.8.0", + wantErrID: "ent.elasticsearch.min_version.app_error", + }, + { + name: "ES 10 is too new", + version: "10.0.0", + wantErrID: "ent.elasticsearch.max_version.app_error", + }, + { + name: "invalid version string", + version: "invalid", + wantErrID: "ent.elasticsearch.start.parse_server_version.app_error", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := newTestClient(t, infoHandler(tc.version)) + version, major, appErr := checkVersion(client, nil) + if tc.wantErrID != "" { + require.NotNil(t, appErr) + assert.Equal(t, tc.wantErrID, appErr.Id) + } else { + require.Nil(t, appErr) + assert.Equal(t, tc.wantVersion, version) + assert.Equal(t, tc.wantMajor, major) + } + }) + } +} + +func TestCheckVersionConnectionError(t *testing.T) { + ts := httptest.NewServer(http.NotFoundHandler()) + ts.Close() // close immediately to force connection error + + client, err := elastic.NewTypedClient(elastic.Config{ + Addresses: []string{ts.URL}, + MaxRetries: 0, + }) + require.NoError(t, err) + + _, _, appErr := checkVersion(client, nil) + require.NotNil(t, appErr) + assert.Equal(t, "ent.elasticsearch.start.get_server_version.app_error", appErr.Id) +}
server/enterprise/elasticsearch/elasticsearch/elasticsearch.go+11 −7 modified@@ -28,7 +28,8 @@ import ( "github.com/elastic/go-elasticsearch/v8/typedapi/types/enums/sortorder" ) -const elasticsearchMaxVersion = 8 +const elasticsearchMinVersion = 7 +const elasticsearchMaxVersion = 9 var ( purgeIndexListAllowedIndexes = []string{common.IndexBaseChannels} @@ -106,7 +107,7 @@ func (es *ElasticsearchInterfaceImpl) Start() *model.AppError { return appErr } - version, major, appErr := checkMaxVersion(es.client, es.Platform.Config()) + version, major, appErr := checkVersion(es.client, es.Platform.Config()) if appErr != nil { return appErr } @@ -1245,7 +1246,7 @@ func (es *ElasticsearchInterfaceImpl) TestConfig(rctx request.CTX, cfg *model.Co return appErr } - _, _, appErr = checkMaxVersion(client, cfg) + _, _, appErr = checkVersion(client, cfg) if appErr != nil { return appErr } @@ -1830,19 +1831,22 @@ func (es *ElasticsearchInterfaceImpl) DeleteFilesBatch(rctx request.CTX, endTime return nil } -func checkMaxVersion(client *elastic.TypedClient, cfg *model.Config) (string, int, *model.AppError) { +func checkVersion(client *elastic.TypedClient, cfg *model.Config) (string, int, *model.AppError) { resp, err := client.API.Core.Info().Do(context.Background()) if err != nil { - return "", 0, model.NewAppError("Elasticsearch.checkMaxVersion", "ent.elasticsearch.start.get_server_version.app_error", map[string]any{"Backend": model.ElasticsearchSettingsESBackend}, "", http.StatusInternalServerError).Wrap(err) + return "", 0, model.NewAppError("Elasticsearch.checkVersion", "ent.elasticsearch.start.get_server_version.app_error", map[string]any{"Backend": model.ElasticsearchSettingsESBackend}, "", http.StatusInternalServerError).Wrap(err) } major, _, _, esErr := common.GetVersionComponents(resp.Version.Int) if esErr != nil { - return "", 0, model.NewAppError("Elasticsearch.checkMaxVersion", "ent.elasticsearch.start.parse_server_version.app_error", map[string]any{"Backend": model.ElasticsearchSettingsESBackend}, "", http.StatusInternalServerError).Wrap(err) + return "", 0, model.NewAppError("Elasticsearch.checkVersion", "ent.elasticsearch.start.parse_server_version.app_error", map[string]any{"Backend": model.ElasticsearchSettingsESBackend}, "", http.StatusInternalServerError).Wrap(esErr) } + if major < elasticsearchMinVersion { + return "", 0, model.NewAppError("Elasticsearch.checkVersion", "ent.elasticsearch.min_version.app_error", map[string]any{"Version": major, "MinVersion": elasticsearchMinVersion, "Backend": model.ElasticsearchSettingsESBackend}, "", http.StatusBadRequest) + } if major > elasticsearchMaxVersion { - return "", 0, model.NewAppError("Elasticsearch.checkMaxVersion", "ent.elasticsearch.max_version.app_error", map[string]any{"Version": major, "MaxVersion": elasticsearchMaxVersion, "Backend": model.ElasticsearchSettingsESBackend}, "", http.StatusBadRequest) + return "", 0, model.NewAppError("Elasticsearch.checkVersion", "ent.elasticsearch.max_version.app_error", map[string]any{"Version": major, "MaxVersion": elasticsearchMaxVersion, "Backend": model.ElasticsearchSettingsESBackend}, "", http.StatusBadRequest) } return resp.Version.Int, major, nil }
server/i18n/en.json+4 −0 modified@@ -8228,6 +8228,10 @@ "id": "ent.elasticsearch.max_version.app_error", "translation": "{{.Backend}} version {{.Version}} is higher than max supported version of {{.MaxVersion}}" }, + { + "id": "ent.elasticsearch.min_version.app_error", + "translation": "{{.Backend}} version {{.Version}} is lower than min supported version of {{.MinVersion}}" + }, { "id": "ent.elasticsearch.not_started.error", "translation": "{{.Backend}} is not started"
server/Makefile+10 −1 modified@@ -1,4 +1,4 @@ -.PHONY: build package run stop run-client run-server run-node run-haserver stop-haserver stop-client stop-server restart restart-server restart-client restart-haserver start-docker update-docker clean-dist clean nuke check-style check-client-style check-server-style check-unit-tests test dist run-client-tests setup-run-client-tests cleanup-run-client-tests test-client build-linux build-osx build-windows package-prep package-linux package-osx package-windows internal-test-web-client vet run-server-for-web-client-tests diff-config prepackaged-plugins prepackaged-binaries test-server test-server-ee test-server-quick test-server-race test-mmctl-unit test-mmctl-e2e test-mmctl test-mmctl-coverage mmctl-build mmctl-docs new-migration migrations-extract test-public mocks-public +.PHONY: build package run stop run-client run-server run-node run-haserver stop-haserver stop-client stop-server restart restart-server restart-client restart-haserver start-docker update-docker clean-dist clean nuke check-style check-client-style check-server-style check-unit-tests test dist run-client-tests setup-run-client-tests cleanup-run-client-tests test-client build-linux build-osx build-windows package-prep package-linux package-osx package-windows internal-test-web-client vet run-server-for-web-client-tests diff-config prepackaged-plugins prepackaged-binaries test-server test-server-ee test-server-elasticsearch test-server-quick test-server-race test-mmctl-unit test-mmctl-e2e test-mmctl test-mmctl-coverage mmctl-build mmctl-docs new-migration migrations-extract test-public mocks-public ROOT := $(dir $(abspath $(lastword $(MAKEFILE_LIST)))) @@ -476,6 +476,15 @@ test-server-ee: check-prereqs-enterprise start-docker gotestsum ## Runs EE tests @echo Running only EE tests $(GOBIN)/gotestsum --packages="$(EE_PACKAGES)" -- $(GOFLAGS) -timeout=20m +ES_PACKAGES=$(shell $(GO) list ./enterprise/elasticsearch/...) + +test-server-elasticsearch: export GOTESTSUM_FORMAT := $(GOTESTSUM_FORMAT) +test-server-elasticsearch: export GOTESTSUM_JUNITFILE := $(GOTESTSUM_JUNITFILE) +test-server-elasticsearch: export GOTESTSUM_JSONFILE := $(GOTESTSUM_JSONFILE) +test-server-elasticsearch: check-prereqs-enterprise start-docker gotestsum ## Runs Elasticsearch tests. + @echo Running only Elasticsearch tests + $(GOBIN)/gotestsum --rerun-fails=3 --packages="$(ES_PACKAGES)" -- $(GOFLAGS) -timeout=20m + test-server-quick: export GOTESTSUM_FORMAT := $(GOTESTSUM_FORMAT) test-server-quick: export GOTESTSUM_JUNITFILE := $(GOTESTSUM_JUNITFILE) test-server-quick: export GOTESTSUM_JSONFILE := $(GOTESTSUM_JSONFILE)
292d4b7ea15bBump Boards FIPS version to v9.2.4 (#36165) (#36170)
1 file changed · +1 −1
server/Makefile+1 −1 modified@@ -174,7 +174,7 @@ PLUGIN_PACKAGES += mattermost-plugin-channel-export-v1.3.0 ifeq ($(FIPS_ENABLED),true) PLUGIN_PACKAGES = mattermost-plugin-playbooks-v2.8.0%2Bc4449ac-fips PLUGIN_PACKAGES += mattermost-plugin-agents-v1.7.2%2B866e2dd-fips - PLUGIN_PACKAGES += mattermost-plugin-boards-v9.2.2%2B4282c63-fips + PLUGIN_PACKAGES += mattermost-plugin-boards-v9.2.4%2B5855fe1-fips endif EE_PACKAGES=$(shell $(GO) list $(BUILD_ENTERPRISE_DIR)/...)
fff6ab3a5851Bump Boards FIPS version to v9.2.4 (#36165) (#36169)
1 file changed · +1 −1
server/Makefile+1 −1 modified@@ -176,7 +176,7 @@ PLUGIN_PACKAGES += mattermost-plugin-channel-export-v1.3.0 ifeq ($(FIPS_ENABLED),true) PLUGIN_PACKAGES = mattermost-plugin-playbooks-v2.8.0%2Bc4449ac-fips PLUGIN_PACKAGES += mattermost-plugin-agents-v1.7.2%2B866e2dd-fips - PLUGIN_PACKAGES += mattermost-plugin-boards-v9.2.2%2B4282c63-fips + PLUGIN_PACKAGES += mattermost-plugin-boards-v9.2.4%2B5855fe1-fips endif EE_PACKAGES=$(shell $(GO) list $(BUILD_ENTERPRISE_DIR)/...)
Vulnerability mechanics
Root cause
"Missing nil-element filtering in outgoing webhook attachment processing allows a nil-pointer dereference panic."
Attack vector
An authenticated user crafts an outgoing webhook callback response that includes a null (`nil`) entry in the attachments array. When Mattermost processes this response via `TriggerWebhook`, the nil attachment is passed to `ProcessSlackAttachments`, which dereferences the nil pointer while iterating over the unfiltered `attachments` slice, causing a panic that terminates the server process. The attacker must have permission to configure or trigger an outgoing webhook whose callback URL returns the malicious payload.
Affected code
The vulnerability is in `server/channels/app/webhook.go` and `server/channels/app/slack.go`. In `TriggerWebhook`, outgoing webhook callback responses containing nil attachment entries are processed without filtering, causing a nil-pointer dereference when `ProcessSlackAttachments` iterates over the raw `attachments` slice instead of the already-filtered `nonNilAttachments` slice [patch_id=2473991][patch_id=2473990]. The `ProcessSlackAttachments` function declared `nonNilAttachments` but then looped over `attachments`, missing the filter.
What the fix does
The fix in `server/channels/app/slack.go` changes the loop variable from `attachments` to `nonNilAttachments`, so only non-nil attachments are processed [patch_id=2473991][patch_id=2473990]. Additionally, a `recover()` block was added in `TriggerWebhook` to catch any remaining panics in the outgoing webhook goroutine, preventing a crash from propagating [patch_id=2473991][patch_id=2473990]. The patches also add nil checks in `forEachPersistentNotificationPost` and `persistentNotificationsAuxiliaryData` to handle missing channels or teams gracefully, and reorder `DeleteChannel` to clean up persistent notifications before deleting the channel record [patch_id=2473992][patch_id=2473994].
Preconditions
- authThe attacker must be an authenticated Mattermost user.
- configThe attacker must be able to configure or trigger an outgoing webhook whose callback URL returns a crafted response.
- inputThe outgoing webhook callback response must contain a null (nil) entry in the attachments array.
Generated on May 25, 2026. Inputs: CWE entries + fix-commit diffs from this CVE's patches. Citations validated against bundle.
References
1- mattermost.com/security-updatesmitrevendor-advisory
News mentions
0No linked articles in our index yet.